public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat•com>
To: Herbert Xu <herbert@gondor•hengli.com.au>
Cc: David Miller <davem@davemloft•net>,
	mirqus@gmail•com, shanwei@cn•fujitsu.com,
	mirq-linux@rere•qmqm.pl, netdev@vger•kernel.org,
	bhutchings@solarflare•com
Subject: Re: tap/bridge: Dropping NETIF_F_GSO/NETIF_F_SG
Date: Tue, 17 May 2011 08:18:45 +0300	[thread overview]
Message-ID: <20110517051845.GA26414@redhat.com> (raw)
In-Reply-To: <20110516234538.GA11832@gondor.apana.org.au>

On Tue, May 17, 2011 at 09:45:38AM +1000, Herbert Xu wrote:
> On Mon, May 16, 2011 at 07:06:15PM -0400, David Miller wrote:
> >
> > Well the check has to exist somewhere.
> > 
> > Currently userspace can configure tun/tap into whatever set
> > of offloads it likes.
> > 
> > We're warning when the user asks for something that needs to be
> > corrected.  So the only thing you can suggest is to duplicate these
> > changes in the tun/tap driver.
> > 
> > But if we do that, and error on bad combinations instead of fixing
> > them up, we know from this discussion that existing virtualization
> > setups and tools are going to stop working.
> 
> Yeah the tun driver is simply busted.  We should never have allowed
> user-space to tweak the feature bits like this.  Instead they should
> have gone through the ethtool interface like everyone else, or at
> least use the same underlying calls as ethtool.
> 
> Actually, I think we can still do that, and apply the same rules
> as ethtool with respect to automatically turning things on/off.
> 
> AFAICS the current set_offload in tun.c does not call anything
> that verifies/fixes up the settings.  If you change the feature
> bits after registering the tun device it may never get fixed up
> at all.

Hmm, we get the warnings about bits dropped on each set_offload
call:
	netdev_update_features is called,
	that calls netdev_fix_features

No?

> Allowing an unprivileged user to tweak feature bits directly with
> no verification is just wrong.
> 
> Cheers,

But we do verify bits, and only allow the user
to tweak these ones:

#define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO|
\
                          NETIF_F_TSO6|NETIF_F_UFO)

No?

> -- 
> Email: Herbert Xu <herbert@gondor•apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  reply	other threads:[~2011-05-17  5:18 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-04 18:18 tap/bridge: Dropping NETIF_F_GSO/NETIF_F_SG Michael S. Tsirkin
2011-05-04 22:34 ` Herbert Xu
2011-05-04 23:28   ` Michał Mirosław
2011-05-05  0:19     ` Herbert Xu
2011-05-05  8:44     ` Michael S. Tsirkin
2011-05-05  9:34       ` Shan Wei
2011-05-05 10:05         ` Herbert Xu
2011-05-16  7:32           ` Michael S. Tsirkin
2011-05-16  8:07             ` Herbert Xu
2011-05-16  8:18               ` Michael S. Tsirkin
2011-05-16  9:38                 ` Herbert Xu
2011-05-16  9:48                   ` Michael S. Tsirkin
2011-05-16 10:43                     ` Herbert Xu
2011-05-16 11:21                       ` Michael S. Tsirkin
2011-05-16 12:18                         ` Herbert Xu
2011-05-16 12:24                           ` Michał Mirosław
2011-05-16 22:46                             ` Herbert Xu
2011-05-16 23:06                               ` David Miller
2011-05-16 23:45                                 ` Herbert Xu
2011-05-17  5:18                                   ` Michael S. Tsirkin [this message]
2011-05-17  5:24                                     ` Herbert Xu
2011-05-17  5:48                                       ` Michael S. Tsirkin
2011-05-17  6:25                                         ` Herbert Xu
2011-05-17  8:08                               ` Michał Mirosław
2011-05-17  8:15                                 ` Michał Mirosław
2011-05-17  8:19                                 ` [PATCH] net: tuntap: Fix tun_net_fix_features() Michał Mirosław
2011-05-17 14:29                                   ` Michael S. Tsirkin
2011-05-17 14:46                                     ` Michał Mirosław
2011-05-17 14:54                                       ` Michael S. Tsirkin
2011-05-17 15:00                                         ` Michał Mirosław
2011-05-17 15:11                                           ` Michael S. Tsirkin
2011-06-01  9:25                                   ` Michael S. Tsirkin
2011-06-20 19:14                                   ` [RESENT PATCH] " Michał Mirosław
2011-06-20 19:25                                     ` Ben Hutchings
2011-06-20 19:44                                       ` Michał Mirosław
2011-05-16 10:53                     ` tap/bridge: Dropping NETIF_F_GSO/NETIF_F_SG Michał Mirosław
2011-05-16  8:28               ` Michael S. Tsirkin
2011-05-05 15:26 ` Michał Mirosław
2011-05-14  6:54   ` Shan Wei
2011-05-16  7:28   ` Michael S. Tsirkin

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=20110517051845.GA26414@redhat.com \
    --to=mst@redhat$(echo .)com \
    --cc=bhutchings@solarflare$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=herbert@gondor$(echo .)hengli.com.au \
    --cc=mirq-linux@rere$(echo .)qmqm.pl \
    --cc=mirqus@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=shanwei@cn$(echo .)fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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