| Message ID | 1311698510-2599-1-git-send-email-jpirko@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
2011/7/26 Jiri Pirko <jpirko@redhat.com>: > For some reason, when rxaccel is disabled, NV_RX3_VLAN_TAG_PRESENT is > still set and some pseudorandom vids appear. So check for > NETIF_F_HW_VLAN_RX as well. Also set correctly hw_features and set vlan > mode on probe. > > Signed-off-by: Jiri Pirko <jpirko@redhat.com> > --- > drivers/net/forcedeth.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c > index e64cd9c..256a272 100644 > --- a/drivers/net/forcedeth.c > +++ b/drivers/net/forcedeth.c > @@ -2764,7 +2764,8 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit) > prefetch(skb->data); > > vlanflags = le32_to_cpu(np->get_rx.ex->buflow); > - if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) { > + if (dev->features & NETIF_F_HW_VLAN_RX && > + vlanflags & NV_RX3_VLAN_TAG_PRESENT) { > u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK; Please add comment that resembles this patch's description. This is a bad idea to do in the general case as this will cause VLAN tagged packets that were in the queue before feature change to be misinterpreted. > > __vlan_hwaccel_put_tag(skb, vid); > @@ -5337,7 +5338,8 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i > np->vlanctl_bits = 0; > if (id->driver_data & DEV_HAS_VLAN) { > np->vlanctl_bits = NVREG_VLANCONTROL_ENABLE; > - dev->features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX; > + dev->hw_features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX; > + dev->features |= dev->hw_features; > } For better readability, hw_features -> features copy should be done only once in this function. Best Regards, Michał Mirosław -- 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
2011/7/26 Michał Mirosław <mirqus@gmail.com>: > 2011/7/26 Jiri Pirko <jpirko@redhat.com>: >> For some reason, when rxaccel is disabled, NV_RX3_VLAN_TAG_PRESENT is >> still set and some pseudorandom vids appear. So check for >> NETIF_F_HW_VLAN_RX as well. Also set correctly hw_features and set vlan >> mode on probe. >> >> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >> --- >> drivers/net/forcedeth.c | 8 ++++++-- >> 1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c >> index e64cd9c..256a272 100644 >> --- a/drivers/net/forcedeth.c >> +++ b/drivers/net/forcedeth.c >> @@ -2764,7 +2764,8 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit) >> prefetch(skb->data); >> >> vlanflags = le32_to_cpu(np->get_rx.ex->buflow); >> - if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) { >> + if (dev->features & NETIF_F_HW_VLAN_RX && >> + vlanflags & NV_RX3_VLAN_TAG_PRESENT) { >> u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK; > > Please add comment that resembles this patch's description. This is a > bad idea to do in the general case as this will cause VLAN tagged > packets that were in the queue before feature change to be > misinterpreted. It may be a bad idea but it's not that uncommon - frequently those vlan status fields come from the parser and merely indicate the presence of a tag in the original packet, not whether it was 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
W dniu 26 lipca 2011 19:49 użytkownik Jesse Gross <jesse@nicira.com> napisał: > 2011/7/26 Michał Mirosław <mirqus@gmail.com>: >> 2011/7/26 Jiri Pirko <jpirko@redhat.com>: >>> For some reason, when rxaccel is disabled, NV_RX3_VLAN_TAG_PRESENT is >>> still set and some pseudorandom vids appear. So check for >>> NETIF_F_HW_VLAN_RX as well. Also set correctly hw_features and set vlan >>> mode on probe. >>> >>> Signed-off-by: Jiri Pirko <jpirko@redhat.com> >>> --- >>> drivers/net/forcedeth.c | 8 ++++++-- >>> 1 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c >>> index e64cd9c..256a272 100644 >>> --- a/drivers/net/forcedeth.c >>> +++ b/drivers/net/forcedeth.c >>> @@ -2764,7 +2764,8 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit) >>> prefetch(skb->data); >>> >>> vlanflags = le32_to_cpu(np->get_rx.ex->buflow); >>> - if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) { >>> + if (dev->features & NETIF_F_HW_VLAN_RX && >>> + vlanflags & NV_RX3_VLAN_TAG_PRESENT) { >>> u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK; >> Please add comment that resembles this patch's description. This is a >> bad idea to do in the general case as this will cause VLAN tagged >> packets that were in the queue before feature change to be >> misinterpreted. > It may be a bad idea but it's not that uncommon - frequently those > vlan status fields come from the parser and merely indicate the > presence of a tag in the original packet, not whether it was stripped. That doesn't make this code applicable for general use. If it's not commented properly people will copy it without thinking if they really need the workaround or not. Best Regards, Michał Mirosław -- 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/forcedeth.c b/drivers/net/forcedeth.c index e64cd9c..256a272 100644 --- a/drivers/net/forcedeth.c +++ b/drivers/net/forcedeth.c @@ -2764,7 +2764,8 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit) prefetch(skb->data); vlanflags = le32_to_cpu(np->get_rx.ex->buflow); - if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) { + if (dev->features & NETIF_F_HW_VLAN_RX && + vlanflags & NV_RX3_VLAN_TAG_PRESENT) { u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK; __vlan_hwaccel_put_tag(skb, vid); @@ -5337,7 +5338,8 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i np->vlanctl_bits = 0; if (id->driver_data & DEV_HAS_VLAN) { np->vlanctl_bits = NVREG_VLANCONTROL_ENABLE; - dev->features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX; + dev->hw_features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX; + dev->features |= dev->hw_features; } np->pause_flags = NV_PAUSEFRAME_RX_CAPABLE | NV_PAUSEFRAME_RX_REQ | NV_PAUSEFRAME_AUTONEG; @@ -5607,6 +5609,8 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i goto out_error; } + nv_vlan_mode(dev, dev->features); + netif_carrier_off(dev); dev_info(&pci_dev->dev, "ifname %s, PHY OUI 0x%x @ %d, addr %pM\n",
For some reason, when rxaccel is disabled, NV_RX3_VLAN_TAG_PRESENT is still set and some pseudorandom vids appear. So check for NETIF_F_HW_VLAN_RX as well. Also set correctly hw_features and set vlan mode on probe. Signed-off-by: Jiri Pirko <jpirko@redhat.com> --- drivers/net/forcedeth.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)