[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] Re: [virtio-dev] Re: [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum
å 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 validatesthe 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 mappedto 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 willstill 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, belowdesctiption 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 checksumoffload 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/185Hi 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? Thanks!
--- 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. @Michaelv3->v4: - Streamline some repetitive descriptions. @Jason- Add how features should work, when to be enabled, and overhead. @Jason @Michaelv2->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.texindex 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-listsCommittee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/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.pdfList Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-listsCommittee: 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]