netdev - Re: [patch net-next-2.6] forcedeth: fix vlans

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHXqBF+qjCzfQmDLdKvanpBuQo1X-zFAr_26SCwVuwbSVwpyPA@mail.gmail.com>
Date:	Tue, 26 Jul 2011 19:55:06 +0200
From:	Michał Mirosław <mirqus@...il.com>
To:	Jesse Gross <jesse@...ira.com>
Cc:	Jiri Pirko <jpirko@...hat.com>, netdev@...r.kernel.org,
	davem@...emloft.net, johnstul@...ibm.com, w41ter@...il.com
Subject: Re: [patch net-next-2.6] forcedeth: fix vlans

W dniu 26 lipca 2011 19:49 użytkownik Jesse Gross <jesse@...ira.com> napisał:
> 2011/7/26 Michał Mirosław <mirqus@...il.com>:
>> 2011/7/26 Jiri Pirko <jpirko@...hat.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@...hat.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html