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

[Cornelia Huck <cohuck@redhat.com>]

On Thu, Aug 04 2022, Heng Qi <hengqi@linux.alibaba.com> wrote:

> å 2022/8/4 äå2:27, Jason Wang åé:
>> On Mon, Aug 1, 2022 at 2:59 PM Heng Qi<hengqi@linux.alibaba.com>  wrote:
>>> @@ -3820,9 +3826,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>   driver MUST NOT use the \field{csum_start} and \field{csum_offset}.
>>>
>>>   If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have
>>> -been negotiated, the driver MAY use \field{hdr_len} only as a hint about the
>>> +been negotiated and the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags}
>>> +is not set, the driver MAY use \field{hdr_len} only as a hint about the
>>>   transport header size.
>>> -The driver MUST NOT rely on \field{hdr_len} to be correct.
>>> +
>>> +If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is not set, the driver
>>> +MUST NOT rely on \field{hdr_len} to be correct.
>> I think we should keep the above description as-is. For whatever case,
>> the driver must not trust the metadata set by the device and must
>> perform necessary sanity tests on them.
>
>
> My idea is to keep the current description as it is,
> but to emphasize in the next version:
> "If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set,
> the driver MAY treat the \field{hdr_len} as the length of the
> protocol header inside the first descriptor."


Just to be clear, you suggest using

"If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have
been negotiated, the driver MAY use \field{hdr_len} only as a hint about the
transport header size.

The driver MUST NOT rely on \field{hdr_len} to be correct.

If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set,
the driver MAY treat the \field{hdr_len} as the length of the
protocol header inside the first descriptor."

(Maybe "...the driver MAY use \field{hdr_len} as a hint about the length
of the protocol header..."? It's still not reliable, right?)

>
>
>>
>>> +
>>>   \begin{note}
>>>   This is due to various bugs in implementations.
>>>   \end{note}