Re: RFC: on [ab]use of skb->cb by VLAN code
> > Do we really need an 'unsigned int' for mac_len? Maybe we could use > > a 16-bit counter here, and then use the other 16 bits for the VLAN bits? > > Not knowing exactly if/how it interacts with that specific field I > will point-out that IPoIB in OFED 1.2 just took their MTU to 65520. > While that doesn't break the bitbank it does get rather close. Leaving aside OFED releases, the IPoIB connected mode code in the standard kernel also allows the MTU to go up to 65520. And there's nothing magic about that value -- we could easily do bigger packets. However, this is irrelevant for two reasons: mac_len is the length of the LL header, not the packet overall, *and* mac_len is already 16 bits as of commit 334a8132. - R. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
Do we really need an 'unsigned int' for mac_len? Maybe we could use a 16-bit counter here, and then use the other 16 bits for the VLAN bits? Not knowing exactly if/how it interacts with that specific field I will point-out that IPoIB in OFED 1.2 just took their MTU to 65520. While that doesn't break the bitbank it does get rather close. rick jones - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
David Miller wrote: From: Ben Greear <[EMAIL PROTECTED]> Date: Mon, 30 Jul 2007 21:30:19 -0700 So, shall we add a new field to the skb in order to get the info out of cb? Looks like a single 32-bit field would be sufficient. I'm trying to brainstorm how to avoid consuming new space in sk_buff and I'd like others to do so as well. This isn't super urgent at all so I'll just think about it on the backburner, things like the NAPI changes and dealing with all of today's networking bug fixes is my top priority at the moment. Ok. From a quick look through skbuff.h, here is an idea: Do we really need an 'unsigned int' for mac_len? Maybe we could use a 16-bit counter here, and then use the other 16 bits for the VLAN bits? (I now think that only 16 bits are needed for VLAN, because it looks like the NICs must know how to grab the vlan encapsulated protocol out of the skb?? Or, maybe I just got confused when reading the vlan hwaccel logic...) Thanks, Ben -- Ben Greear <[EMAIL PROTECTED]> Candela Technologies Inc http://www.candelatech.com - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
From: Ben Greear <[EMAIL PROTECTED]> Date: Mon, 30 Jul 2007 21:30:19 -0700 > So, shall we add a new field to the skb in order to get the info out of cb? > > Looks like a single 32-bit field would be sufficient. I'm trying to brainstorm how to avoid consuming new space in sk_buff and I'd like others to do so as well. This isn't super urgent at all so I'll just think about it on the backburner, things like the NAPI changes and dealing with all of today's networking bug fixes is my top priority at the moment. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
David Miller wrote: From: jamal <[EMAIL PROTECTED]> Date: Mon, 30 Jul 2007 22:02:04 -0400 I came across the issue because i used cb in batching to store transient state which is used between qdisc dequeueing and hardware enqueueing (looked and smelled legit to me). Right, dequeue->device should be OK and doesn't work because of the VLAN issue. So, shall we add a new field to the skb in order to get the info out of cb? Looks like a single 32-bit field would be sufficient. Thanks, Ben -- Ben Greear <[EMAIL PROTECTED]> Candela Technologies Inc http://www.candelatech.com - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
From: jamal <[EMAIL PROTECTED]> Date: Mon, 30 Jul 2007 22:02:04 -0400 > I came across the issue because i used cb in batching to store transient > state which is used between qdisc dequeueing and hardware enqueueing > (looked and smelled legit to me). Right, dequeue->device should be OK and doesn't work because of the VLAN issue. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
On Mon, 2007-30-07 at 18:33 -0700, David Miller wrote: > I understand the concern, but how much qdisc stuff can possibly > happen between those two ->hard_start_xmit() calls and do we > want to support that in any way anyways? >From a quick glance only netem seems to use it in the fast path (in a legit way) Theoretically, you could have many generations (i.e parents and children, grandchildren etc) of netdevices stacked on top of each other each with qdiscs. In a simple example: dont know how well these days Vlans->bonding->somehardwarenetdevice works. Redirect will could also result in a graph of unrelated netdevices (and it is fair game to trample on cb anywhere along the path) I came across the issue because i used cb in batching to store transient state which is used between qdisc dequeueing and hardware enqueueing (looked and smelled legit to me). > The only alternative I see is to add more things to struct sk_buff > and that's usually very unpopular :-) I know ;-> Thats why i asked the question. cheers, jamal - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: on [ab]use of skb->cb by VLAN code
From: jamal <[EMAIL PROTECTED]> Date: Mon, 30 Jul 2007 20:59:16 -0400 > This seems like a bad use since there can be a lot of things between > a real hardware driver and something that sets a vlan tag (qdiscs come > to mind). I understand the concern, but how much qdisc stuff can possibly happen between those two ->hard_start_xmit() calls and do we want to support that in any way anyways? The only alternative I see is to add more things to struct sk_buff and that's usually very unpopular :-) - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: on [ab]use of skb->cb by VLAN code
I was going to forget this, but its been playing in the back of my head and wont go away Matt Carlson recently (while fixing the tg3 driver in my batching patches) pointed to me that skb->cb[] was being used to pass around vlan data. This seems like a bad use since there can be a lot of things between a real hardware driver and something that sets a vlan tag (qdiscs come to mind). Creating a new skb field be the reasonable thing to do here but i know that we are trying to avoid adding new fields. Thoughts? cheers, jamal - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html