netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>,
	"Ziyang Xuan (William)" <william.xuanziyang@huawei.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Vasily Averin <vvs@virtuozzo.com>,
	Kees Cook <keescook@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] net: vlan: allow vlan device MTU change follow real device from smaller to bigger
Date: Wed, 23 Feb 2022 12:26:18 +0100	[thread overview]
Message-ID: <20220223112618.GA19531@debian.home> (raw)
In-Reply-To: <20220222152815.1056ca24@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Tue, Feb 22, 2022 at 03:28:15PM -0800, Jakub Kicinski wrote:
> On Tue, 22 Feb 2022 11:37:33 +0100 Guillaume Nault wrote:
> > What about an explicit option:
> > 
> >   ip link add link eth1 dev eth1.100 type vlan id 100 follow-parent-mtu
> > 
> > 
> > Or for something more future proof, an option that can accept several
> > policies:
> > 
> >   mtu-update <reduce-only,follow,...>
> > 
> >       reduce-only (default):
> >         update vlan's MTU only if the new MTU is smaller than the
> >         current one (current behaviour).
> > 
> >       follow:
> >         always follow the MTU of the parent device.
> > 
> > Then if anyone wants more complex policies:
> > 
> >       follow-if-not-modified:
> >         follow the MTU of the parent device as long as the VLAN's MTU
> >         was not manually changed. Otherwise only adjust the VLAN's MTU
> >         when the parent's one is set to a smaller value.
> > 
> >       follow-if-not-modified-but-not-quite:
> >         like follow-if-not-modified but revert back to the VLAN's
> >         last manually modified MTU, if any, whenever possible (that is,
> >         when the parent device's MTU is set back to a higher value).
> >         That probably requires the possibility to dump the last
> >         modified MTU, so the administrator can anticipate the
> >         consequences of modifying the parent device.
> > 
> >      yet-another-policy (because people have a lot of imagination):
> >        for example, keep the MTU 4 bytes lower than the parent device,
> >        to account for VLAN overhead.
> > 
> > Of course feel free to suggest better names and policies :).
> > 
> > This way, we can keep the current behaviour and avoid unexpected
> > heuristics that are difficult to explain (and even more difficult for
> > network admins to figure out on their own).
> 
> My $0.02 would be that if we want to make changes that require new uAPI
> we should do it across uppers.

Do you mean something like:

  ip link set dev eth0 vlan-mtu-policy <policy-name>

that'd affect all existing (and future) vlans of eth0?

Then I think that for non-ethernet devices, we should reject this
option and skip it when dumping config. But yes, that's another
possibility.

I personnaly don't really mind, as long as we keep a clear behaviour.

What I'd really like to avoid is something like:
  - By default it behaves this way.
  - If you modified the MTU it behaves in another way
  - But if you modified the MTU but later restored the
    original MTU, then you're back to the default behaviour
    (or not?), unless the MTU of the upper device was also
    changed meanwhile, in which case ... to be continued ...
  - BTW, you might not be able to tell how the VLAN's MTU is going to
    behave by simply looking at its configuration, because that also
    depends on past configurations.
  - Well, and if your kernel is older than xxx, then you always get the
    default behaviour.
  - ... and we might modify the heuristics again in the future to
    accomodate with situations or use cases we failed to consider.


  reply	other threads:[~2022-02-23 11:26 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-21 12:46 [PATCH net] net: vlan: allow vlan device MTU change follow real device from smaller to bigger Ziyang Xuan
2022-02-21 15:43 ` Eric Dumazet
2022-02-22  0:58   ` Herbert Xu
2022-02-22  2:06     ` Ziyang Xuan (William)
2022-02-22  2:27       ` Eric Dumazet
2022-02-22  7:31         ` Ziyang Xuan (William)
2022-02-23  1:55           ` Ziyang Xuan (William)
2022-02-22 10:37         ` Guillaume Nault
2022-02-22 23:28           ` Jakub Kicinski
2022-02-23 11:26             ` Guillaume Nault [this message]
2022-02-23 15:17               ` Stephen Hemminger
2022-02-23 16:34                 ` Guillaume Nault
2022-02-23 16:03               ` Jakub Kicinski
2022-02-23 16:58                 ` Guillaume Nault
2022-02-23 17:37                   ` Jakub Kicinski
2022-02-23 19:46                     ` Guillaume Nault
2022-02-23 17:05                 ` Stephen Hemminger

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=20220223112618.GA19531@debian.home \
    --to=gnault@redhat.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vvs@virtuozzo.com \
    --cc=william.xuanziyang@huawei.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).