Subject: Re: virtio_net: can change MTU after installing program

From: "Michael S. Tsirkin" <[email-masked]>
Date: Wed, 26 Feb 2020 11:55:02 -0500

On Wed, Feb 26, 2020 at 09:03:47AM -0700, David Ahern wrote:
> On 2/26/20 2:51 AM, Toke Høiland-Jørgensen wrote:
> > "Michael S. Tsirkin" <[email-masked]> writes:
> > 
> >> On Tue, Feb 25, 2020 at 08:32:14PM -0700, David Ahern wrote:
> >>> Another issue is that virtio_net checks the MTU when a program is
> >>> installed, but does not restrict an MTU change after:
> >>>
> >>> # ip li sh dev eth0
> >>> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdp qdisc fq_codel
> >>> state UP mode DEFAULT group default qlen 1000
> >>>     link/ether 5a:39:e6:01:a5:36 brd ff:ff:ff:ff:ff:ff
> >>>     prog/xdp id 13 tag c5595e4590d58063 jited
> >>>
> >>> # ip li set dev eth0 mtu 8192
> >>>
> >>> # ip li sh dev eth0
> >>> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 8192 xdp qdisc fq_codel
> >>> state UP mode DEFAULT group default qlen 1000
> >>>
> >>>
> >>
> >> Cc Toke who has tested this on other cards and has some input.
> > 
> > Well, my comment was just that we already restrict MTU changes on mlx5
> > when an XDP program is loaded:
> > 
> > $ sudo ip link set dev ens1f1 mtu 8192
> > RTNETLINK answers: Invalid argument
> > 
> > Reading through the rest of the thread I don't have any strong opinions
> > about whether this should propagate out from the host or not. I suspect
> > it would not be worth the trouble, though, and as you say it's already
> > possible to configure regular network devices in a way that is
> > incompatible with the rest of the network.
> > 
> 
> Both mlx5 and sfc restrict MTU change to XDP limits; virtio does not
> which strikes me as a problem.

OK that seems to indicate an ndo callback as a reasonable way
to handle this. Right? The only problem is this might break
guests if they happen to reverse the order of
operations:
	1. set mtu
	2. detach xdp prog
would previously work fine, and would now give an error.

If we want to make it transparent for userspace,
I guess we can defer the actual update until xdp prog is detached.
Sound ugly and might still confuse some userspace ... worth it?

-- 
MST