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

[Jason Wang <jasowang@redhat.com>]

å 2022/9/5 17:43, Xuan Zhuo åé:
> 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.


I don't understand this issue fully, but if it's a driver behavior, we 
can change.


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


I remember that XDP supports multi-buffer now, can it be used here?

Thanks


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