Date:   Tue, 10 Jan 2017 04:51:34 +0200
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     John Fastabend <john.fastabend@...il.com>,
        john.r.fastabend@...el.com, netdev@...r.kernel.org,
        alexei.starovoitov@...il.com, daniel@...earbox.net
Subject: Re: [net PATCH] net: virtio: cap mtu when XDP programs are running

On Tue, Jan 10, 2017 at 10:29:39AM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月10日 07:58, Michael S. Tsirkin wrote:
> > On Mon, Jan 09, 2017 at 03:49:27PM -0800, John Fastabend wrote:
> > > On 17-01-09 03:24 PM, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 09, 2017 at 03:13:15PM -0800, John Fastabend wrote:
> > > > > On 17-01-09 03:05 PM, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jan 05, 2017 at 11:09:14AM +0800, Jason Wang wrote:
> > > > > > > On 2017年01月05日 02:57, John Fastabend wrote:
> > > > > > > > [...]
> > > > > > > > 
> > > > > > > > > On 2017年01月04日 00:48, John Fastabend wrote:
> > > > > > > > > > On 17-01-02 10:14 PM, Jason Wang wrote:
> > > > > > > > > > > On 2017年01月03日 06:30, John Fastabend wrote:
> > > > > > > > > > > > XDP programs can not consume multiple pages so we cap the MTU to
> > > > > > > > > > > > avoid this case. Virtio-net however only checks the MTU at XDP
> > > > > > > > > > > > program load and does not block MTU changes after the program
> > > > > > > > > > > > has loaded.
> > > > > > > > > > > > 
> > > > > > > > > > > > This patch sets/clears the max_mtu value at XDP load/unload time.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: John Fastabend<john.r.fastabend@...el.com>
> > > > > > > > > > > > ---
> > > > > > > > [...]
> > > > > > > > 
> > > > > > > > > > OK so this logic is a bit too simply. When it resets the max_mtu I guess it
> > > > > > > > > > needs to read the mtu via
> > > > > > > > > > 
> > > > > > > > > >       virtio_cread16(vdev, ...)
> > > > > > > > > > 
> > > > > > > > > > or we may break the negotiated mtu.
> > > > > > > > > Yes, this is a problem (even use ETH_MAX_MTU). We may need a method to notify
> > > > > > > > > the device about the mtu in this case which is not supported by virtio now.
> > > > > > > > Note this is not really a XDP specific problem. The guest can change the MTU
> > > > > > > > after init time even without XDP which I assume should ideally result in a
> > > > > > > > notification if the MTU is negotiated.
> > > > > > > Yes, Michael, do you think we need add some mechanism to notify host about
> > > > > > > MTU change in this case?
> > > > > > > 
> > > > > > > Thanks
> > > > > > Why does host care?
> > > > > > 
> > > > > Well the guest will drop packets after mtu has been reduced.
> > > > I didn't know. What place in code does this?
> > > > 
> > > hmm in many of the drivers it is convention to use the mtu to set the rx
> > > buffer sizes and a receive side max length filter. For example in the Intel
> > > drivers if a packet with length greater than MTU + some headroom is received we
> > > drop it. I guess in the networking stack RX path though nothing forces this and
> > > virtio doesn't have any code to drop packets on rx size.
> > > 
> > > In virtio I don't see any existing case currently. In the XDP case though we
> > > need to ensure packets fit in a page for the time being which is why I was
> > > looking at this code and generated this patch.
> > I'd say just look at the hardware max mtu. Ignore the configured mtu.
> > 
> > 
> 
> Does this work for small buffers consider it always allocate skb with size
> of GOOD_PACKET_LEN?

Spec says hardware won't send in packets > max mtu in config space.

> I think in any case, we should limit max_mtu to
> GOOD_PACKET_LEN for small buffers.
> 
> Thanks

XDP seems to have a bunch of weird restrictions, I just
do not like it that the logic spills out to all drivers.
What if someone decides to extend it to two pages in the future?
Recode it all in all drivers ...

Why can't net core enforce mtu?

-- 
MST

Powered by blists - more mailing lists