MAINTAINERS | 1 + drivers/net/can/dev/skb.c | 123 +++++++++++++++++++++++++++++----------------- drivers/net/can/vxcan.c | 15 +++++- include/linux/can/core.h | 1 + include/linux/can/skb.h | 38 +++++--------- include/linux/skbuff.h | 3 ++ include/net/can.h | 28 +++++++++++ net/can/Kconfig | 1 + net/can/af_can.c | 23 ++++++--- net/can/bcm.c | 26 +++++++--- net/can/gw.c | 42 +++++++++------- net/can/isotp.c | 46 +++++++++++------ net/can/j1939/socket.c | 16 ++++-- net/can/j1939/transport.c | 39 +++++++++++---- net/can/raw.c | 23 ++++++--- net/core/skbuff.c | 4 ++ 16 files changed, 287 insertions(+), 142 deletions(-)
CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain
CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data. Those
CAN skbs do not have network/mac/transport headers nor other such
references for encapsulated protocols like ethernet/IP protocols.
To store data for CAN specific use-cases all CAN bus related skbuffs are
created with a 16 byte private skb headroom (struct can_skb_priv). Using
the skb headroom and accessing skb->head for this private data led to
several problems in the past likely due to "The struct can_skb_priv
business is highly unconventional for the networking stack." [1]
This patch set aims to remove the unconventional skb headroom usage for CAN
bus related skbuffs and use the common skb extensions instead.
[1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/
---
Changes in v2:
- Patch#1: use u32 instead of __u32 for "struct uniqframe::hash"
- Patch#3: use u{8,16} instead of __u{8,16} for "struct can_skb_ext"
- Patch#6: add missing patch
- Link to v1: https://patch.msgid.link/20260125201601.5018-1-socketcan@hartkopp.net
Changes in v3:
- Patch#2: new patch: rename dev_put() in CAN subsystem suggested by Checkpatch
- Patch#3: use netdev_put() instead of dev_put() suggested by Checkpatch
- Patch#3: initialize can_gw_hops in can_skb_ext_add() suggested by AI-bot
- Patch#6: add linebreak in sock_alloc_send_skb() to fit the 80 columns (Checkpatch)
- Link to v2: https://lore.kernel.org/linux-can/20260128-can-skb-ext-v2-0-fe64aa152c8a@pengutronix.de/
Changes in v4:
Solve the netdev_put() / dev_put() suggestion from Checkpatch in a separate
patch set. Therefore these changes are based on V2 again.
- Patch#2: initialize can_gw_hops in can_skb_ext_add() suggested by AI-bot
- Patch#5: add linebreak in sock_alloc_send_skb() to fit the 80 columns
- Link to v2: https://lore.kernel.org/linux-can/20260128-can-skb-ext-v2-0-fe64aa152c8a@pengutronix.de/
Changes in v5:
- Patch#2: use skb_ext_add() for the cloned skb suggested by AI-bot
- Patch#2: use netdev_put() instead of dev_put() suggested by Checkpatch
- managed to get the receipient list with b4 prep --auto-to-cc
- Link to v4: https://lore.kernel.org/netdev/20260128-can_skb_ext-v1-0-330f60fd5d7e@hartkopp.net/
Changes in v6:
- Patch#1: drop extern prototype in header file (Checkpatch)
- Patch#6: vxcan: correctly reset drop counter with skb extensions (AI-bot)
- Link to v5: https://patch.msgid.link/20260129-can_skb_ext-v5-0-21252fdc8900@hartkopp.net
Changes in v7:
- Patch#2: common handling for cloned skbs in gw.c j1939/transport.c vxcan.c
- Patch#6: vxcan: correctly reset drop counter based on updated Patch#2
- Link to v6: https://patch.msgid.link/20260130-can_skb_ext-v6-0-8fceafab7f26@hartkopp.net
Changes in v8:
- Patch#2: Fix double free in vxcan.c patch (AI-bot)
- Patch#2: Fix missing return value settings in error paths (AI-bot)
- Patch#2: Fix an indention issue in the original code (kernel test robot)
- Link to v7: https://patch.msgid.link/20260131-can_skb_ext-v7-0-dd0f8f84a83d@hartkopp.net
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
Oliver Hartkopp (6):
can: use skb hash instead of private variable in headroom
can: add CAN skb extension infrastructure
can: move ifindex to CAN skb extensions
can: move frame_len to CAN skb extensions
can: remove private CAN skb headroom infrastructure
can: gw: use can_gw_hops instead of sk_buff::csum_start
MAINTAINERS | 1 +
drivers/net/can/dev/skb.c | 123 +++++++++++++++++++++++++++++-----------------
drivers/net/can/vxcan.c | 15 +++++-
include/linux/can/core.h | 1 +
include/linux/can/skb.h | 38 +++++---------
include/linux/skbuff.h | 3 ++
include/net/can.h | 28 +++++++++++
net/can/Kconfig | 1 +
net/can/af_can.c | 23 ++++++---
net/can/bcm.c | 26 +++++++---
net/can/gw.c | 42 +++++++++-------
net/can/isotp.c | 46 +++++++++++------
net/can/j1939/socket.c | 16 ++++--
net/can/j1939/transport.c | 39 +++++++++++----
net/can/raw.c | 23 ++++++---
net/core/skbuff.c | 4 ++
16 files changed, 287 insertions(+), 142 deletions(-)
---
base-commit: 239f09e258b906deced5c2a7c1ac8aed301b558b
change-id: 20260128-can_skb_ext-0e7a99ed1984
Best regards,
--
Oliver Hartkopp <socketcan@hartkopp.net>
On 2/1/26 3:33 PM, Oliver Hartkopp via B4 Relay wrote: > CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain > CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data. Those > CAN skbs do not have network/mac/transport headers nor other such > references for encapsulated protocols like ethernet/IP protocols. > > To store data for CAN specific use-cases all CAN bus related skbuffs are > created with a 16 byte private skb headroom (struct can_skb_priv). Using > the skb headroom and accessing skb->head for this private data led to > several problems in the past likely due to "The struct can_skb_priv > business is highly unconventional for the networking stack." [1] > > This patch set aims to remove the unconventional skb headroom usage for CAN > bus related skbuffs and use the common skb extensions instead. > > [1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/ Could you please share how skb_ext size change with this series? (possibly breaking down the actual size to each separate extension). Ideally/hopefully the skbuff_ext_cache size is not going to change, and that would ensure that this change will not cause any indirect regressions. /P
Hello Paolo,
On 03.02.26 15:40, Paolo Abeni wrote:
> On 2/1/26 3:33 PM, Oliver Hartkopp via B4 Relay wrote:
>> CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain
>> CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data. Those
>> CAN skbs do not have network/mac/transport headers nor other such
>> references for encapsulated protocols like ethernet/IP protocols.
>>
>> To store data for CAN specific use-cases all CAN bus related skbuffs are
>> created with a 16 byte private skb headroom (struct can_skb_priv). Using
>> the skb headroom and accessing skb->head for this private data led to
>> several problems in the past likely due to "The struct can_skb_priv
>> business is highly unconventional for the networking stack." [1]
>>
>> This patch set aims to remove the unconventional skb headroom usage for CAN
>> bus related skbuffs and use the common skb extensions instead.
>>
>> [1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/
>
> Could you please share how skb_ext size change with this series?
> (possibly breaking down the actual size to each separate extension).
>
> Ideally/hopefully the skbuff_ext_cache size is not going to change, and
> that would ensure that this change will not cause any indirect regressions.
>
> /P
I'm not really sure what your question is about and how I could actively
change the impact of this series.
When CONFIG_CAN is enabled the skbuff_ext_cache element would increase
in size by 8 bytes (sizeof(struct can_skb_ext)) on my machine (see
pahole output below).
So when everything is enabled it would be
CONFIG_SKB_EXTENSIONS
8 bytes sizeof(struct skb_ext)
CONFIG_BRIDGE_NETFILTER
32 bytes sizeof(struct nf_bridge_info)
CONFIG_XFRM
88 bytes sizeof(struct sec_path)
CONFIG_NET_TC_SKB_EXT
16 bytes sizeof(struct tc_skb_ext)
CONFIG_MPTCP
32 bytes sizeof(struct mptcp_ext)
CONFIG_MCTP_FLOWS
8 bytes sizeof(struct mctp_flow)
CONFIG_INET_PSP
8 bytes sizeof(struct psp_skb_ext)
CONFIG_CAN
8 bytes sizeof(struct can_skb_ext)
---------
200 bytes total skbuff_ext_cache element size
(255 * 8 = 2040 bytes max space for skb extension users).
Does this answer your question?
Best regards,
Oliver
static const u8 skb_ext_type_len[] = {
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
[SKB_EXT_BRIDGE_NF] = SKB_EXT_CHUNKSIZEOF(struct nf_bridge_info),
#endif
#ifdef CONFIG_XFRM
[SKB_EXT_SEC_PATH] = SKB_EXT_CHUNKSIZEOF(struct sec_path),
#endif
#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
[TC_SKB_EXT] = SKB_EXT_CHUNKSIZEOF(struct tc_skb_ext),
#endif
#if IS_ENABLED(CONFIG_MPTCP)
[SKB_EXT_MPTCP] = SKB_EXT_CHUNKSIZEOF(struct mptcp_ext),
#endif
#if IS_ENABLED(CONFIG_MCTP_FLOWS)
[SKB_EXT_MCTP] = SKB_EXT_CHUNKSIZEOF(struct mctp_flow),
#endif
#if IS_ENABLED(CONFIG_INET_PSP)
[SKB_EXT_PSP] = SKB_EXT_CHUNKSIZEOF(struct psp_skb_ext),
#endif
#if IS_ENABLED(CONFIG_CAN)
[SKB_EXT_CAN] = SKB_EXT_CHUNKSIZEOF(struct can_skb_ext),
#endif
};
CONFIG_SKB_EXTENSIONS
struct skb_ext {
refcount_t refcnt; /* 0 4 */
u8 offset[3]; /* 4 3 */
u8 chunks; /* 7 1 */
char data[]
__attribute__((__aligned__(8))); /* 8 0 */
/* size: 8, cachelines: 1, members: 4 */
/* forced alignments: 1 */
/* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)));
CONFIG_BRIDGE_NETFILTER
struct nf_bridge_info {
enum {
BRNF_PROTO_UNCHANGED = 0,
BRNF_PROTO_8021Q = 1,
BRNF_PROTO_PPPOE = 2,
} orig_proto:8; /* 0: 0
4 */
/* Bitfield combined with next fields */
u8 pkt_otherhost:1; /* 1: 0 1 */
u8 in_prerouting:1; /* 1: 1 1 */
u8 bridged_dnat:1; /* 1: 2 1 */
u8 sabotage_in_done:1; /* 1: 3 1 */
/* XXX 4 bits hole, try to pack */
__u16 frag_max_size; /* 2 2 */
int physinif; /* 4 4 */
struct net_device * physoutdev; /* 8 8 */
union {
__be32 ipv4_daddr; /* 16 4 */
struct in6_addr ipv6_daddr; /* 16 16 */
char neigh_header[8]; /* 16 8 */
}; /* 16 16 */
/* size: 32, cachelines: 1, members: 9 */
/* sum members: 30 */
/* sum bitfield members: 12 bits, bit holes: 1, sum bit holes:
4 bits */
/* last cacheline: 32 bytes */
};
CONFIG_XFRM
struct sec_path {
int len; /* 0 4 */
int olen; /* 4 4 */
int verified_cnt; /* 8 4 */
/* XXX 4 bytes hole, try to pack */
struct xfrm_state * xvec[6]; /* 16 48 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct xfrm_offload ovec[1]; /* 64 24 */
/* size: 88, cachelines: 2, members: 5 */
/* sum members: 84, holes: 1, sum holes: 4 */
/* last cacheline: 24 bytes */
};
CONFIG_NET_TC_SKB_EXT
struct tc_skb_ext {
union {
u64 act_miss_cookie; /* 0 8 */
__u32 chain; /* 0 4 */
}; /* 0 8 */
__u16 mru; /* 8 2 */
__u16 zone; /* 10 2 */
u8 post_ct:1; /* 12: 0 1 */
u8 post_ct_snat:1; /* 12: 1 1 */
u8 post_ct_dnat:1; /* 12: 2 1 */
u8 act_miss:1; /* 12: 3 1 */
u8 l2_miss:1; /* 12: 4 1 */
/* size: 16, cachelines: 1, members: 8 */
/* padding: 3 */
/* bit_padding: 3 bits */
/* last cacheline: 16 bytes */
};
CONFIG_MPTCP
struct mptcp_ext {
union {
u64 data_ack; /* 0 8 */
u32 data_ack32; /* 0 4 */
}; /* 0 8 */
u64 data_seq; /* 8 8 */
u32 subflow_seq; /* 16 4 */
u16 data_len; /* 20 2 */
__sum16 csum; /* 22 2 */
u8 use_map:1; /* 24: 0 1 */
u8 dsn64:1; /* 24: 1 1 */
u8 data_fin:1; /* 24: 2 1 */
u8 use_ack:1; /* 24: 3 1 */
u8 ack64:1; /* 24: 4 1 */
u8 mpc_map:1; /* 24: 5 1 */
u8 frozen:1; /* 24: 6 1 */
u8 reset_transient:1; /* 24: 7 1 */
u8 reset_reason:4; /* 25: 0 1 */
u8 csum_reqd:1; /* 25: 4 1 */
u8 infinite_map:1; /* 25: 5 1 */
/* size: 32, cachelines: 1, members: 16 */
/* padding: 6 */
/* bit_padding: 2 bits */
/* last cacheline: 32 bytes */
};
CONFIG_MCTP_FLOWS
struct mctp_flow {
struct mctp_sk_key * key; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
CONFIG_INET_PSP
struct psp_skb_ext {
__be32 spi; /* 0 4 */
u16 dev_id; /* 4 2 */
u8 generation; /* 6 1 */
u8 version; /* 7 1 */
/* size: 8, cachelines: 1, members: 4 */
/* last cacheline: 8 bytes */
};
CONFIG_CAN
struct can_skb_ext {
int can_iif; /* 0 4 */
u16 can_framelen; /* 4 2 */
u8 can_gw_hops; /* 6 1 */
u8 can_ext_flags; /* 7 1 */
/* size: 8, cachelines: 1, members: 4 */
/* last cacheline: 8 bytes */
};
On 2/3/26 8:19 PM, Oliver Hartkopp wrote: > On 03.02.26 15:40, Paolo Abeni wrote: >> On 2/1/26 3:33 PM, Oliver Hartkopp via B4 Relay wrote: >>> CAN bus related skbuffs (ETH_P_CAN/ETH_P_CANFD/ETH_P_CANXL) simply contain >>> CAN frame structs for CAN CC/FD/XL of skb->len length at skb->data. Those >>> CAN skbs do not have network/mac/transport headers nor other such >>> references for encapsulated protocols like ethernet/IP protocols. >>> >>> To store data for CAN specific use-cases all CAN bus related skbuffs are >>> created with a 16 byte private skb headroom (struct can_skb_priv). Using >>> the skb headroom and accessing skb->head for this private data led to >>> several problems in the past likely due to "The struct can_skb_priv >>> business is highly unconventional for the networking stack." [1] >>> >>> This patch set aims to remove the unconventional skb headroom usage for CAN >>> bus related skbuffs and use the common skb extensions instead. >>> >>> [1] https://lore.kernel.org/linux-can/20260104074222.29e660ac@kernel.org/ >> >> Could you please share how skb_ext size change with this series? >> (possibly breaking down the actual size to each separate extension). >> >> Ideally/hopefully the skbuff_ext_cache size is not going to change, and >> that would ensure that this change will not cause any indirect regressions. >> >> /P > > I'm not really sure what your question is about and how I could actively > change the impact of this series. > > When CONFIG_CAN is enabled the skbuff_ext_cache element would increase > in size by 8 bytes (sizeof(struct can_skb_ext)) on my machine (see > pahole output below). > > So when everything is enabled it would be > > CONFIG_SKB_EXTENSIONS > 8 bytes sizeof(struct skb_ext) > CONFIG_BRIDGE_NETFILTER > 32 bytes sizeof(struct nf_bridge_info) > CONFIG_XFRM > 88 bytes sizeof(struct sec_path) > CONFIG_NET_TC_SKB_EXT > 16 bytes sizeof(struct tc_skb_ext) > CONFIG_MPTCP > 32 bytes sizeof(struct mptcp_ext) > CONFIG_MCTP_FLOWS > 8 bytes sizeof(struct mctp_flow) > CONFIG_INET_PSP > 8 bytes sizeof(struct psp_skb_ext) > CONFIG_CAN > 8 bytes sizeof(struct can_skb_ext) > --------- > 200 bytes total skbuff_ext_cache element size > (255 * 8 = 2040 bytes max space for skb extension users). > > Does this answer your question? Yes, thank you for the collaboration! I think there is mistake above: sizeof(struct skb_ext) should be 16 when more than 3 skb extensions are enabled. that means that the total extension size already exceeds the 3 cachelines (192 bytes) boundary when all extensions are enabled and adding the CAN one should not cause any regressions is such scenario. Note that the "all skb extensions enabled" is possibly/likely NOT the most common/relevant one, but I think there is some space we can squeeze elsewhere if needed. TL:DR; LGTM! Thanks, Paolo
On 05.02.26 11:54, Paolo Abeni wrote:
> On 2/3/26 8:19 PM, Oliver Hartkopp wrote:
>> When CONFIG_CAN is enabled the skbuff_ext_cache element would increase
>> in size by 8 bytes (sizeof(struct can_skb_ext)) on my machine (see
>> pahole output below).
>>
>> So when everything is enabled it would be
>>
>> CONFIG_SKB_EXTENSIONS
>> 8 bytes sizeof(struct skb_ext)
>> CONFIG_BRIDGE_NETFILTER
>> 32 bytes sizeof(struct nf_bridge_info)
>> CONFIG_XFRM
>> 88 bytes sizeof(struct sec_path)
>> CONFIG_NET_TC_SKB_EXT
>> 16 bytes sizeof(struct tc_skb_ext)
>> CONFIG_MPTCP
>> 32 bytes sizeof(struct mptcp_ext)
>> CONFIG_MCTP_FLOWS
>> 8 bytes sizeof(struct mctp_flow)
>> CONFIG_INET_PSP
>> 8 bytes sizeof(struct psp_skb_ext)
>> CONFIG_CAN
>> 8 bytes sizeof(struct can_skb_ext)
>> ---------
>> 200 bytes total skbuff_ext_cache element size
>> (255 * 8 = 2040 bytes max space for skb extension users).
>>
>> Does this answer your question?
>
> Yes, thank you for the collaboration!
>
> I think there is mistake above: sizeof(struct skb_ext) should be 16 when
> more than 3 skb extensions are enabled.
Oh, yes. You are right!
struct skb_ext {
refcount_t refcnt; /* 0 4 */
u8 offset[7]; /* 4 7 */
u8 chunks; /* 11 1 */
/* XXX 4 bytes hole, try to pack */
char data[]
__attribute__((__aligned__(8))); /* 16 0 */
/* size: 16, cachelines: 1, members: 4 */
/* sum members: 12, holes: 1, sum holes: 4 */
/* forced alignments: 1, forced holes: 1, sum forced holes: 4 */
/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
After I enabled all kernel configs that would increase the skb
extensions I just copied the missing pahole structs into the mail - not
looking for the struct skb_ext again ...
> that means that the total extension size already exceeds the 3
> cachelines (192 bytes) boundary when all extensions are enabled and
> adding the CAN one should not cause any regressions is such scenario.
>
> Note that the "all skb extensions enabled" is possibly/likely NOT the
> most common/relevant one, but I think there is some space we can squeeze
> elsewhere if needed.
Right.
Many thanks!
Oliver
© 2016 - 2026 Red Hat, Inc.