netdev - Re: [BUG] bnx2 + vlan + TSO : doesnt work

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]

Date:	Tue, 18 Jan 2011 01:32:33 -0500
From:	Jesse Gross <jesse@...ira.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	David Miller <davem@...emloft.net>, bhutchings@...arflare.com,
	netdev@...r.kernel.org
Subject: Re: [BUG] bnx2 + vlan + TSO : doesnt work

On Tue, Jan 18, 2011 at 1:21 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Le lundi 17 janvier 2011 à 22:09 -0800, David Miller a écrit :
>> From: Jesse Gross <jesse@...ira.com>
>> Date: Mon, 17 Jan 2011 16:13:18 -0800
>>
>> > I think it is better for netif_skb_features() to actually return the
>> > correct features rather than bypass it here.  NETIF_F_HW_VLAN_TX
>> > doesn't depend on any other offloads, so we can always include it if
>> > it is in dev->features.
>> >
>> > Separately, this means there is a problem with bnx2 because it allows
>> > vlan insertion to be turned off, which would have the same effect.
>> > Maybe it is looking directly at skb->protocol or similar for TSO.
>>
>> Please, someone cons up an acceptable fix fast.
>>
>
> I just woke up, and honestly dont understand why only bnx2 is affected
> by this problem of masking NETIF_F_HW_VLAN_TX

If NETIF_F_HW_VLAN_TX is masked then the tag will be inserted in
software, which is generally OK.  The problem is that some drivers
assume that if they can do vlan tagging in hardware it will always be
used and therefore don't expect vlan tags when setting up TSO, etc.

>
> And I dont understand all this netif_skb_features() stuff : if we want
> to actually test dev->features & NETIF_F_HW_VLAN_TX, and this flag
> doesnt depend on other offloads, why are we doing features &
> vlan_features.

The idea is to put all of the logic in one place rather than having
pieces that are really interdependent scattered around in the
different offloads.  So we could test dev->features directly for vlans
but I would rather just have netif_skb_features() return the right
values to start off with.

>
> Jesse, I dont understand why you say "bnx2 allows vlan insertion to be
> turned off". Really.

You can disable it using Ethtool, which will turn off
NETIF_F_HW_VLAN_TX the same as this bug.

I'm running a quick test on a patch to always allow NETIF_F_HW_VLAN_TX
to be returned from netif_skb_features().
--
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