Re: [bpf, xdp] headroom - was: Re: Question about to KMSAN: uninit-value in can_receive

[Oliver Hartkopp]

Sorry for answering myself:

The below idea using skb->cb definitely does not work :-/

But as we never use encapsulation in CAN skbs we can use the 
inner_protocol and inner_xxx_header space when skb->encapsulation is false:

	union {
		/* encapsulation == true */
		struct {
			union {
				__be16		inner_protocol;
				__u8		inner_ipproto;
			};

			__u16			inner_transport_header;
			__u16			inner_network_header;
			__u16			inner_mac_header;
		};
		/* encapsulation == false */
		struct {
			int can_iif;
			__u16 can_frame_len;
		};
	};


Best regards,
Oliver

On 07.01.26 16:34, Oliver Hartkopp wrote:
> Hello Jakub,
>
> On 07.01.26 01:23, Jakub Kicinski wrote:
> > On Tue, 6 Jan 2026 13:04:41 +0100 Oliver Hartkopp wrote:
> > > When such skb is echo'ed back after successful transmission via
> > > netif_rx() this leads to skb->skb_iif = skb->dev->ifindex;
> > >
> > > To prevent a loopback the CAN frame must not be sent back to the
> > > originating interface - even when it has been routed to different CAN
> > > interfaces in the meantime (which always overwrites skb_iif).
> > >
> > > Therefore we need to maintain the "real original" incoming interface.
> >
> > Alternatively perhaps for this particular use case you could use
> > something like metadata_dst to mark the frame as forwarded / annotate
> > with the originating ifindex?
>
> I looked into it and the way how skb_dst is shared in the union behind 
> cb[] does not look very promising for skbs that wander up and down in 
> the network layer. And it is pretty complex to just store a single 
> interface index integer value.
>
> While looking into _sk_redir to see how the _skb_refdst union is used, 
> I've seen that the _sk_redir function was removed from struct tcp_skb_cb 
> (commit e3526bb92a208).
>
> Today we use skb->cb only for passing (address) information from the 
> network layer to the socket layer and user space. But the space in cb[] 
> could also hold the content we currently store in the problematic skb 
> headroom.
>
> Would using skb->cb be a good approach for CAN skbs (that do not have 
> any of the Ethernet/TCP/IP requirements/features) or will there still be 
> networking code (besides CAN drivers and CAN network layer) that writes 
> into cb[] when passing the CAN skb up and down in the stack?
>
> /**
>   * struct can_skb_cb - private data inside CAN skb->cb
>   * cb[] is 64 bit aligned which is also recommended for struct 
> sockaddr_can
>   * @magic:    to check if someone wrote to our CAN skb->cb space
>   * @flags:    extra flags for CAN_RAW and CAN_BCM sockets
>   * @can_addr:    socket address information to userspace
>   * @can_iif:    ifindex of the first interface the CAN frame appeared on
>   * @skbcnt:    atomic counter to have an unique id together with skb 
> pointer
>   * @frame_len:    bql length cache of CAN frame in data link layer
>   */
> struct can_skb_cb {
>      u32 magic;
>      u32 flags;
>      struct sockaddr_can can_addr;
>      int can_iif;
>      int skbcnt;
>      unsigned int frame_len;
> };
>
> If not: We also don't have vlans nor inner[protocol|headers] in CAN 
> where we might store the 4 byte can_iif integer ...
>
> Many thanks and best regards,
> Oliver