Re: [virtio-comment] [PATCH] Fix virtio_net_hdr struct size when VIRTIO_NET_F_HASH_REPORT is negociated.

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Aug 30 2022, Cyril Germond <cgermond@kalray.eu> wrote:

>>> diff --git a/content.tex b/content.tex
>>> index e863709..b900f35 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -3620,8 +3620,7 @@ \subsubsection{Setting Up Receive
>>> Buffers}\label{sec:Device Types / Network Devi
>>>  features are used, the maximum incoming packet
>>>  will be to 65550 bytes long (the maximum size of a
>>>  TCP or UDP packet, plus the 14 byte ethernet header), otherwise
>>> -1514 bytes.  The 12-byte struct virtio_net_hdr is prepended to this,
>>> -making for 65562 or 1526 bytes.
>>> +1514 bytes.  The struct virtio_net_hdr (which length depends on
>>> VIRTIO_NET_F_HASH_REPORT feature negotiation) is prepended to this.
>> 
>> The normative statement right below also cites the 65562/1526 numbers; I
>> assume that we want to adjust them as well? In that case, I don't think
>> we can get around mentioning concrete numbers for all four cases
>> (with/without offloads x with/without HASH_REPORT).
>> 
>> Any other opinions?
> I agree this parts needs update. Amongst the  3 alternate proposals for updating the normative statements of 5.1.6.3.1 (in the case VIRTIO_NET_F_MRG_RXBUF is not negotiated), which one would suit better (or eventually another one) ?
>
> 1/
> -If VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6 or VIRTIO_NET_F_GUEST_UFO are negotiated, the driver SHOULD populate the receive queue(s) with buffers of at least 65570 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated, 65562 bytes otherwise.
> â Otherwise, the driver SHOULD populate the receive queue(s) with buffers of at least 1534 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated, 1526 bytes otherwise.
>
> 2/
> -If VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6 or VIRTIO_NET_F_GUEST_UFO are negotiated, the driver SHOULD populate the receive queue(s) with buffers of at least :
>       - 65570 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated
>       - 65562 bytes otherwise.
> â Otherwise, the driver SHOULD populate the receive queue(s) with buffers of at least 
>       - 1534 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated
>       - 1526 bytes otherwise.
>
> 3/
> â If VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6 or VIRTIO_NET_F_GUEST_UFO are negotiated, the driver SHOULD populate the receive queue(s) with buffers of at least 65550 bytes plus the size of the struct virtio_net_hdr.
> â Otherwise, the driver SHOULD populate the receive queue(s) with buffers of at least 1514 bytes plus the size of the struct virtio_net_hdr.

I'd probably go with option 1, if we do not expect more
header-size-changing features in the foreseeable future, option 2
otherwise (IMHO, 1 is easier to read, while 2 is easier to extend.)

I'll let the virtio-net folks decide on that one.