netdev - Re: [PATCH net] bonding: fix vlan

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:	Wed, 07 May 2014 14:08:46 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	mkubecek@...e.cz
Cc:	jay.vosburgh@...onical.com, netdev@...r.kernel.org,
	vfalico@...il.com, andy@...yhouse.net, mirq-linux@...e.qmqm.pl
Subject: Re: [PATCH net] bonding: fix vlan_features computing

From: Michal Kubecek <mkubecek@...e.cz>
Date: Tue, 6 May 2014 15:03:15 +0200

> On Tue, May 06, 2014 at 12:53:27AM -0700, Jay Vosburgh wrote:
>> 
>> 	The VLAN device doesn't get _HW_CSUM (aka _GEN_CSUM)?  In
>> ethtool, this is tx-checksum-ip-generic, I believe.
>> 
>> 	I have only one machine with a _IP_CSUM only device (a tg3) set
>> up at the moment, it's running a Fedora 20 3.13 kernel, and a vlan ->
>> bond0 -> eth0 stack shows "generic" offload for the vlan and bond0, and
>> "ipv4" offload for the eth0 itself.
> 
> I checked now and at least since 3.12 bond has NETIF_F_HW_CSUM in its
> features even if its slaves have only NETIF_F_IP(V6)_CSUM. Therefore
> NETIF_F_HW_CSUM in vlan_features does no harm and vlan on top of the
> bond gets NETIF_F_HW_CSUM.
> 
> Unfortunately it means that while I still think the change I proposed
> is correct, it would break things unless computing features of the bond
> is fixed in similar way or vlan_dev_fix_features is modified to handle
> checksumming features more carefully.

I think we need to think more about what exact behavior is desired in
mixed csum feature set cases.  Probably whatever csum offload type is
the most prevelant should be the one advertised by the master.  It
means we will do that software checksum fixup for the oddball slaves,
but it seems the best we can do.

Also, like Jay, I agree that whatever we do in bonding needs to be
done in team too and I'd like the bug fixed at the same time in both
places.

Therefore I'm marking this patch as "changes requested", thanks.
--
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