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

[Jason Wang <jasowang@redhat.com>]

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. 
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.


> I guess we need a counter in this slow path so we can notice it
> happening ...


Right, this could be added.

Thanks