Re: [virtio-dev] Re: [PATCH v7] virtio_net: support split header

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

On Mon, Sep 05, 2022 at 05:43:27PM +0800, Xuan Zhuo wrote:
> On Mon, 5 Sep 2022 16:37:57 +0800, Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > å 2022/9/5 äå3:52, Xuan Zhuo åé:
> > > On Sun, 4 Sep 2022 16:31:59 -0400, "Michael S. Tsirkin"<mst@redhat.com>  wrote:
> > >> On Fri, Sep 02, 2022 at 04:58:16PM +0800, Heng Qi wrote:
> > >>> When VIRTIO_NET_F_SPLIT_TRANSPORT_HEADER is negotiated,
> > >>> the driver requires that the buffers submitted to receiveq
> > >>> MUST be composed of at least two descriptors,
> > >>> which means that each buffer the device gets is a descriptor chain,
> > >>> even if the device does not split the header for some packets.
> > >>>
> > >>> To store packet in the descriptor chain without header splitting
> > >>> by the device, the device MUST start with the first descriptor of
> > >>> the descriptor chain to store the packet, and MUST NOT set the
> > >>> VIRTIO_NET_HDR_F_SPLIT_TRANSPORT_HEADER bit in \field{flags}.
> > >>>
> > >>> Thanks.
> > >> Descriptor chains will hurt performance badly.
> > > I understand the reasons for the performance impact here are:
> > > 1. Two buffers are used
> > > 2. One buffer occupies two descs
> > >
> > > This is the same as my understanding in the case of mergeable. We also need to
> > > pack the packets into two buffers, and a packet will eventually occupy two
> > > descs.
> > >
> > >
> > >> How about simply making this feature depend on mergeable buffers?
> > >> Then we have a separate buffer for the header and
> > >> this works cleanly.
> > >
> > > Under mergeable, each buffer is independent, and the split header requires two
> > > unequal descs.
> > >
> > > If we implement it based on mergeable, then consider the scenario of tcp
> > > zerocopy, when we fill receive vq, each buffer is an separate page, and if we use an
> > > separate buffer to save the header, then this is a waste, we may
> > > have to copy at the driver layer.
> > >
> > > @Qi Do you think there will be other problems with this approach?
> > >
> > > Thanks.
> > >
> > When we think about specs, we shouldn't be too distracted by the implementation.
> >
> > But when we did think about this, suppose the driver fills by page based on
> > mergeable mode. in order to use the xdp program, the driver usually takes
> > the beginning of a single page as the headroom, and fills the rest of the page
> > into the virtqueue. Therefore, the empty buffer obtained by the
> > device is always smaller than a page when we implement split header
> > based on this mode, that is, the data load finally obtained by the driver
> > is offset from the beginning of the page. This does not enjoy the benefits of zero copy.
> >
> > At the same time, since the header is always only more than 100 bytes,
> > the page occupied by the header is a waste of the buffer.
> 
> 
> Yeah that reminds me that merge doesn't feel like it handles this very well.
> 
> The essence is that the two buffers used by the split header are different.
> 
> Desc Chain is used to bind a small buffer desc and a page desc. I didn't think
> of a better way to deal with this problem.
> 
> Thanks.
> 

I sent some suggestions avoiding use of descriptors completely,
using offsets instead. take a look.

> >
> > Thanks.
> >
> >
> >
> >
> >
> >
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org