Can ndo_select_queue save data in skb->cb?

Can ndo_select_queue save data in skb->cb?

[James Yonan]

Is it permissible for a net driver's ndo_select_queue method to save 
data in skb->cb for later use in ndo_start_xmit?

Also, is it necessary for users of skb->cb to zero out their private 
data after use to prevent it from being misinterpreted by other layers? 
  I noticed some commits in the log (such as 462fb2) are zeroing out the 
skb->cb area for this reason.

Thanks,
James

Re: Can ndo_select_queue save data in skb->cb?

[Eric Dumazet]

On Wed, 2014-11-05 at 10:57 -0700, James Yonan wrote:
> Is it permissible for a net driver's ndo_select_queue method to save 
> data in skb->cb for later use in ndo_start_xmit?
> 
> Also, is it necessary for users of skb->cb to zero out their private 
> data after use to prevent it from being misinterpreted by other layers? 
>   I noticed some commits in the log (such as 462fb2) are zeroing out the 
> skb->cb area for this reason.

Its ok to use skb->cb[] from ndo_select_queue()

Look at bond_select_queue() for such a case.

You do not need to cleanup skb->cb[] to zero before giving skb to the
driver.

Re: Can ndo_select_queue save data in skb->cb?

[Cong Wang]

On Wed, Nov 5, 2014 at 10:16 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-11-05 at 10:57 -0700, James Yonan wrote:
>> Is it permissible for a net driver's ndo_select_queue method to save
>> data in skb->cb for later use in ndo_start_xmit?
>>
>> Also, is it necessary for users of skb->cb to zero out their private
>> data after use to prevent it from being misinterpreted by other layers?
>>   I noticed some commits in the log (such as 462fb2) are zeroing out the
>> skb->cb area for this reason.
>
> Its ok to use skb->cb[] from ndo_select_queue()
>
> Look at bond_select_queue() for such a case.
>
> You do not need to cleanup skb->cb[] to zero before giving skb to the
> driver.
>

That is only because qdisc layer saves data for bond,
which means if you have more data to save, you have to put
more into qdisc CB, it is just ugly.

Re: Can ndo_select_queue save data in skb->cb?

[Eric Dumazet]

On Wed, 2014-11-05 at 10:38 -0800, Cong Wang wrote:
> That is only because qdisc layer saves data for bond,
> which means if you have more data to save, you have to put
> more into qdisc CB, it is just ugly.

Right, using skb->cb[] is ugly, dangerous, and all this.

If you can, avoiding it is safer.

Note that I pointed to the bonding case for a legal case,
I had no time to do a full explanation, feel free to do so ;)

Re: Can ndo_select_queue save data in skb->cb?

[John Fastabend]

On 11/05/2014 10:53 AM, Eric Dumazet wrote:
> On Wed, 2014-11-05 at 10:38 -0800, Cong Wang wrote:
>> That is only because qdisc layer saves data for bond,
>> which means if you have more data to save, you have to put
>> more into qdisc CB, it is just ugly.
>
> Right, using skb->cb[] is ugly, dangerous, and all this.
>
> If you can, avoiding it is safer.
>
> Note that I pointed to the bonding case for a legal case,
> I had no time to do a full explanation, feel free to do so ;)
>

I would prefer to get rid of select_queue() versus expand
it. It mostly seems to be used for hacks around the stack
for FCoE/DCB/whatever. The wireless drivers and bonding/team
drivers are two examples where its not clear how to remove
it. See xen-netfront.c for an example where I'm not sure
why its needed at all.

James, why is your driver feeding itself data via cb? Maybe
we could solve the problem some other way.

Thanks,
John

>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
John Fastabend         Intel Corporation