Re: [PATCH v2 net-next 10/10] net: bridge: implement auto-normalization of MTU for hardware datapath

From: Jakub Kicinski <kuba@kernel.org>
To: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Cc: Vladimir Oltean <olteanv@gmail.com>,
	Ido Schimmel <idosch@idosch.org>, Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	murali.policharla@broadcom.com,
	Stephen Hemminger <stephen@networkplumber.org>,
	Jiri Pirko <jiri@resnulli.us>, netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH v2 net-next 10/10] net: bridge: implement auto-normalization of MTU for hardware datapath
Date: Thu, 26 Mar 2020 11:49:35 -0700	[thread overview]
Message-ID: <20200326114935.22885985@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <e68cbeb2-f8db-319c-9c4c-32eb3b91a7b9@cumulusnetworks.com>

On Thu, 26 Mar 2020 14:38:57 +0200 Nikolay Aleksandrov wrote:
> On 26/03/2020 14:25, Vladimir Oltean wrote:
> > On Thu, 26 Mar 2020 at 14:19, Nikolay Aleksandrov
> > <nikolay@cumulusnetworks.com> wrote:  
> >>
> >> On 26/03/2020 14:18, Vladimir Oltean wrote:  
> >>> On Thu, 26 Mar 2020 at 14:06, Nikolay Aleksandrov
> >>> <nikolay@cumulusnetworks.com> wrote:  
> >>>>
> >>>> On 26/03/2020 13:35, Ido Schimmel wrote:  
> >>>>> On Thu, Mar 26, 2020 at 12:25:20PM +0200, Vladimir Oltean wrote:  
> >>>>>> Hi Ido,
> >>>>>>
> >>>>>> On Thu, 26 Mar 2020 at 12:17, Ido Schimmel <idosch@idosch.org> wrote:  
> >>>>>>>  
> >>> [snip]  
> >>>>>
> >>>>> I think you should be more explicit about it. Did you consider listening
> >>>>> to 'NETDEV_PRECHANGEMTU' notifications in relevant drivers and vetoing
> >>>>> unsupported configurations with an appropriate extack message? If you
> >>>>> can't veto (in order not to break user space), you can still emit an
> >>>>> extack message.
> >>>>>  
> >>>>
> >>>> +1, this sounds more appropriate IMO
> >>>>  
> >>>
> >>> And what does vetoing gain me exactly? The practical inability to
> >>> change the MTU of any interface that is already bridged and applies
> >>> length check on RX?
> >>>  
> >>
> >> I was referring to moving the logic to NETDEV_PRECHANGEMTU, the rest is up to you.
> >>  
> > 
> > If I'm not going to veto, then I don't see a lot of sense in listening
> > on this particular notifier either. I can do the normalization just
> > fine on NETDEV_CHANGEMTU.
> 
> I should've been more explicit - I meant I agree that this change doesn't belong in
> the bridge, and handling it in a notifier in the driver seems like a better place.
> Yes - if it's not going to be a vetto, then CHANGEMTU is fine.

I'm not sure pushing behavior decisions like that out to the drivers 
is ever a good idea. Linux should abstract HW differences after all,
we don't want different drivers to perform different magic behind
user's back. 

I'd think if HW is unable to apply given configuration vetoing is both
correct and expected..