| Message ID | 1237539869-30721-1-git-send-email-leoli@freescale.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Mar 20, 2009 at 5:04 PM, Li Yang <leoli@freescale.com> wrote: > The bridging device used a constant hard_header_len. This will cause > headroom shortage for ports with additional hardware header. The patch > makes bridging device to use the maximum value of all ports. > > Signed-off-by: Li Yang <leoli@freescale.com> > --- > Fixes the following BUG when using bridging with gianfar driver: > > skb_under_panic: text:c0224b84 len:122 put:8 head:dfb81800 data:dfb817fa tail:0xdfb81874 end:0xdfb818a0 dev:eth1 > ------------[ cut here ]------------ > Kernel BUG at c02d9444 [verbose debug info unavailable] > Oops: Exception in kernel mode, sig: 5 [#1] > Call Trace: > [df2dbb20] [c02d9444] skb_under_panic+0x48/0x5c (unreliable) > [df2dbb30] [c0224b94] gfar_start_xmit+0x384/0x400 > [df2dbb60] [c02e1c8c] dev_hard_start_xmit+0x258/0x2cc > [df2dbba0] [c02f264c] __qdisc_run+0x5c/0x1f8 > [df2dbbd0] [c02e4bf4] dev_queue_xmit+0x264/0x2d0 > [df2dbbf0] [c036fdc8] br_dev_queue_push_xmit+0x90/0xf8 > [df2dbc00] [c036fcc8] br_flood+0xc8/0x120 > [df2dbc30] [c036ebe0] br_dev_xmit+0xbc/0xc0 > [df2dbc40] [c02e1c8c] dev_hard_start_xmit+0x258/0x2cc > [df2dbc80] [c02e4c04] dev_queue_xmit+0x274/0x2d0 > [df2dbca0] [c02ebaa8] neigh_resolve_output+0xfc/0x25c > ............ Any comment about this? Is it possible to be included in 2.6.29? - Leo -- 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
From: Li Yang <leoli@freescale.com> Date: Mon, 23 Mar 2009 15:59:24 +0800 > Any comment about this? Is it possible to be included in 2.6.29? Patience please? I reviewed and applied more than 80 patches yesterday, maybe I'll get to your's after I recover from that. Your patch is in the queue at: http://patchwork.ozlabs.org/project/netdev/list/ You can monitor it's state and what's happening to it here. -- 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
On Mon, Mar 23, 2009 at 4:02 PM, David Miller <davem@davemloft.net> wrote: > From: Li Yang <leoli@freescale.com> > Date: Mon, 23 Mar 2009 15:59:24 +0800 > >> Any comment about this? Is it possible to be included in 2.6.29? > > Patience please? I reviewed and applied more than 80 patches > yesterday, maybe I'll get to your's after I recover from that. > > Your patch is in the queue at: > > http://patchwork.ozlabs.org/project/netdev/list/ > > You can monitor it's state and what's happening to it here. Good to know it is being tracked. After you have applied all those patches, I was wondering if this one failed to get your attention. Thanks. - Leo -- 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
From: Li Yang <leoli@freescale.com> Date: Mon, 23 Mar 2009 16:15:34 +0800 > I was wondering if this one failed to get your attention. yeah, happens all the time -- 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
On Fri, 20 Mar 2009 17:04:29 +0800 Li Yang <leoli@freescale.com> wrote: > The bridging device used a constant hard_header_len. This will cause > headroom shortage for ports with additional hardware header. The patch > makes bridging device to use the maximum value of all ports. > > Signed-off-by: Li Yang <leoli@freescale.com> > --- That ensures big enough header for locally generated packets, but any drivers that need bigger headroom still must handle bridged packets that come in with smaller space. When bridging packets, the skb comes from the allocation by the receiving driver. Almost all drivers will use dev_alloc_skb() which will allocate NET_SKB_PAD (16) bytes of additional headroom. This is used to hold copy of ethernet header for the bridge/netfilter code. So your patch is fine as an optimization but a driver can not safely depend on any additional headroom. The driver must check if there is space, and if no space is available, reallocate and copy. -- 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
From: Li Yang <leoli@freescale.com> Date: Fri, 20 Mar 2009 17:04:29 +0800 > The bridging device used a constant hard_header_len. This will cause > headroom shortage for ports with additional hardware header. The patch > makes bridging device to use the maximum value of all ports. > > Signed-off-by: Li Yang <leoli@freescale.com> Your driver must be able to cope with any amount of available headroom, no matter what hacks we put into the bridging layer. Please fix your driver, I'm not applying this patch. -- 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
On Wed, Mar 25, 2009 at 2:40 PM, David Miller <davem@davemloft.net> wrote: > From: Li Yang <leoli@freescale.com> > Date: Fri, 20 Mar 2009 17:04:29 +0800 > >> The bridging device used a constant hard_header_len. This will cause >> headroom shortage for ports with additional hardware header. The patch >> makes bridging device to use the maximum value of all ports. >> >> Signed-off-by: Li Yang <leoli@freescale.com> > > Your driver must be able to cope with any amount of > available headroom, no matter what hacks we put into > the bridging layer. > > Please fix your driver, I'm not applying this patch. Ok. But it's not good to reallocate every packet generated locally. Why not take this patch too? - Leo -- 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
From: Li Yang <leoli@freescale.com> Date: Wed, 25 Mar 2009 15:05:20 +0800 > On Wed, Mar 25, 2009 at 2:40 PM, David Miller <davem@davemloft.net> wrote: > > From: Li Yang <leoli@freescale.com> > > Date: Fri, 20 Mar 2009 17:04:29 +0800 > > > >> The bridging device used a constant hard_header_len. This will cause > >> headroom shortage for ports with additional hardware header. The patch > >> makes bridging device to use the maximum value of all ports. > >> > >> Signed-off-by: Li Yang <leoli@freescale.com> > > > > Your driver must be able to cope with any amount of > > available headroom, no matter what hacks we put into > > the bridging layer. > > > > Please fix your driver, I'm not applying this patch. > > Ok. But it's not good to reallocate every packet generated locally. > Why not take this patch too? Because as Stephen showed it didn't handle all cases. Look at the patch I posted, that's the way to go. -- 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
On Wed, Mar 25, 2009 at 3:06 PM, David Miller <davem@davemloft.net> wrote: > From: Li Yang <leoli@freescale.com> > Date: Wed, 25 Mar 2009 15:05:20 +0800 > >> On Wed, Mar 25, 2009 at 2:40 PM, David Miller <davem@davemloft.net> wrote: >> > From: Li Yang <leoli@freescale.com> >> > Date: Fri, 20 Mar 2009 17:04:29 +0800 >> > >> >> The bridging device used a constant hard_header_len. This will cause >> >> headroom shortage for ports with additional hardware header. The patch >> >> makes bridging device to use the maximum value of all ports. >> >> >> >> Signed-off-by: Li Yang <leoli@freescale.com> >> > >> > Your driver must be able to cope with any amount of >> > available headroom, no matter what hacks we put into >> > the bridging layer. >> > >> > Please fix your driver, I'm not applying this patch. >> >> Ok. But it's not good to reallocate every packet generated locally. >> Why not take this patch too? > > Because as Stephen showed it didn't handle all cases. > > Look at the patch I posted, that's the way to go. Patch coming right away. However I have some comment about your way. The choice is yours. - Leo -- 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
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 727c5c5..d34303d 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -348,6 +348,7 @@ void br_features_recompute(struct net_bridge *br) { struct net_bridge_port *p; unsigned long features, mask; + unsigned short max_hard_header_len = ETH_HLEN; features = mask = br->feature_mask; if (list_empty(&br->port_list)) @@ -358,7 +359,10 @@ void br_features_recompute(struct net_bridge *br) list_for_each_entry(p, &br->port_list, list) { features = netdev_increment_features(features, p->dev->features, mask); + if (p->dev->hard_header_len > max_hard_header_len) + max_hard_header_len = p->dev->hard_header_len; } + br->dev->hard_header_len = max_hard_header_len; done: br->dev->features = netdev_fix_features(features, NULL);
The bridging device used a constant hard_header_len. This will cause headroom shortage for ports with additional hardware header. The patch makes bridging device to use the maximum value of all ports. Signed-off-by: Li Yang <leoli@freescale.com> --- Fixes the following BUG when using bridging with gianfar driver: skb_under_panic: text:c0224b84 len:122 put:8 head:dfb81800 data:dfb817fa tail:0xdfb81874 end:0xdfb818a0 dev:eth1 ------------[ cut here ]------------ Kernel BUG at c02d9444 [verbose debug info unavailable] Oops: Exception in kernel mode, sig: 5 [#1] Call Trace: [df2dbb20] [c02d9444] skb_under_panic+0x48/0x5c (unreliable) [df2dbb30] [c0224b94] gfar_start_xmit+0x384/0x400 [df2dbb60] [c02e1c8c] dev_hard_start_xmit+0x258/0x2cc [df2dbba0] [c02f264c] __qdisc_run+0x5c/0x1f8 [df2dbbd0] [c02e4bf4] dev_queue_xmit+0x264/0x2d0 [df2dbbf0] [c036fdc8] br_dev_queue_push_xmit+0x90/0xf8 [df2dbc00] [c036fcc8] br_flood+0xc8/0x120 [df2dbc30] [c036ebe0] br_dev_xmit+0xbc/0xc0 [df2dbc40] [c02e1c8c] dev_hard_start_xmit+0x258/0x2cc [df2dbc80] [c02e4c04] dev_queue_xmit+0x274/0x2d0 [df2dbca0] [c02ebaa8] neigh_resolve_output+0xfc/0x25c ............ net/bridge/br_if.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)