Re: [PATCH v7] virtio_net: support split header

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

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.


>
> Thanks.
>
>
>
>
>
>
>