netdev - Re: [PATCH] bonding: allow TSO being set on bonding master

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: <CAHo-Oozt9tmfhhFHj0o+bGfRevcZw+YD6PpHAr-U_S76AbC+2A@mail.gmail.com>
Date:	Wed, 15 May 2013 21:31:10 -0700
From:	Maciej Żenczykowski <zenczykowski@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Michał Mirosław <mirqus@...il.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>, Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Tom Herbert <therbert@...gle.com>,
	Neal Cardwell <ncardwell@...gle.com>,
	Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH] bonding: allow TSO being set on bonding master

I think it should always be possible to enable both TSO and GSO on the
bond master - regardless of what the slaves support and/or currently
have enabled.
At the last moment as we're passing a packet to a slave, we should
check whether the slave will like it and if not GSO it.
This means TSO or GSO enabled on the master can effectively almost
force (or force emulation of) GSO on slaves.
Even if the slaves themselves don't support GSO.

There's a lot more room for argument with regards to the default state
of TSO/GSO on the master.
I would argue for default to on for both.
One could argue that if none of the slaves support GSO or TSO then
maybe it should default to off, but I don't buy that.

I don't really understand the point of passing GSO/TSO up from the
slaves to the master.
Although the maximum size and/or number of segs for a packet probably
needs to be propagated from TSO capable devices,
unless one were to add a "split big GSO packet into smaller (but still
larger than mtu) TSO packets" step as well.

On Wed, May 15, 2013 at 4:30 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Wed, 2013-05-15 at 16:18 -0700, Eric Dumazet wrote:
>> On Thu, 2013-05-16 at 00:55 +0200, Michał Mirosław wrote:
>>
>> > Have you tried adding it to NETIF_F_ONE_FOR_ALL set? 'Team' and bridge
>> > could then also use it.
>>
>> Good point, I am testing this, thanks !
>>
>
> Hmm, this doesnt work.
>
>
> # ethtool -k bond0 | grep tcp-segmentation-offload
> tcp-segmentation-offload: off
> # ethtool -K bond0 tso on
> # ethtool -k bond0 | grep tcp-segmentation-offload
> tcp-segmentation-offload: off
>
> Do you have something different in mind ?
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 77f5202..c8d9db0 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -132,7 +132,7 @@ enum {
>   * for all in netdev_increment_features.
>   */
>  #define NETIF_F_ONE_FOR_ALL    (NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ROBUST | \
> -                                NETIF_F_SG | NETIF_F_HIGHDMA |         \
> +                                NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_ALL_TSO |       \
>                                  NETIF_F_FRAGLIST | NETIF_F_VLAN_CHALLENGED)
>  /*
>   * If one device doesn't support one of these features, then disable it
>
>
> --
> 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
--
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