Re: [PATCH net-next v8 0/6] move CAN skb headroom content to skb extensions

[Paolo Abeni <pabeni@redhat.com>]

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