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 1:04 PM
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: RE: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?

quoted

From: Haiyang Zhang <haiyangz@microsoft.com>
Sent: Saturday, February 15, 2020 7:20 AM
To: Dexuan Cui <decui@microsoft.com>; Stephen Hemminger

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);

quoted

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

quoted

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

Got it. So if the upper layer saves something in the cb, it must do a skb_clone()
and pass the new skb to hv_netvsc. hv_netvsc is the lowest layer in the network
stack, so it can use all the 48 bytes without calling skb_clone().

BTW, now I happen to have a different question: in netvsc_probe() we have
net->needed_headroom = RNDIS_AND_PPI_SIZE;
I think this means when the network stack (ARP, IP, ICMP, TCP, UDP,etc) passes
a
skb to hv_netvsc, the skb's headroom is increased by an extra size of
net->needed_headroom, right? Then in netvsc_xmit(), why do we still need to
call skb_cow_head(skb, RNDIS_AND_PPI_SIZE)? -- this looks unnecessary to me?

skb_cow_head() only expands the headroom if it is not enough, in case some 
upper layer path didn't reserve enough.

PS, what does the "cow" here mean? Copy On Write? It looks skb_cow_head()
just copies the data (if necessary) and it has nothing to do with the
write-protection in the MMU code.

Unrelated to MMU. It just copies some data to make room for writing.

Thanks,
- Haiyang