* [patch net-next-2.6] forcedeth: fix vlans
@ 2011-07-26 16:41 Jiri Pirko
2011-07-26 17:45 ` Michał Mirosław
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Pirko @ 2011-07-26 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, johnstul, w41ter, mirqus
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;
__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",
--
1.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [patch net-next-2.6] forcedeth: fix vlans
2011-07-26 16:41 [patch net-next-2.6] forcedeth: fix vlans Jiri Pirko
@ 2011-07-26 17:45 ` Michał Mirosław
2011-07-26 17:49 ` Jesse Gross
0 siblings, 1 reply; 8+ messages in thread
From: Michał Mirosław @ 2011-07-26 17:45 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, johnstul, w41ter
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch net-next-2.6] forcedeth: fix vlans
2011-07-26 17:45 ` Michał Mirosław
@ 2011-07-26 17:49 ` Jesse Gross
2011-07-26 17:55 ` Michał Mirosław
0 siblings, 1 reply; 8+ messages in thread
From: Jesse Gross @ 2011-07-26 17:49 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Jiri Pirko, netdev, davem, johnstul, w41ter
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.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch net-next-2.6] forcedeth: fix vlans
2011-07-26 17:49 ` Jesse Gross
@ 2011-07-26 17:55 ` Michał Mirosław
2011-07-26 20:19 ` [patch net-next-2.6 V2] " Jiri Pirko
0 siblings, 1 reply; 8+ messages in thread
From: Michał Mirosław @ 2011-07-26 17:55 UTC (permalink / raw)
To: Jesse Gross; +Cc: Jiri Pirko, netdev, davem, johnstul, w41ter
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [patch net-next-2.6 V2] forcedeth: fix vlans
2011-07-26 17:55 ` Michał Mirosław
@ 2011-07-26 20:19 ` Jiri Pirko
2011-07-26 22:52 ` walt
2011-07-28 5:41 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Jiri Pirko @ 2011-07-26 20:19 UTC (permalink / raw)
To: Michał Mirosław; +Cc: Jesse Gross, netdev, davem, johnstul, w41ter
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 | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index e64cd9c..e55df30 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -2764,7 +2764,14 @@ 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) {
+
+ /*
+ * There's need to check for NETIF_F_HW_VLAN_RX here.
+ * Even if vlan rx accel is disabled,
+ * NV_RX3_VLAN_TAG_PRESENT is pseudo randomly set.
+ */
+ 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);
@@ -5331,15 +5338,16 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK;
dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_SG |
NETIF_F_TSO | NETIF_F_RXCSUM;
- dev->features |= dev->hw_features;
}
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;
if ((id->driver_data & DEV_HAS_PAUSEFRAME_TX_V1) ||
(id->driver_data & DEV_HAS_PAUSEFRAME_TX_V2) ||
@@ -5607,6 +5615,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",
--
1.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [patch net-next-2.6 V2] forcedeth: fix vlans
2011-07-26 20:19 ` [patch net-next-2.6 V2] " Jiri Pirko
@ 2011-07-26 22:52 ` walt
2011-07-26 23:05 ` David Miller
2011-07-28 5:41 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: walt @ 2011-07-26 22:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: Michał Mirosław, Jesse Gross, netdev, davem, johnstul
Hi Jiri. Is this patch an addition to the first one, or a replacement for it?
Thanks for the fast work.
On 07/26/2011 01:19 PM, Jiri Pirko wrote:
> 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 | 16 +++++++++++++---
> 1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
> index e64cd9c..e55df30 100644
> --- a/drivers/net/forcedeth.c
> +++ b/drivers/net/forcedeth.c
> @@ -2764,7 +2764,14 @@ 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) {
> +
> + /*
> + * There's need to check for NETIF_F_HW_VLAN_RX here.
> + * Even if vlan rx accel is disabled,
> + * NV_RX3_VLAN_TAG_PRESENT is pseudo randomly set.
> + */
> + 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);
> @@ -5331,15 +5338,16 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
> np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK;
> dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_SG |
> NETIF_F_TSO | NETIF_F_RXCSUM;
> - dev->features |= dev->hw_features;
> }
>
> 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;
> if ((id->driver_data & DEV_HAS_PAUSEFRAME_TX_V1) ||
> (id->driver_data & DEV_HAS_PAUSEFRAME_TX_V2) ||
> @@ -5607,6 +5615,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",
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch net-next-2.6 V2] forcedeth: fix vlans
2011-07-26 20:19 ` [patch net-next-2.6 V2] " Jiri Pirko
2011-07-26 22:52 ` walt
@ 2011-07-28 5:41 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2011-07-28 5:41 UTC (permalink / raw)
To: jpirko; +Cc: mirqus, jesse, netdev, johnstul, w41ter
From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 26 Jul 2011 22:19:28 +0200
> 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>
Applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-07-28 5:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-26 16:41 [patch net-next-2.6] forcedeth: fix vlans Jiri Pirko
2011-07-26 17:45 ` Michał Mirosław
2011-07-26 17:49 ` Jesse Gross
2011-07-26 17:55 ` Michał Mirosław
2011-07-26 20:19 ` [patch net-next-2.6 V2] " Jiri Pirko
2011-07-26 22:52 ` walt
2011-07-26 23:05 ` David Miller
2011-07-28 5:41 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).