RE: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?

-----Original Message-----
From: Dexuan Cui <decui@microsoft.com>
Sent: Saturday, February 15, 2020 12:24 AM
To: Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger
[off-list ref]; David S. Miller [off-list ref];
netdev@vger.kernel.org; KY Srinivasan [off-list ref]; linux-
kernel@vger.kernel.org
Cc: linux-hyperv@vger.kernel.org
Subject: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?

Hi,
It looks all the layers of drivers among the network stack can use the 48-byte
skb->cb array. Is there any rule how they should coordinate with each other?

I noticed the last 16 bytes are used by struct skb_gso_cb:

include/linux/skbuff.h:
struct skb_gso_cb {
        union {
                int     mac_offset;
                int     data_offset;
        };
        int     encap_level;
        __wsum  csum;
        __u16   csum_start;
};
#define SKB_SGO_CB_OFFSET       32
#define SKB_GSO_CB(skb) ((struct skb_gso_cb *)((skb)->cb +
SKB_SGO_CB_OFFSET))

Does this mean a low level NIC driver (e.g. hv_netvsc) should only use the first
32 bytes? What if the upper layer network stack starts to take up more space in
the future?

According to the comments in skbuff.h below, it is the responsibility of the owning
layer to make a SKB clone, if it wants to keep the data across layers. So, every layer
can still use all of the 48 bytes.

        /*
         * This is the control buffer. It is free to use for every
         * layer. Please put your private variables there. If you
         * want to keep them across layers you have to do a skb_clone()
         * first. This is owned by whoever has the skb queued ATM.
         */
        char                    cb[48] __aligned(8);

Now hv_netvsc assumes it can use all of the 48-bytes, though it uses only
20 bytes, but just in case the struct hv_netvsc_packet grows to >32 bytes in the
future, should we change the BUILD_BUG_ON() in netvsc_start_xmit() to
BUILD_BUG_ON(sizeof(struct hv_netvsc_packet) > SKB_SGO_CB_OFFSET); ?

Based on the explanation above, the existing hv_netvsc code is correct.

Thanks,
- Haiyang