netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: mkubecek@suse.cz
Cc: jay.vosburgh@canonical.com, netdev@vger.kernel.org,
	vfalico@gmail.com, andy@greyhouse.net, mirq-linux@rere.qmqm.pl
Subject: Re: [PATCH net] bonding: fix vlan_features computing
Date: Wed, 07 May 2014 14:08:46 -0400 (EDT)	[thread overview]
Message-ID: <20140507.140846.1449010184057650775.davem@davemloft.net> (raw)
In-Reply-To: <20140506130315.GB18366@unicorn.suse.cz>

From: Michal Kubecek <mkubecek@suse.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.

  reply	other threads:[~2014-05-07 18:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20140502165448.D8078E635B@unicorn.suse.cz>
2014-05-06  3:45 ` [PATCH net] bonding: fix vlan_features computing David Miller
2014-05-06  7:53   ` Jay Vosburgh
2014-05-06  8:44     ` Michal Kubecek
2014-05-06 13:03     ` Michal Kubecek
2014-05-07 18:08       ` David Miller [this message]
2014-05-20  6:29         ` [PATCH net-next v2 0/3] net: device features handling fixes Michal Kubecek
2014-05-20  6:29           ` [PATCH net-next v2 1/3] vlan: more careful checksum features handling Michal Kubecek
2014-05-20  6:29           ` [PATCH net-next v2 2/3] bonding: fix vlan_features computing Michal Kubecek
2014-05-20  6:29           ` [PATCH net-next v2 3/3] teaming: " Michal Kubecek
2014-05-22 19:07           ` [PATCH net-next v2 0/3] net: device features handling fixes David Miller
2014-05-02 16:57 [PATCH net] bonding: fix vlan_features computing Michal Kubecek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140507.140846.1449010184057650775.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=andy@greyhouse.net \
    --cc=jay.vosburgh@canonical.com \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).