Virtualization - Aug 2022 - [virtio-dev] [PATCH] virtio-net: use mtu size as buffer length for big packets

On Wed, Aug 10, 2022 at 02:59:55PM +0800, Jason Wang
wrote:
> On Wed, Aug 10, 2022 at 2:15 PM Michael S. Tsirkin <mst at
redhat.com> wrote:
> >
> > On Wed, Aug 10, 2022 at 02:14:07AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Aug 09, 2022 at 04:24:23PM -0700, Si-Wei Liu wrote:
> > > >
> > > >
> > > > On 8/9/2022 3:49 PM, Parav Pandit wrote:
> > > > > > From: Michael S. Tsirkin <mst at redhat.com>
> > > > > > Sent: Tuesday, August 9, 2022 6:26 PM
> > > > > > To: Parav Pandit <parav at nvidia.com>
> > > > > > Cc: Si-Wei Liu <si-wei.liu at oracle.com>;
Jason Wang
> > > > > > <jasowang at redhat.com>; Gavin Li
<gavinl at nvidia.com>; Hemminger,
> > > > > > Stephen <stephen at networkplumber.org>;
davem
> > > > > > <davem at davemloft.net>; virtualization
<virtualization at lists.linux-
> > > > > > foundation.org>; Virtio-Dev <virtio-dev at
lists.oasis-open.org>;
> > > > > > jesse.brandeburg at intel.com; alexander.h.duyck
at intel.com;
> > > > > > kubakici at wp.pl; sridhar.samudrala at intel.com;
loseweigh at gmail.com; Gavi
> > > > > > Teitz <gavi at nvidia.com>
> > > > > > Subject: Re: [virtio-dev] [PATCH] virtio-net: use
mtu size as buffer length for
> > > > > > big packets
> > > > > >
> > > > > > On Tue, Aug 09, 2022 at 09:49:03PM +0000, Parav
Pandit wrote:
> > > > > > > > From: Michael S. Tsirkin <mst at
redhat.com>
> > > > > > > > Sent: Tuesday, August 9, 2022 5:38 PM
> > > > > > > [..]
> > > > > > > > > > I think virtio-net driver
doesn't differentiate MTU and MRU, in
> > > > > > > > > > which case the receive buffer
will be reduced to fit the 1500B
> > > > > > > > > > payload size when mtu is
lowered down to 1500 from 9000.
> > > > > > > > > How? Driver reduced the mXu to
1500, say it is improved to post
> > > > > > > > > buffers of
> > > > > > > > 1500 bytes.
> > > > > > > > > Device doesn't know about it
because mtu in config space is RO field.
> > > > > > > > > Device keep dropping 9K packets
because buffers posted are 1500
> > > > > > bytes.
> > > > > > > > > This is because device follows the
spec " The device MUST NOT pass
> > > > > > > > received packets that exceed mtu".
> > > > > > > >
> > > > > > > >
> > > > > > > > The "mtu" here is the device
config field, which is
> > > > > > > >
> > > > > > > >          /* Default maximum transmit
unit advice */
> > > > > > > >
> > > > > > > It is the field from struct
virtio_net_config.mtu. right?
> > > > > > > This is RO field for driver.
> > > > > > >
> > > > > > > > there is no guarantee device will not
get a bigger packet.
> > > > > > > Right. That is what I also hinted.
> > > > > > > Hence, allocating buffers worth upto mtu is
safer.
> > > > > > yes
> > > > > >
> > > > > > > When user overrides it, driver can be further
optimized to honor such new
> > > > > > value on rx buffer posting.
> > > > > >
> > > > > > no, not without a feature bit promising device
won't get wedged.
> > > > > >
> > > > > I mean to say as_it_stands today, driver can decide to
post smaller buffers with larger mtu.
> > > > > Why device should be affected with it?
> > > > > ( I am not proposing such weird configuration but
asking for sake of correctness).
> > > > I am also confused how the device can be wedged in this
case.
> > >
> > > Yea sorry. I misunderstood the code. It can't be.
> >
> > Here's a problem as I see it. Let's say we reduce mtu.
> > Small buffers are added. Now we increase mtu.
> > Device will drop all packets until small buffers are consumed.
> >
> > Should we make this depend on the vq reset ability maybe?
> 
> The advantage of this is to keep TX working. Or we can use device
> reset as a fallback if there's no vq reset.
> 
> Thanks
Device reset is really annoying in that it loses all the state:
rx filters etc etc.
> 
> >
> > > --
> > > MST
> >

> From: Michael S. Tsirkin <mst at redhat.com>
> Sent: Wednesday, August 10, 2022 5:03 AM
> > >
> > > Should we make this depend on the vq reset ability maybe?
> >
> > The advantage of this is to keep TX working. Or we can use device
> > reset as a fallback if there's no vq reset.
> >
> > Thanks
> 
> Device reset is really annoying in that it loses all the state:
> rx filters etc etc.
The elegant solution is let driver tell the new mtu to the device.
One way to do so is by using existing ctrl vq.
If merged buffer is done, and new mtu is > minimum posting size, no need to
undergo vq reset.
If merged buffer is not done, and buffer posted are smaller than new mtu,
undergo vq reset optionally.