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

2022-09-13 Thread Jason Wang



在 2022/9/9 20:38, Xuan Zhuo 写道:

On Fri, 9 Sep 2022 07:15:02 -0400, "Michael S. Tsirkin"  wrote:

On Fri, Sep 09, 2022 at 03:41:54PM +0800, Heng Qi wrote:


在 2022/9/5 上午4:27, Michael S. Tsirkin 写道:

On Fri, Sep 02, 2022 at 03:36:25PM +0800, Heng Qi wrote:

We need to clarify that the purpose of header splitting is to make all payloads
can be independently in a page, which is beneficial for the zerocopy
implemented by the upper layer.

absolutely, pls add motivation.


If the driver does not enforce that the buffers submitted to the receiveq MUST
be composed of at least two descriptors, then header splitting will become 
meaningless,
or the VIRTIO_NET_F_SPLIT_TRANSPORT_HEADER feature should not be negotiated at 
this time.


Thanks.



This seems very narrow and unecessarily wasteful of descriptors.
What is wrong in this:

.. 

seems to achieve the goal of data in a separate page without
using extra descriptors.

thus my proposal to replace the requirement of a separate
descriptor with an offset of data from beginning of
buffer that driver sets.



We have carefully considered your suggestion.

We refer to spec v7 and earlier as scheme A for short. Review scheme A
below:

| receive buffer |

| 0th descriptor | 1th descriptor |

| virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->| payload |

We use a buffer plus a separate page when allocating the receive

buffer. In this way, we can ensure that all payloads can be

independently in a page, which is very beneficial for the zerocopy

implemented by the upper layer.

scheme A better solves the problem of headroom, tailroom and memory waste,
but as you said, this solution relies on descriptor chain.

Our rethinking approach is no longer based on or using descriptor chain.

We refer to your proposed offset-based scheme as scheme B:

As you suggested, scheme B gives the device a buffer, using offset to
indicate where to place the payload like this:

.. 

But how to apply for this buffer? Since we want the payload to be placed on
a separate page, the method we consider is to directly apply to the driver
for two pages of contiguous memory.

Then the beginning of this contiguous memory is used to store the headroom,
and the contiguous memory after the headroom is directly handed over to the
device. similar to the following:

<-- receive buffer(2 pages)
->

<<-- first page
--->< second page -->>

<>

Based on your previous suggestion, we also considered another new scheme C.

This scheme is implemented based on mergeable buffer, filling a separate
page each time.

If the split header is negotiated and the packet can be successfully split
by the device, the device needs to find at least two buffers, namely two
pages, one for the virtio-net header and transport header, and the other for
the data payload. Like the following:

| receive buffer1(page) | receive buffer2 (page) |

| virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->| payload |

At the same time, if XDP is considered, then the device needs to add
headroom at the beginning of receive buffer1 when receiving packets, so that
the driver can process programs similar to XDP. In order to solve this
problem, can scheme C introduce an offset, which requires the device to
write data from the offset position to receive buffer1, like the following:

| receive buffer (page) | receive buffer (page) |

| <-- offset(hold) --> | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|
payload |

And in fact, B and C both use an offset now, right?


B: offset is used to get the position to place the payload.
C: The offset is used to reserve some space for the device, which the driver can
use as headroom.

In order to make the payload page-aligned, we can only hand over the entire
page to the device, so we cannot reserve some headroom in advance.



For C, it might be better to do some tweak since mergeable buffer 
doesn't forbid using a descriptor chain as a single buffer.


So if it's a descriptor chain we got back the method A by placing the 
payload in a dedicated buffer. If it's not placing the payload in an 
adjacent buffer.


Thanks





Then we simply compare the advantages and disadvantages of scheme A(spec
v7), scheme B (offset buffer(2 pages)) and scheme C (based on mergeable
buffer):

1. desc chain:

- A depends on desciptor chain; - B, C do not depend on desciptor chain.

2. page alloc

- B fills two consecutive pages, which causes a great waste of memory for
small packages such as arp; - C fills a single page, slightly better than B.

3. Memory waste:

- The memory waste of scheme A is mainly the 0th descriptor that is skipped
by the device;

there's also the cost of the indirect buffer since that is used when
there is a chain.

Yes



- When scheme B and scheme C successfully split the header,
there is a huge waste of the first page, but the 

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

2022-09-13 Thread Heng Qi
On Fri, Sep 09, 2022 at 08:47:57PM +0800, Xuan Zhuo wrote:
> 
> hi
>Qi also sent another same email today. Due to some email client problems,
>this email has some confusion in the format, so we can discuss
>under another one.
> 
>https://lists.oasis-open.org/archives/virtio-dev/202209/msg00066.html
> 

Yes. Due to some formatting issues with the mail client, I resent this new email
which may be in a clear style for your review.

Do you have more questions about the contents of this new email?

Looking forward to your comments.

Thanks.

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

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org