| Message ID | 20101215013431.GA21173@mcarlson.broadcom.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 14 Dec 2010 17:34:31 -0800 "Matt Carlson" <mcarlson@broadcom.com> wrote: > On Tue, Dec 14, 2010 at 04:24:30PM -0800, Michael Leun wrote: > > On Tue, 14 Dec 2010 11:15:00 -0800 > > "Matt Carlson" <mcarlson@broadcom.com> wrote: > > > Michael, I'm wondering if the difference in behavior can be > > > explained by the presence or absence of management firmware. Can > > > you look at the driver sign-on messages in your syslogs for > > > ASF[]? I'm half expecting the 5752 to show "ASF[0]" and the 5714 > > > to show "ASF[1]". If you see this, and the below patch doesn't > > > fix the problem, let me know. I have another test I'd like you > > > to run. > > > > Do I understand this correct? "Management firmware" or ASF is some > > feature, vendor decides to built into network card (firmware) or > > not? > > Right. > > > If so, would'nt one expect two oneboard network cards in one server > > to look alike? > > Mostly, yes. Except for..... > > > HP Proliant DL320G5 > > > > <6>tg3.c:v3.113 (August 2, 2010) > > <6>tg3 0000:03:04.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 > > <6>tg3 0000:03:04.0: eth0: Tigon3 [partno(N/A) rev 9003] > > (PCIX:133MHz:64-bit) MAC address xx:xx:xx:xx:xx:xx <6>tg3 > > 0000:03:04.0: eth0: attached PHY is 5714 (10/100/1000Base-T > > Ethernet) (WireSpeed[1]) <6>tg3 0000:03:04.0: eth0: RXcsums[1] > > LinkChgREG[0] MIirq[0] ASF[1] TSOcap[1] > This => ^^^^^^ > > <6>tg3 0000:03:04.0: eth0: dma_rwctrl[76148000] dma_mask[64-bit] > > <6>tg3 0000:03:04.1: PCI INT B -> GSI 17 (level, low) -> IRQ 17 > > <6>tg3 0000:03:04.1: eth1: Tigon3 [partno(N/A) rev 9003] > > (PCIX:133MHz:64-bit) MAC address xx:xx:xx:xx:xx:xx <6>tg3 > > 0000:03:04.1: eth1: attached PHY is 5714 (10/100/1000Base-T > > Ethernet) (WireSpeed[1]) <6>tg3 0000:03:04.1: eth1: RXcsums[1] > > LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1] > And this => ^^^^^^ > > <6>tg3 0000:03:04.1: eth1: dma_rwctrl[76148000] dma_mask[64-bit] > > So management firmware is turned off on the second port. > > > Lenovo ThinkPad z61m > > > > [ 2.679130] tg3.c:v3.113 (August 2, 2010) > > [ 2.679176] tg3 0000:02:00.0: PCI INT A -> GSI 16 (level, low) > > -> IRQ 16 [ 2.679188] tg3 0000:02:00.0: setting latency timer to > > 64 [ 2.728572] tg3 0000:02:00.0: eth0: Tigon3 [partno(BCM95752m) > > rev 6002] (PCI Express) MAC address xx:xx:xx:xx:xx:xx > > [ 2.728577] tg3 0000:02:00.0: eth0: attached PHY is 5752 > > (10/100/1000Base-T Ethernet) (WireSpeed[1]) [ 2.728581] tg3 > > 0000:02:00.0: eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] > > TSOcap[1] > ^^^^^^ > And it isn't present on the 5752. > > > [ 2.728585] tg3 0000:02:00.0: eth0: dma_rwctrl[76180000] > > dma_mask[64-bit] > > > > > > > ---- > > > > > > [PATCH] tg3: Use new VLAN code > > > > Unfortunately had'nt time to try much now, but with 2.6.37-rc5 / > > your patch on the DL320, single user mode (nothing configured on > > eth) just after ifconfig eth0/eth1 up I see NO vlan tags on eth0 > > but I see vlan tags on eth1, so there clearly is a difference. > > > > I should have checked if I still see vlan tags on eth1 if I > > configure some vlan there - if helpful maybe I can do this (have to > > look, when I can effort another downtime). > > This would be helpful, just to solidify our findings. > > > I wonder, if the difference in that both onboard cards is really > > there or if there is some malfunction in detecion? > > Please run the above test first, but afterwards, can you apply the > below patch on top of your current sources. I suspect eth1 will > begin to act like eth0. > > This patch is just a test. > > [PATCH] tg3: Always strip VLAN tags > > This patch configures the hardware to always strip VLAN tags from > incoming packets. OK - all tests done on that DL320G5: For completeness, 2.6.37-rc5 unpatched: eth0, no vlan configured: totally broken - see double tagged vlans without tag, single or untagged packets missing at all eth0, vlan configured: see packets without vlan tag (see double tagged packets with one vlan tag) eth1 same as originally reported: without vlan configured see vlan tags (single and double tagged as expected) with vlan configured: see packets without vlan tag (see double tagged packets with one vlan tag) 2.6.37-rc5, your tg3 use new vlan-code patch: eth0, no vlan configured: see packets without vlan tag (see double tagged packets with one vlan tag) eth1, no vlan configured: see vlan tags (single and double tagged as expected) eth0, vlan configured: as without vlan eth1, vlan configured: as without vlan 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop eth1 no vlan configured: see packets without vlan tag (see double tagged packets with one vlan tag) eth1 with vlan: the same Hope I did not miss a test I should have done - hope, I did not confuse anything. If something is not what you would expect you might ask, I've a script from that session and can look (but cannot post that, sorry).
On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun <lkml20101129@newton.leun.net> wrote: > OK - all tests done on that DL320G5: > > For completeness, 2.6.37-rc5 unpatched: > > eth0, no vlan configured: totally broken - see double tagged vlans > without tag, single or untagged packets missing at all Random behavior? This one is somewhat hard to explain - maybe there are some other factors. eth0 has ASF on, so it always strips tags. I would expect it to behave like the vlan configured case. > > eth0, vlan configured: see packets without vlan tag (see double tagged > packets with one vlan tag) Both ASF and vlan group configured cause tag stripping to be enabled. Missing tag. > > eth1 same as originally reported: > without vlan configured see vlan tags (single and double tagged as > expected) No ASF and no vlan group means tag stripping is disabled. Have tag. > with vlan configured: see packets without vlan tag (see double tagged > packets with one vlan tag) Configuring vlan group causes stripping to be enabled. Missing tag. > > > 2.6.37-rc5, your tg3 use new vlan-code patch: > > eth0, no vlan configured: see packets without vlan tag (see double > tagged packets with one vlan tag) ASF enables tag stripping. Missing tag. > eth1, no vlan configured: see vlan tags (single and double tagged as > expected) No ASF, no vlan group means no stripping. Have tag. > > > eth0, vlan configured: as without vlan ASF enables stripping. Missing tag. > eth1, vlan configured: as without vlan With this patch vlan stripping is only enabled when ASF is on, so no stripping. Have tag. > > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > > eth1 no vlan configured: see packets without vlan tag (see double tagged > packets with one vlan tag) With the second patch, vlan stripping is always enabled. Missing tag. > eth1 with vlan: the same Stripping still always enabled. Missing tag. The bottom line is whenever vlan stripping is enabled we're missing the outer tag. It might be worth adding some debugging in the area before napi_gro_receive/vlan_gro_receive (depending on version). My guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for vlan packets on this NIC. You said that everything works on the 5752? Matt, is it possible that the 5714 either has a problem with vlan stripping or a different way of reporting it? Also, why does ASF require vlan stripping? -- 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 Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: > On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun > <lkml20101129@newton.leun.net> wrote: > > OK - all tests done on that DL320G5: > > > > For completeness, 2.6.37-rc5 unpatched: > > > > eth0, no vlan configured: totally broken - see double tagged vlans > > without tag, single or untagged packets missing at all > > Random behavior? This one is somewhat hard to explain - maybe there > are some other factors. eth0 has ASF on, so it always strips tags. I > would expect it to behave like the vlan configured case. > > > > > eth0, vlan configured: see packets without vlan tag (see double tagged > > packets with one vlan tag) > > Both ASF and vlan group configured cause tag stripping to be enabled. > Missing tag. > > > > > eth1 same as originally reported: > > without vlan configured see vlan tags (single and double tagged as > > expected) > > No ASF and no vlan group means tag stripping is disabled. Have tag. > > > with vlan configured: see packets without vlan tag (see double tagged > > packets with one vlan tag) > > Configuring vlan group causes stripping to be enabled. Missing tag. > > > > > > > 2.6.37-rc5, your tg3 use new vlan-code patch: > > > > eth0, no vlan configured: ?see packets without vlan tag (see double > > tagged packets with one vlan tag) > > ASF enables tag stripping. Missing tag. > > > eth1, no vlan configured: see vlan tags (single and double tagged as > > expected) > > No ASF, no vlan group means no stripping. Have tag. > > > > > > > eth0, vlan configured: as without vlan > > ASF enables stripping. Missing tag. > > > eth1, vlan configured: as without vlan > > With this patch vlan stripping is only enabled when ASF is on, so no > stripping. Have tag. > > > > > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > > > > eth1 no vlan configured: see packets without vlan tag (see double tagged > > packets with one vlan tag) > > With the second patch, vlan stripping is always enabled. Missing tag. > > > eth1 with vlan: the same > > Stripping still always enabled. Missing tag. > > The bottom line is whenever vlan stripping is enabled we're missing > the outer tag. It might be worth adding some debugging in the area > before napi_gro_receive/vlan_gro_receive (depending on version). My > guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for > vlan packets on this NIC. > > You said that everything works on the 5752? Matt, is it possible that > the 5714 either has a problem with vlan stripping or a different way > of reporting it? I don't think this is a 5714 specific issue. I think the problem is rooted in the fact that the VLAN tag stripping is enabled. Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. The patch here is using __vlan_hwaccel_put_tag(), which informs the stack a VLAN tag is present. If this is indeed a reporting problem, I'm not sure what else the driver should be doing. > Also, why does ASF require vlan stripping? This is a firmware limitation. -- 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 Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun >> <lkml20101129@newton.leun.net> wrote: >> > OK - all tests done on that DL320G5: >> > >> > For completeness, 2.6.37-rc5 unpatched: >> > >> > eth0, no vlan configured: totally broken - see double tagged vlans >> > without tag, single or untagged packets missing at all >> >> Random behavior? This one is somewhat hard to explain - maybe there >> are some other factors. eth0 has ASF on, so it always strips tags. I >> would expect it to behave like the vlan configured case. >> >> > >> > eth0, vlan configured: see packets without vlan tag (see double tagged >> > packets with one vlan tag) >> >> Both ASF and vlan group configured cause tag stripping to be enabled. >> Missing tag. >> >> > >> > eth1 same as originally reported: >> > without vlan configured see vlan tags (single and double tagged as >> > expected) >> >> No ASF and no vlan group means tag stripping is disabled. Have tag. >> >> > with vlan configured: see packets without vlan tag (see double tagged >> > packets with one vlan tag) >> >> Configuring vlan group causes stripping to be enabled. Missing tag. >> >> > >> > >> > 2.6.37-rc5, your tg3 use new vlan-code patch: >> > >> > eth0, no vlan configured: ?see packets without vlan tag (see double >> > tagged packets with one vlan tag) >> >> ASF enables tag stripping. Missing tag. >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as >> > expected) >> >> No ASF, no vlan group means no stripping. Have tag. >> >> > >> > >> > eth0, vlan configured: as without vlan >> >> ASF enables stripping. Missing tag. >> >> > eth1, vlan configured: as without vlan >> >> With this patch vlan stripping is only enabled when ASF is on, so no >> stripping. Have tag. >> >> > >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop >> > >> > eth1 no vlan configured: see packets without vlan tag (see double tagged >> > packets with one vlan tag) >> >> With the second patch, vlan stripping is always enabled. Missing tag. >> >> > eth1 with vlan: the same >> >> Stripping still always enabled. Missing tag. >> >> The bottom line is whenever vlan stripping is enabled we're missing >> the outer tag. It might be worth adding some debugging in the area >> before napi_gro_receive/vlan_gro_receive (depending on version). My >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for >> vlan packets on this NIC. >> >> You said that everything works on the 5752? Matt, is it possible that >> the 5714 either has a problem with vlan stripping or a different way >> of reporting it? > > I don't think this is a 5714 specific issue. I think the problem is > rooted in the fact that the VLAN tag stripping is enabled. It's definitely related to vlan stripping being enabled. Other cards using tg3 seem to work fine with stripping though, which is why I thought it might be specific to the 5714. > > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. > > The patch here is using __vlan_hwaccel_put_tag(), which informs the > stack a VLAN tag is present. If this is indeed a reporting problem, I'm > not sure what else the driver should be doing. The code to hand off the tag to the stack looks OK to me. Michael was seeing this on older versions of the kernel as well with this NIC, which predates both this patch and the larger vlan changes so it doesn't seem like a problem with passing the tag to the network stack. It's hard to know exactly what is going on though without seeing what the hardware is reporting. -- 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 Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: > On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: > >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun > >> <lkml20101129@newton.leun.net> wrote: > >> > OK - all tests done on that DL320G5: > >> > > >> > For completeness, 2.6.37-rc5 unpatched: > >> > > >> > eth0, no vlan configured: totally broken - see double tagged vlans > >> > without tag, single or untagged packets missing at all > >> > >> Random behavior? ?This one is somewhat hard to explain - maybe there > >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I > >> would expect it to behave like the vlan configured case. > >> > >> > > >> > eth0, vlan configured: see packets without vlan tag (see double tagged > >> > packets with one vlan tag) > >> > >> Both ASF and vlan group configured cause tag stripping to be enabled. > >> Missing tag. > >> > >> > > >> > eth1 same as originally reported: > >> > without vlan configured see vlan tags (single and double tagged as > >> > expected) > >> > >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. > >> > >> > with vlan configured: see packets without vlan tag (see double tagged > >> > packets with one vlan tag) > >> > >> Configuring vlan group causes stripping to be enabled. ?Missing tag. > >> > >> > > >> > > >> > 2.6.37-rc5, your tg3 use new vlan-code patch: > >> > > >> > eth0, no vlan configured: ?see packets without vlan tag (see double > >> > tagged packets with one vlan tag) > >> > >> ASF enables tag stripping. ?Missing tag. > >> > >> > eth1, no vlan configured: see vlan tags (single and double tagged as > >> > expected) > >> > >> No ASF, no vlan group means no stripping. ?Have tag. > >> > >> > > >> > > >> > eth0, vlan configured: as without vlan > >> > >> ASF enables stripping. ?Missing tag. > >> > >> > eth1, vlan configured: as without vlan > >> > >> With this patch vlan stripping is only enabled when ASF is on, so no > >> stripping. ?Have tag. > >> > >> > > >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > >> > > >> > eth1 no vlan configured: see packets without vlan tag (see double tagged > >> > packets with one vlan tag) > >> > >> With the second patch, vlan stripping is always enabled. ?Missing tag. > >> > >> > eth1 with vlan: the same > >> > >> Stripping still always enabled. ?Missing tag. > >> > >> The bottom line is whenever vlan stripping is enabled we're missing > >> the outer tag. ?It might be worth adding some debugging in the area > >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My > >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for > >> vlan packets on this NIC. > >> > >> You said that everything works on the 5752? ?Matt, is it possible that > >> the 5714 either has a problem with vlan stripping or a different way > >> of reporting it? > > > > I don't think this is a 5714 specific issue. ?I think the problem is > > rooted in the fact that the VLAN tag stripping is enabled. > > It's definitely related to vlan stripping being enabled. Other cards > using tg3 seem to work fine with stripping though, which is why I > thought it might be specific to the 5714. I just tested this on a 5714S, using a net-next-2.6 snapshot obtained today. It does the right thing in both cases (2nd tg3 patch ommited / applied). The tag is always visible in the packet stream as seen from tcpdump. > > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. > > > > The patch here is using __vlan_hwaccel_put_tag(), which informs the > > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm > > not sure what else the driver should be doing. > > The code to hand off the tag to the stack looks OK to me. Michael was > seeing this on older versions of the kernel as well with this NIC, > which predates both this patch and the larger vlan changes so it > doesn't seem like a problem with passing the tag to the network stack. > It's hard to know exactly what is going on though without seeing what > the hardware is reporting. When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set when receiving a packet. The driver skips the __vlan_hwaccel_put_tag() call. When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and __vlan_hwaccel_put_tag() is called to reinject the packet. -- 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, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun >> >> <lkml20101129@newton.leun.net> wrote: >> >> > OK - all tests done on that DL320G5: >> >> > >> >> > For completeness, 2.6.37-rc5 unpatched: >> >> > >> >> > eth0, no vlan configured: totally broken - see double tagged vlans >> >> > without tag, single or untagged packets missing at all >> >> >> >> Random behavior? ?This one is somewhat hard to explain - maybe there >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I >> >> would expect it to behave like the vlan configured case. >> >> >> >> > >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged >> >> > packets with one vlan tag) >> >> >> >> Both ASF and vlan group configured cause tag stripping to be enabled. >> >> Missing tag. >> >> >> >> > >> >> > eth1 same as originally reported: >> >> > without vlan configured see vlan tags (single and double tagged as >> >> > expected) >> >> >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. >> >> >> >> > with vlan configured: see packets without vlan tag (see double tagged >> >> > packets with one vlan tag) >> >> >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. >> >> >> >> > >> >> > >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: >> >> > >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double >> >> > tagged packets with one vlan tag) >> >> >> >> ASF enables tag stripping. ?Missing tag. >> >> >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as >> >> > expected) >> >> >> >> No ASF, no vlan group means no stripping. ?Have tag. >> >> >> >> > >> >> > >> >> > eth0, vlan configured: as without vlan >> >> >> >> ASF enables stripping. ?Missing tag. >> >> >> >> > eth1, vlan configured: as without vlan >> >> >> >> With this patch vlan stripping is only enabled when ASF is on, so no >> >> stripping. ?Have tag. >> >> >> >> > >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop >> >> > >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged >> >> > packets with one vlan tag) >> >> >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. >> >> >> >> > eth1 with vlan: the same >> >> >> >> Stripping still always enabled. ?Missing tag. >> >> >> >> The bottom line is whenever vlan stripping is enabled we're missing >> >> the outer tag. ?It might be worth adding some debugging in the area >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for >> >> vlan packets on this NIC. >> >> >> >> You said that everything works on the 5752? ?Matt, is it possible that >> >> the 5714 either has a problem with vlan stripping or a different way >> >> of reporting it? >> > >> > I don't think this is a 5714 specific issue. ?I think the problem is >> > rooted in the fact that the VLAN tag stripping is enabled. >> >> It's definitely related to vlan stripping being enabled. Other cards >> using tg3 seem to work fine with stripping though, which is why I >> thought it might be specific to the 5714. > > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained > today. It does the right thing in both cases (2nd tg3 patch ommited / > applied). The tag is always visible in the packet stream as seen from > tcpdump. > >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. >> > >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm >> > not sure what else the driver should be doing. >> >> The code to hand off the tag to the stack looks OK to me. Michael was >> seeing this on older versions of the kernel as well with this NIC, >> which predates both this patch and the larger vlan changes so it >> doesn't seem like a problem with passing the tag to the network stack. >> It's hard to know exactly what is going on though without seeing what >> the hardware is reporting. > > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set > when receiving a packet. The driver skips the __vlan_hwaccel_put_tag() > call. > > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and > __vlan_hwaccel_put_tag() is called to reinject the packet. OK, thanks for testing it out. I'm not sure that there's anything more we can do without hearing from Michael. -- 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 Thu, Jan 13, 2011 at 07:06:22AM -0800, Jesse Gross wrote: > On Wed, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: > >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: > >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun > >> >> <lkml20101129@newton.leun.net> wrote: > >> >> > OK - all tests done on that DL320G5: > >> >> > > >> >> > For completeness, 2.6.37-rc5 unpatched: > >> >> > > >> >> > eth0, no vlan configured: totally broken - see double tagged vlans > >> >> > without tag, single or untagged packets missing at all > >> >> > >> >> Random behavior? ?This one is somewhat hard to explain - maybe there > >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I > >> >> would expect it to behave like the vlan configured case. > >> >> > >> >> > > >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged > >> >> > packets with one vlan tag) > >> >> > >> >> Both ASF and vlan group configured cause tag stripping to be enabled. > >> >> Missing tag. > >> >> > >> >> > > >> >> > eth1 same as originally reported: > >> >> > without vlan configured see vlan tags (single and double tagged as > >> >> > expected) > >> >> > >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. > >> >> > >> >> > with vlan configured: see packets without vlan tag (see double tagged > >> >> > packets with one vlan tag) > >> >> > >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. > >> >> > >> >> > > >> >> > > >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: > >> >> > > >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double > >> >> > tagged packets with one vlan tag) > >> >> > >> >> ASF enables tag stripping. ?Missing tag. > >> >> > >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as > >> >> > expected) > >> >> > >> >> No ASF, no vlan group means no stripping. ?Have tag. > >> >> > >> >> > > >> >> > > >> >> > eth0, vlan configured: as without vlan > >> >> > >> >> ASF enables stripping. ?Missing tag. > >> >> > >> >> > eth1, vlan configured: as without vlan > >> >> > >> >> With this patch vlan stripping is only enabled when ASF is on, so no > >> >> stripping. ?Have tag. > >> >> > >> >> > > >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > >> >> > > >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged > >> >> > packets with one vlan tag) > >> >> > >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. > >> >> > >> >> > eth1 with vlan: the same > >> >> > >> >> Stripping still always enabled. ?Missing tag. > >> >> > >> >> The bottom line is whenever vlan stripping is enabled we're missing > >> >> the outer tag. ?It might be worth adding some debugging in the area > >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My > >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for > >> >> vlan packets on this NIC. > >> >> > >> >> You said that everything works on the 5752? ?Matt, is it possible that > >> >> the 5714 either has a problem with vlan stripping or a different way > >> >> of reporting it? > >> > > >> > I don't think this is a 5714 specific issue. ?I think the problem is > >> > rooted in the fact that the VLAN tag stripping is enabled. > >> > >> It's definitely related to vlan stripping being enabled. ?Other cards > >> using tg3 seem to work fine with stripping though, which is why I > >> thought it might be specific to the 5714. > > > > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained > > today. ?It does the right thing in both cases (2nd tg3 patch ommited / > > applied). ?The tag is always visible in the packet stream as seen from > > tcpdump. > > > >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. > >> > > >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the > >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm > >> > not sure what else the driver should be doing. > >> > >> The code to hand off the tag to the stack looks OK to me. ?Michael was > >> seeing this on older versions of the kernel as well with this NIC, > >> which predates both this patch and the larger vlan changes so it > >> doesn't seem like a problem with passing the tag to the network stack. > >> ?It's hard to know exactly what is going on though without seeing what > >> the hardware is reporting. > > > > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set > > when receiving a packet. ?The driver skips the __vlan_hwaccel_put_tag() > > call. > > > > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and > > __vlan_hwaccel_put_tag() is called to reinject the packet. > > OK, thanks for testing it out. I'm not sure that there's anything > more we can do without hearing from Michael. In the meantime, I think what we have should go upstream. Just to be absolutely clear though, your position is that VLAN tags should always be stripped? -- 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 Thu, Jan 13, 2011 at 3:50 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > On Thu, Jan 13, 2011 at 07:06:22AM -0800, Jesse Gross wrote: >> On Wed, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: >> >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: >> >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun >> >> >> <lkml20101129@newton.leun.net> wrote: >> >> >> > OK - all tests done on that DL320G5: >> >> >> > >> >> >> > For completeness, 2.6.37-rc5 unpatched: >> >> >> > >> >> >> > eth0, no vlan configured: totally broken - see double tagged vlans >> >> >> > without tag, single or untagged packets missing at all >> >> >> >> >> >> Random behavior? ?This one is somewhat hard to explain - maybe there >> >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I >> >> >> would expect it to behave like the vlan configured case. >> >> >> >> >> >> > >> >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged >> >> >> > packets with one vlan tag) >> >> >> >> >> >> Both ASF and vlan group configured cause tag stripping to be enabled. >> >> >> Missing tag. >> >> >> >> >> >> > >> >> >> > eth1 same as originally reported: >> >> >> > without vlan configured see vlan tags (single and double tagged as >> >> >> > expected) >> >> >> >> >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. >> >> >> >> >> >> > with vlan configured: see packets without vlan tag (see double tagged >> >> >> > packets with one vlan tag) >> >> >> >> >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. >> >> >> >> >> >> > >> >> >> > >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: >> >> >> > >> >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double >> >> >> > tagged packets with one vlan tag) >> >> >> >> >> >> ASF enables tag stripping. ?Missing tag. >> >> >> >> >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as >> >> >> > expected) >> >> >> >> >> >> No ASF, no vlan group means no stripping. ?Have tag. >> >> >> >> >> >> > >> >> >> > >> >> >> > eth0, vlan configured: as without vlan >> >> >> >> >> >> ASF enables stripping. ?Missing tag. >> >> >> >> >> >> > eth1, vlan configured: as without vlan >> >> >> >> >> >> With this patch vlan stripping is only enabled when ASF is on, so no >> >> >> stripping. ?Have tag. >> >> >> >> >> >> > >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop >> >> >> > >> >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged >> >> >> > packets with one vlan tag) >> >> >> >> >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. >> >> >> >> >> >> > eth1 with vlan: the same >> >> >> >> >> >> Stripping still always enabled. ?Missing tag. >> >> >> >> >> >> The bottom line is whenever vlan stripping is enabled we're missing >> >> >> the outer tag. ?It might be worth adding some debugging in the area >> >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My >> >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for >> >> >> vlan packets on this NIC. >> >> >> >> >> >> You said that everything works on the 5752? ?Matt, is it possible that >> >> >> the 5714 either has a problem with vlan stripping or a different way >> >> >> of reporting it? >> >> > >> >> > I don't think this is a 5714 specific issue. ?I think the problem is >> >> > rooted in the fact that the VLAN tag stripping is enabled. >> >> >> >> It's definitely related to vlan stripping being enabled. ?Other cards >> >> using tg3 seem to work fine with stripping though, which is why I >> >> thought it might be specific to the 5714. >> > >> > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained >> > today. ?It does the right thing in both cases (2nd tg3 patch ommited / >> > applied). ?The tag is always visible in the packet stream as seen from >> > tcpdump. >> > >> >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. >> >> > >> >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the >> >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm >> >> > not sure what else the driver should be doing. >> >> >> >> The code to hand off the tag to the stack looks OK to me. ?Michael was >> >> seeing this on older versions of the kernel as well with this NIC, >> >> which predates both this patch and the larger vlan changes so it >> >> doesn't seem like a problem with passing the tag to the network stack. >> >> ?It's hard to know exactly what is going on though without seeing what >> >> the hardware is reporting. >> > >> > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set >> > when receiving a packet. ?The driver skips the __vlan_hwaccel_put_tag() >> > call. >> > >> > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and >> > __vlan_hwaccel_put_tag() is called to reinject the packet. >> >> OK, thanks for testing it out. I'm not sure that there's anything >> more we can do without hearing from Michael. > > In the meantime, I think what we have should go upstream. Just to be > absolutely clear though, your position is that VLAN tags should always > be stripped? That's what the other converted drivers do by default (though most of them also provide an ethtool set_flags() method to change this). It's generally the most efficient and is now safe to do in all cases. It's also the consistent with what was happening before, since stripping was enabled when a vlan device was configured. So, yes, normally I think stripping should be enabled. I assumed that disabling stripping in most situations was just an oversight. Was there a reason why you feel it is better not to use it? -- 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 Thu, Jan 13, 2011 at 01:58:40PM -0800, Jesse Gross wrote: > On Thu, Jan 13, 2011 at 3:50 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > > On Thu, Jan 13, 2011 at 07:06:22AM -0800, Jesse Gross wrote: > >> On Wed, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: > >> >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: > >> >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun > >> >> >> <lkml20101129@newton.leun.net> wrote: > >> >> >> > OK - all tests done on that DL320G5: > >> >> >> > > >> >> >> > For completeness, 2.6.37-rc5 unpatched: > >> >> >> > > >> >> >> > eth0, no vlan configured: totally broken - see double tagged vlans > >> >> >> > without tag, single or untagged packets missing at all > >> >> >> > >> >> >> Random behavior? ?This one is somewhat hard to explain - maybe there > >> >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I > >> >> >> would expect it to behave like the vlan configured case. > >> >> >> > >> >> >> > > >> >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged > >> >> >> > packets with one vlan tag) > >> >> >> > >> >> >> Both ASF and vlan group configured cause tag stripping to be enabled. > >> >> >> Missing tag. > >> >> >> > >> >> >> > > >> >> >> > eth1 same as originally reported: > >> >> >> > without vlan configured see vlan tags (single and double tagged as > >> >> >> > expected) > >> >> >> > >> >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. > >> >> >> > >> >> >> > with vlan configured: see packets without vlan tag (see double tagged > >> >> >> > packets with one vlan tag) > >> >> >> > >> >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. > >> >> >> > >> >> >> > > >> >> >> > > >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: > >> >> >> > > >> >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double > >> >> >> > tagged packets with one vlan tag) > >> >> >> > >> >> >> ASF enables tag stripping. ?Missing tag. > >> >> >> > >> >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as > >> >> >> > expected) > >> >> >> > >> >> >> No ASF, no vlan group means no stripping. ?Have tag. > >> >> >> > >> >> >> > > >> >> >> > > >> >> >> > eth0, vlan configured: as without vlan > >> >> >> > >> >> >> ASF enables stripping. ?Missing tag. > >> >> >> > >> >> >> > eth1, vlan configured: as without vlan > >> >> >> > >> >> >> With this patch vlan stripping is only enabled when ASF is on, so no > >> >> >> stripping. ?Have tag. > >> >> >> > >> >> >> > > >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > >> >> >> > > >> >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged > >> >> >> > packets with one vlan tag) > >> >> >> > >> >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. > >> >> >> > >> >> >> > eth1 with vlan: the same > >> >> >> > >> >> >> Stripping still always enabled. ?Missing tag. > >> >> >> > >> >> >> The bottom line is whenever vlan stripping is enabled we're missing > >> >> >> the outer tag. ?It might be worth adding some debugging in the area > >> >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My > >> >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for > >> >> >> vlan packets on this NIC. > >> >> >> > >> >> >> You said that everything works on the 5752? ?Matt, is it possible that > >> >> >> the 5714 either has a problem with vlan stripping or a different way > >> >> >> of reporting it? > >> >> > > >> >> > I don't think this is a 5714 specific issue. ?I think the problem is > >> >> > rooted in the fact that the VLAN tag stripping is enabled. > >> >> > >> >> It's definitely related to vlan stripping being enabled. ?Other cards > >> >> using tg3 seem to work fine with stripping though, which is why I > >> >> thought it might be specific to the 5714. > >> > > >> > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained > >> > today. ?It does the right thing in both cases (2nd tg3 patch ommited / > >> > applied). ?The tag is always visible in the packet stream as seen from > >> > tcpdump. > >> > > >> >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. > >> >> > > >> >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the > >> >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm > >> >> > not sure what else the driver should be doing. > >> >> > >> >> The code to hand off the tag to the stack looks OK to me. ?Michael was > >> >> seeing this on older versions of the kernel as well with this NIC, > >> >> which predates both this patch and the larger vlan changes so it > >> >> doesn't seem like a problem with passing the tag to the network stack. > >> >> ?It's hard to know exactly what is going on though without seeing what > >> >> the hardware is reporting. > >> > > >> > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set > >> > when receiving a packet. ?The driver skips the __vlan_hwaccel_put_tag() > >> > call. > >> > > >> > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and > >> > __vlan_hwaccel_put_tag() is called to reinject the packet. > >> > >> OK, thanks for testing it out. ?I'm not sure that there's anything > >> more we can do without hearing from Michael. > > > > In the meantime, I think what we have should go upstream. ?Just to be > > absolutely clear though, your position is that VLAN tags should always > > be stripped? > > That's what the other converted drivers do by default (though most of > them also provide an ethtool set_flags() method to change this). It's > generally the most efficient and is now safe to do in all cases. It's > also the consistent with what was happening before, since stripping > was enabled when a vlan device was configured. So, yes, normally I > think stripping should be enabled. > > I assumed that disabling stripping in most situations was just an > oversight. Was there a reason why you feel it is better not to use > it? Actually, the tg3 driver was trying to disable VLAN tag stripping when possible. I believe this was primarily to support the raw packet interface. -- 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 Thu, Jan 13, 2011 at 8:15 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > On Thu, Jan 13, 2011 at 01:58:40PM -0800, Jesse Gross wrote: >> On Thu, Jan 13, 2011 at 3:50 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> > On Thu, Jan 13, 2011 at 07:06:22AM -0800, Jesse Gross wrote: >> >> On Wed, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> >> > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: >> >> >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: >> >> >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: >> >> >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun >> >> >> >> <lkml20101129@newton.leun.net> wrote: >> >> >> >> > OK - all tests done on that DL320G5: >> >> >> >> > >> >> >> >> > For completeness, 2.6.37-rc5 unpatched: >> >> >> >> > >> >> >> >> > eth0, no vlan configured: totally broken - see double tagged vlans >> >> >> >> > without tag, single or untagged packets missing at all >> >> >> >> >> >> >> >> Random behavior? ?This one is somewhat hard to explain - maybe there >> >> >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I >> >> >> >> would expect it to behave like the vlan configured case. >> >> >> >> >> >> >> >> > >> >> >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged >> >> >> >> > packets with one vlan tag) >> >> >> >> >> >> >> >> Both ASF and vlan group configured cause tag stripping to be enabled. >> >> >> >> Missing tag. >> >> >> >> >> >> >> >> > >> >> >> >> > eth1 same as originally reported: >> >> >> >> > without vlan configured see vlan tags (single and double tagged as >> >> >> >> > expected) >> >> >> >> >> >> >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. >> >> >> >> >> >> >> >> > with vlan configured: see packets without vlan tag (see double tagged >> >> >> >> > packets with one vlan tag) >> >> >> >> >> >> >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. >> >> >> >> >> >> >> >> > >> >> >> >> > >> >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: >> >> >> >> > >> >> >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double >> >> >> >> > tagged packets with one vlan tag) >> >> >> >> >> >> >> >> ASF enables tag stripping. ?Missing tag. >> >> >> >> >> >> >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as >> >> >> >> > expected) >> >> >> >> >> >> >> >> No ASF, no vlan group means no stripping. ?Have tag. >> >> >> >> >> >> >> >> > >> >> >> >> > >> >> >> >> > eth0, vlan configured: as without vlan >> >> >> >> >> >> >> >> ASF enables stripping. ?Missing tag. >> >> >> >> >> >> >> >> > eth1, vlan configured: as without vlan >> >> >> >> >> >> >> >> With this patch vlan stripping is only enabled when ASF is on, so no >> >> >> >> stripping. ?Have tag. >> >> >> >> >> >> >> >> > >> >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop >> >> >> >> > >> >> >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged >> >> >> >> > packets with one vlan tag) >> >> >> >> >> >> >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. >> >> >> >> >> >> >> >> > eth1 with vlan: the same >> >> >> >> >> >> >> >> Stripping still always enabled. ?Missing tag. >> >> >> >> >> >> >> >> The bottom line is whenever vlan stripping is enabled we're missing >> >> >> >> the outer tag. ?It might be worth adding some debugging in the area >> >> >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My >> >> >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for >> >> >> >> vlan packets on this NIC. >> >> >> >> >> >> >> >> You said that everything works on the 5752? ?Matt, is it possible that >> >> >> >> the 5714 either has a problem with vlan stripping or a different way >> >> >> >> of reporting it? >> >> >> > >> >> >> > I don't think this is a 5714 specific issue. ?I think the problem is >> >> >> > rooted in the fact that the VLAN tag stripping is enabled. >> >> >> >> >> >> It's definitely related to vlan stripping being enabled. ?Other cards >> >> >> using tg3 seem to work fine with stripping though, which is why I >> >> >> thought it might be specific to the 5714. >> >> > >> >> > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained >> >> > today. ?It does the right thing in both cases (2nd tg3 patch ommited / >> >> > applied). ?The tag is always visible in the packet stream as seen from >> >> > tcpdump. >> >> > >> >> >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. >> >> >> > >> >> >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the >> >> >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm >> >> >> > not sure what else the driver should be doing. >> >> >> >> >> >> The code to hand off the tag to the stack looks OK to me. ?Michael was >> >> >> seeing this on older versions of the kernel as well with this NIC, >> >> >> which predates both this patch and the larger vlan changes so it >> >> >> doesn't seem like a problem with passing the tag to the network stack. >> >> >> ?It's hard to know exactly what is going on though without seeing what >> >> >> the hardware is reporting. >> >> > >> >> > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set >> >> > when receiving a packet. ?The driver skips the __vlan_hwaccel_put_tag() >> >> > call. >> >> > >> >> > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and >> >> > __vlan_hwaccel_put_tag() is called to reinject the packet. >> >> >> >> OK, thanks for testing it out. ?I'm not sure that there's anything >> >> more we can do without hearing from Michael. >> > >> > In the meantime, I think what we have should go upstream. ?Just to be >> > absolutely clear though, your position is that VLAN tags should always >> > be stripped? >> >> That's what the other converted drivers do by default (though most of >> them also provide an ethtool set_flags() method to change this). It's >> generally the most efficient and is now safe to do in all cases. It's >> also the consistent with what was happening before, since stripping >> was enabled when a vlan device was configured. So, yes, normally I >> think stripping should be enabled. >> >> I assumed that disabling stripping in most situations was just an >> oversight. Was there a reason why you feel it is better not to use >> it? > > Actually, the tg3 driver was trying to disable VLAN tag stripping > when possible. I believe this was primarily to support the raw packet > interface. Hmm, is this really true? if (!tp->vlgrp && !(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) rx_mode |= RX_MODE_KEEP_VLAN_TAG; So if a vlan device is registered or ASF is enabled we will use stripping. That seems like it is using stripping in the common case when vlans are used. Before 2.6.37 it was only possible to deliver stripped tags if a vlan group was configured. This means that if ASF was enabled and forced stripping but no group was configured we would lose the tag. In this situation tg3 manually reinserts the tags so raw packet capture will see them, as you say. However, in the current tree this limitation no longer exists and it is always possible to deliver stripped tags to the networking core, which should do the right thing in all situations. So I believe the only reason to disable tag stripping is for debugging or some other special purpose situation. -- 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, Jan 14, 2011 at 09:49:47AM -0800, Jesse Gross wrote: > On Thu, Jan 13, 2011 at 8:15 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > > On Thu, Jan 13, 2011 at 01:58:40PM -0800, Jesse Gross wrote: > >> On Thu, Jan 13, 2011 at 3:50 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> > On Thu, Jan 13, 2011 at 07:06:22AM -0800, Jesse Gross wrote: > >> >> On Wed, Jan 12, 2011 at 8:21 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> >> > On Thu, Jan 06, 2011 at 08:36:27PM -0800, Jesse Gross wrote: > >> >> >> On Thu, Jan 6, 2011 at 10:24 PM, Matt Carlson <mcarlson@broadcom.com> wrote: > >> >> >> > On Sat, Dec 18, 2010 at 07:38:00PM -0800, Jesse Gross wrote: > >> >> >> >> On Tue, Dec 14, 2010 at 11:16 PM, Michael Leun > >> >> >> >> <lkml20101129@newton.leun.net> wrote: > >> >> >> >> > OK - all tests done on that DL320G5: > >> >> >> >> > > >> >> >> >> > For completeness, 2.6.37-rc5 unpatched: > >> >> >> >> > > >> >> >> >> > eth0, no vlan configured: totally broken - see double tagged vlans > >> >> >> >> > without tag, single or untagged packets missing at all > >> >> >> >> > >> >> >> >> Random behavior? ?This one is somewhat hard to explain - maybe there > >> >> >> >> are some other factors. ?eth0 has ASF on, so it always strips tags. ?I > >> >> >> >> would expect it to behave like the vlan configured case. > >> >> >> >> > >> >> >> >> > > >> >> >> >> > eth0, vlan configured: see packets without vlan tag (see double tagged > >> >> >> >> > packets with one vlan tag) > >> >> >> >> > >> >> >> >> Both ASF and vlan group configured cause tag stripping to be enabled. > >> >> >> >> Missing tag. > >> >> >> >> > >> >> >> >> > > >> >> >> >> > eth1 same as originally reported: > >> >> >> >> > without vlan configured see vlan tags (single and double tagged as > >> >> >> >> > expected) > >> >> >> >> > >> >> >> >> No ASF and no vlan group means tag stripping is disabled. ?Have tag. > >> >> >> >> > >> >> >> >> > with vlan configured: see packets without vlan tag (see double tagged > >> >> >> >> > packets with one vlan tag) > >> >> >> >> > >> >> >> >> Configuring vlan group causes stripping to be enabled. ?Missing tag. > >> >> >> >> > >> >> >> >> > > >> >> >> >> > > >> >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch: > >> >> >> >> > > >> >> >> >> > eth0, no vlan configured: ?see packets without vlan tag (see double > >> >> >> >> > tagged packets with one vlan tag) > >> >> >> >> > >> >> >> >> ASF enables tag stripping. ?Missing tag. > >> >> >> >> > >> >> >> >> > eth1, no vlan configured: see vlan tags (single and double tagged as > >> >> >> >> > expected) > >> >> >> >> > >> >> >> >> No ASF, no vlan group means no stripping. ?Have tag. > >> >> >> >> > >> >> >> >> > > >> >> >> >> > > >> >> >> >> > eth0, vlan configured: as without vlan > >> >> >> >> > >> >> >> >> ASF enables stripping. ?Missing tag. > >> >> >> >> > >> >> >> >> > eth1, vlan configured: as without vlan > >> >> >> >> > >> >> >> >> With this patch vlan stripping is only enabled when ASF is on, so no > >> >> >> >> stripping. ?Have tag. > >> >> >> >> > >> >> >> >> > > >> >> >> >> > 2.6.37-rc5, your tg3 use new vlan-code patch with test patch ontop > >> >> >> >> > > >> >> >> >> > eth1 no vlan configured: see packets without vlan tag (see double tagged > >> >> >> >> > packets with one vlan tag) > >> >> >> >> > >> >> >> >> With the second patch, vlan stripping is always enabled. ?Missing tag. > >> >> >> >> > >> >> >> >> > eth1 with vlan: the same > >> >> >> >> > >> >> >> >> Stripping still always enabled. ?Missing tag. > >> >> >> >> > >> >> >> >> The bottom line is whenever vlan stripping is enabled we're missing > >> >> >> >> the outer tag. ?It might be worth adding some debugging in the area > >> >> >> >> before napi_gro_receive/vlan_gro_receive (depending on version). ?My > >> >> >> >> guess is that (desc->type_flags & RXD_FLAG_VLAN) is false even for > >> >> >> >> vlan packets on this NIC. > >> >> >> >> > >> >> >> >> You said that everything works on the 5752? ?Matt, is it possible that > >> >> >> >> the 5714 either has a problem with vlan stripping or a different way > >> >> >> >> of reporting it? > >> >> >> > > >> >> >> > I don't think this is a 5714 specific issue. ?I think the problem is > >> >> >> > rooted in the fact that the VLAN tag stripping is enabled. > >> >> >> > >> >> >> It's definitely related to vlan stripping being enabled. ?Other cards > >> >> >> using tg3 seem to work fine with stripping though, which is why I > >> >> >> thought it might be specific to the 5714. > >> >> > > >> >> > I just tested this on a 5714S, using a net-next-2.6 snapshot obtained > >> >> > today. ?It does the right thing in both cases (2nd tg3 patch ommited / > >> >> > applied). ?The tag is always visible in the packet stream as seen from > >> >> > tcpdump. > >> >> > > >> >> >> > Your RXD_FLAG_VLAN idea sounds unlikely to me, but it's worth a check. > >> >> >> > > >> >> >> > The patch here is using __vlan_hwaccel_put_tag(), which informs the > >> >> >> > stack a VLAN tag is present. ?If this is indeed a reporting problem, I'm > >> >> >> > not sure what else the driver should be doing. > >> >> >> > >> >> >> The code to hand off the tag to the stack looks OK to me. ?Michael was > >> >> >> seeing this on older versions of the kernel as well with this NIC, > >> >> >> which predates both this patch and the larger vlan changes so it > >> >> >> doesn't seem like a problem with passing the tag to the network stack. > >> >> >> ?It's hard to know exactly what is going on though without seeing what > >> >> >> the hardware is reporting. > >> >> > > >> >> > When RX_MODE_KEEP_VLAN_TAG is set, the RXD_FLAG_VLAN flag will not be set > >> >> > when receiving a packet. ?The driver skips the __vlan_hwaccel_put_tag() > >> >> > call. > >> >> > > >> >> > When RX_MODE_KEEP_VLAN_TAG is unset, the RXD_FLAG_VLAN flag is set, and > >> >> > __vlan_hwaccel_put_tag() is called to reinject the packet. > >> >> > >> >> OK, thanks for testing it out. ?I'm not sure that there's anything > >> >> more we can do without hearing from Michael. > >> > > >> > In the meantime, I think what we have should go upstream. ?Just to be > >> > absolutely clear though, your position is that VLAN tags should always > >> > be stripped? > >> > >> That's what the other converted drivers do by default (though most of > >> them also provide an ethtool set_flags() method to change this). ?It's > >> generally the most efficient and is now safe to do in all cases. ?It's > >> also the consistent with what was happening before, since stripping > >> was enabled when a vlan device was configured. ?So, yes, normally I > >> think stripping should be enabled. > >> > >> I assumed that disabling stripping in most situations was just an > >> oversight. ?Was there a reason why you feel it is better not to use > >> it? > > > > Actually, the tg3 driver was trying to disable VLAN tag stripping > > when possible. ?I believe this was primarily to support the raw packet > > interface. > > Hmm, is this really true? > > if (!tp->vlgrp && > !(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) > rx_mode |= RX_MODE_KEEP_VLAN_TAG; > > So if a vlan device is registered or ASF is enabled we will use > stripping. That seems like it is using stripping in the common case > when vlans are used. Right. > Before 2.6.37 it was only possible to deliver stripped tags if a vlan > group was configured. This means that if ASF was enabled and forced > stripping but no group was configured we would lose the tag. In this > situation tg3 manually reinserts the tags so raw packet capture will > see them, as you say. Right. VLAN tagged frames can still arrive if CONFIG_VLAN_8021Q or CONFIG_VLAN_8021Q_MODULE is not set. That's why the driver was trying to keep them inline. To eliminate the unnecessary overhead of reinjecting the VLAN tag. > However, in the current tree this limitation no > longer exists and it is always possible to deliver stripped tags to > the networking core, which should do the right thing in all > situations. Yes. Even though the stack is capable of reinjecting the VLAN tag, doesn't it make sense to avoid that if we knew they would never be needed out-of-packet? > So I believe the only reason to disable tag stripping is for debugging > or some other special purpose situation. Nope. I think we covered it all. Thanks for the info. -- 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, Jan 14, 2011 at 10:38 AM, Matt Carlson <mcarlson@broadcom.com> wrote: >> >> > In the meantime, I think what we have should go upstream. ?Just to be >> >> > absolutely clear though, your position is that VLAN tags should always >> >> > be stripped? >> >> >> >> That's what the other converted drivers do by default (though most of >> >> them also provide an ethtool set_flags() method to change this). ?It's >> >> generally the most efficient and is now safe to do in all cases. ?It's >> >> also the consistent with what was happening before, since stripping >> >> was enabled when a vlan device was configured. ?So, yes, normally I >> >> think stripping should be enabled. >> >> >> >> I assumed that disabling stripping in most situations was just an >> >> oversight. ?Was there a reason why you feel it is better not to use >> >> it? >> > >> > Actually, the tg3 driver was trying to disable VLAN tag stripping >> > when possible. ?I believe this was primarily to support the raw packet >> > interface. >> >> Hmm, is this really true? >> >> if (!tp->vlgrp && >> !(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) >> rx_mode |= RX_MODE_KEEP_VLAN_TAG; >> >> So if a vlan device is registered or ASF is enabled we will use >> stripping. That seems like it is using stripping in the common case >> when vlans are used. > > Right. > >> Before 2.6.37 it was only possible to deliver stripped tags if a vlan >> group was configured. This means that if ASF was enabled and forced >> stripping but no group was configured we would lose the tag. In this >> situation tg3 manually reinserts the tags so raw packet capture will >> see them, as you say. > > Right. VLAN tagged frames can still arrive if CONFIG_VLAN_8021Q or > CONFIG_VLAN_8021Q_MODULE is not set. That's why the driver was trying > to keep them inline. To eliminate the unnecessary overhead of > reinjecting the VLAN tag. OK, I understand now why you are saying that the driver was trying to keep stripping disabled. I was thinking about having vlan groups configured as the common case for receiving vlans and therefore the case to optimize. > >> However, in the current tree this limitation no >> longer exists and it is always possible to deliver stripped tags to >> the networking core, which should do the right thing in all >> situations. > > Yes. Even though the stack is capable of reinjecting the VLAN tag, > doesn't it make sense to avoid that if we knew they would never be > needed out-of-packet? Maybe in theory but a relatively small optimization to the raw packet interface doesn't seem terribly important to me. After all of the drivers have been converted to the new vlan interfaces I'll be removing the ndo_vlan_rx_register() function from net_device_ops, so the driver won't actually know how the vlan tag will be used. The impetus for this is that there were quite a few bugs and inconsistent behavior in drivers around stripping logic. Therefore, by moving all of this into the networking core we can reduce code and get consistent behavior, which is more important than a corner case optimization. -- 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/drivers/net/tg3.c b/drivers/net/tg3.c index 3682205..964293f 100644 --- a/drivers/net/tg3.c +++ b/drivers/net/tg3.c @@ -9505,8 +9505,10 @@ static void __tg3_set_rx_mode(struct net_device *dev) /* When ASF is in use, we always keep the RX_MODE_KEEP_VLAN_TAG * flag clear. */ +#if 0 if (!(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) rx_mode |= RX_MODE_KEEP_VLAN_TAG; +#endif if (dev->flags & IFF_PROMISC) { /* Promiscuous mode. */