[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] Re: [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum
å 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>--- v7->v8: - Drop the new FULLY_CSUM feature and optimize the description. @Jason v6->v7: - FULLY_CSUM no longer depends on GUEST_CSUM. - The description related to DATA_VALID has nothing to do with whether GUEST_CSUM is negotiated or not. @Jason v5->v6: - Rewrite and clarify patch description. @Michael v4->v5: - Remove the modification to the GUEST_CSUM. @Jason - The description of this feature has been reorganized for greater clarity. @Michael v3->v4: - Streamline some repetitive descriptions. @Jason - Add how features should work, when to be enabled, and overhead. @Jason @Michael v2->v3: - Add a section named "Driver Handles Fully Checksummed Packets" and more descriptions. @Michael v1->v2: - Modify full checksum functionality as a configurable offload that is initially turned off. @Jason device-types/net/description.tex | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/device-types/net/description.tex b/device-types/net/description.tex index aff5e08..14978c0 100644 --- a/device-types/net/description.tex +++ b/device-types/net/description.tex @@ -723,8 +723,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network \field{num_buffers} is one, then the entire packet will be contained within this buffer, immediately following the struct virtio_net_hdr. -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the - VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be +\item The VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be set: if so, device has validated the packet checksum. In case of multiple encapsulated protocols, one level of checksums has been validated. @@ -783,9 +782,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network packet together, such that at least \field{num_buffers} are observed by driver as used. -If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set -\field{flags} to zero and SHOULD supply a fully checksummed -packet to the driver. +If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device SHOULD supply +a fully checksummed packet to the driver. If VIRTIO_NET_F_GUEST_TSO4 is not negotiated, the device MUST NOT set \field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV4. @@ -842,12 +840,6 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network not less than the length of the headers, including the transport header. -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the -device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in -\field{flags}, if so, the device MUST validate the packet -checksum (in case of multiple encapsulated protocols, one level -of checksums is validated). - \drivernormative{\paragraph}{Processing of Incoming Packets}{Device Types / Network Device / Device Operation / Processing of Incoming Packets} -- 1.8.3.1This 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/
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]