Re: [virtio-comment] Re: [virtio-dev] Re: [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Feb 27 2024, Heng Qi <hengqi@linux.alibaba.com> wrote:

> 在 2024/2/20 上午10:38, Heng Qi 写道:
>>
>>
>> 在 2024/1/16 下午1:52, Xuan Zhuo 写道:
>>> On Wed,  3 Jan 2024 14:35:02 +0800, Heng Qi 
>>> <hengqi@linux.alibaba.com> wrote:
>>>> There is a historical error in virtio spec:
>>>>    "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST* 
>>>> set flags
>>>>     to zero and SHOULD supply a fully checksummed packet to the 
>>>> driver."
>>>>
>>>> Currently in Linux and virtio-related implementations, the device 
>>>> validates
>>>> the packet checksum and sets DATA_VALID regardless of whether
>>>> VIRTIO_NET_F_GUEST_CSUM is negotiated.
>>>>
>>>> Please refer to the following summary and thread[1] for details and 
>>>> reasons:
>>>>
>>>> Summary
>>>> =============
>>>> 1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with 
>>>> partially
>>>> checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM 
>>>> is mapped
>>>> to NETIF_F_RXCSUM.
>>>> GUEST_CSUM only indicates whether the driver handles partially 
>>>> checksummed packets.
>>>> When XDP is loaded, the GUEST_CSUM's offload will be disabled, which 
>>>> means that
>>>> packets have NEEDS_CSUM set will be dropped, and packets have 
>>>> DATA_VALID set will
>>>> still be received.
>>>>
>>>> 2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it 
>>>> was actually
>>>> expected that rx checksum offload (the driver can set 
>>>> CHECKSUM_UNNECESSARY) had
>>>> nothing to do with whether GUEST_CSUM was negotiated. But due to an 
>>>> error, below
>>>> desctiption was added incorrectly:
>>>>    "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST* 
>>>> set flags
>>>>     to zero and SHOULD supply a fully checksummed packet to the 
>>>> driver."
>>>>
>>>> 3. We now hope to correct this error. Let the setting of DATA_VALID 
>>>> not be
>>>> controlled by whether GUEST_CSUM is negotiated, but only controlled 
>>>> by whether
>>>> rx checksum offload is enabled on the OS side. The state of this rx 
>>>> checksum
>>>> offload is not aware of the device.
>>>>
>>>> 4. [Optional] NETIF_F_RXCSUM corresponding to rx checksum offload 
>>>> should be
>>>> added to dev->hw_features. When the user turns off rx checksum 
>>>> offload through
>>>> ethtool -K, neither NEEDS_CSUM nor DATA_VALID should be taken care 
>>>> of, that is,
>>>> all packets will be marked as CHECKSUM_NONE.
>>>>
>>>> Related Link
>>>> =============
>>>> [1] 
>>>> https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
>>>> [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")
>>>>
>>>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/185
>>
>> Hi Cornelia,
>>
>> Can this proposal be opened for voting?
>>
>> Thank you very much!
>>
>>>>
>>>> Suggested-by: Jason Wang <jasowang@redhat.com>
>>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Hi Cornelia.
>
> Did you miss this email? :)
>
> Both Jason and Xuan have replied to Reviewed-by tag.
> Can we initiate a vote?

We currently can't do any voting (OASIS infrastructure update), and we
probably have to sort out the fallout from the new platform once it's
up.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/