netdev - Re: [PATCH net-next 1/6] net: add new netdevice feature for tunnel offloading

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] [day] [month] [year] [list]

Message-ID: <20170704143733.GA32160@bistromath.localdomain>
Date:   Tue, 4 Jul 2017 16:37:33 +0200
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     netdev@...r.kernel.org,
        Hannes Frederic Sowa <hannes@...essinduktion.org>
Subject: Re: [PATCH net-next 1/6] net: add new netdevice feature for tunnel
 offloading

2017-06-30, 17:50:01 +0200, Jiri Benc wrote:
> On Fri, 30 Jun 2017 15:19:45 +0200, Sabrina Dubroca wrote:
> > This adds a new netdevice feature, so that tunnel offloading can be
> > disabled by the administrator on some netdevices, using the
> > "tunnel-offload" ethtool feature.
> > 
> > This feature is set for all devices that provide ndo_udp_tunnel_add.
> 
> This patchset looks great, Sabrina! A few comments below.
> 
> > --- a/include/linux/netdev_features.h
> > +++ b/include/linux/netdev_features.h
> > @@ -76,6 +76,7 @@ enum {
> >  	NETIF_F_HW_TC_BIT,		/* Offload TC infrastructure */
> >  	NETIF_F_HW_ESP_BIT,		/* Hardware ESP transformation offload */
> >  	NETIF_F_HW_ESP_TX_CSUM_BIT,	/* ESP with TX checksum offload */
> > +	NETIF_F_TUNNEL_OFFLOAD_BIT,     /* Tunnel offloads */
> 
> "Tunnel offload" is very broad. Could we be more specific, e.g. "RSS
> tunnel offload" or such? NETIF_F_HW_TUNNEL_RSS?

Yeah, "tunnel offload" is too broad. But RSS isn't correct here, since
that's not the only thing that devices do with that feature.

Maybe "udp tunnel rx port" instead? Although, as Hannes pointed out
yesterday, we're not really sure that drivers/devices will only use it
for RX purposes either.


> > --- a/net/core/ethtool.c
> > +++ b/net/core/ethtool.c
> > @@ -106,6 +106,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> >  	[NETIF_F_HW_TC_BIT] =		 "hw-tc-offload",
> >  	[NETIF_F_HW_ESP_BIT] =		 "esp-hw-offload",
> >  	[NETIF_F_HW_ESP_TX_CSUM_BIT] =	 "esp-tx-csum-hw-offload",
> > +	[NETIF_F_TUNNEL_OFFLOAD_BIT] =	 "tunnel-offload",
> >  };
> 
> And here, too. "rss-tunnel-offload"?

Yep, I'll update all that when we figure out a reasonable name.


Another thing I was thinking about would be to modify ethtool to show
these UDP tunneling features as a hierarchy (like we already have for
tx-checksumming). Both for this new feature and the existing
tx-udp_tnl-*.


Thanks for the comments,

-- 
Sabrina