Re: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add an optional device control over the receive buffers length

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jan 23, 2020 at 06:00:30PM +0800, Jason Wang wrote:
> 
> On 2020/1/23 äå3:46, Michael S. Tsirkin wrote:
> > > > So linux can switch between skb and XDP mode. In skb mode buffer size
> > > > varies, it works by merging multiple buffers. In XDP a single buffer
> > > > is made big enough to hold the whole packet. Length is fixed.
> > > > 
> > > > If we are in XDP mode but buffer was added while in skb mode,
> > > > or vice versa, we recover generally by copying data to
> > > > the correct buffer. This is a temporary slowdown -
> > > > better than dropping packets.
> > > > 
> > > > So let's assume the device ratio is 1.
> > > > I guess while in XDP mode, we'll write XDP buffer length into
> > > > this field. But in skb mode, we can make the buffer smaller.
> > > > This implies driver needs to change the min_rx_buf_len?
> > > I'm not sure I get here. The header room should be invisible from device
> > > point of view I think.
> > > 
> > > Thanks
> > > 
> > In fact I am confused. We have this comment:
> > 
> >                  /* This happens when rx buffer size is underestimated
> >                   * or headroom is not enough because of the buffer
> >                   * was refilled before XDP is set. This should only
> >                   * happen for the first several packets, so we don't
> >                   * care much about its performance.
> >                   */
> > 
> > but what does ensure that num_buf == 1 except for the first several
> > packets?
> 
> 
> We disable GUEST_TSO, GUEST_UFO, for XDP and the minimal packet is 1500.

did you mean minimal buffer? that's the part I don't see.
what sets buffer size to 1500?

> So
> it should be fine unless guest MTU is greater than 1500.
> 
> If guest MTU is greater than 1500 it could be a problem which needs to be
> fixed.
> 
> 
> > 
> > We seem to be calling add_recvbuf_mergeable which in turn uses ewma
> > to estimate the packet size, but it seems that XDP never updates ewma so
> > the size will be whatever it happened to be, no?
> 
> 
> It looks to me we can't use EWMA which may cause packet size under
> estimation. Sticking to MTU should be fine.
> 

So I'm not seeing things, it's a bug right?

> > 
> > I guess we need a counter in this slow path so we can notice it
> > happening ...
> 
> 
> Right, this could be added.
> 
> Thanks