[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH] virtio-net: support distinguishing between partial and full checksum
å 2023/10/13 äå10:00, Jason Wang åé:
On Wed, Oct 11, 2023 at 2:43âPM Heng Qi <hengqi@linux.alibaba.com> wrote:virtio-net works in a virtualized system and is somewhat different from physical nics. One of the differences is that to save virtio device resources, rx may receive packets with partial checksum. However, XDP may cause partially checksummed packets to be dropped. So XDP loading conflicts with the feature VIRTIO_NET_F_GUEST_CSUM. This patch lets the device to supply fully checksummed packets to the driver. Then XDP can coexist with VIRTIO_NET_F_GUEST_CSUM to enjoy the benefits of device verification checksum. In addition, implementation of some performant devices do not generate partially checksummed packets, but the standard driver still need to clear VIRTIO_NET_F_GUEST_CSUM when loading XDP. If these devices negotiate the new feature provided by this patch, then the driver can load XDP. A new feature bit VIRTIO_NET_F_GUEST_FULL_CSUM is added to solve the above situation, which forces the device to supply fully checksummed packets to the driver. Use case example: If VIRTIO_NET_F_GUEST_FULL_CSUM is negotiated and after XDP processes a packetI believe this feature needs to be turned on dynamically after XDP is enabled via control offloads instead of trying to negotiating it from the probe?
I guess what you mean is that we need to add requirements for this feature: The device must supply fully checksummed packets when it receives the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET command with a offloads parameter carrying the VIRTIO_NET_F_GUEST_FULL_CSUM feature bit, otherwise it works as described by VIRTIO_NET_F_GUEST_FULL_CSUM, right?We did not do this because dynamically enabling offload when xdp is loaded will cause both partial checksum and full checksum packets to exist in the queue, resulting in partialled
checksummed packets to be dropped.
with full checksum, the VIRTIO_NET_HDR_F_DATA_VALID bit is still retained, resulting in the stack not needing to validate the checksum again. This is useful for guests with performant devices: e.g. bringing the driver advantages such as cpu savings. Several solutions have been discussed in the previous proposal[1]. After historical discussion, we have tried the method proposed by Jason[2], but some complex scenarios and challenges are difficult to deal with. We now return to the method suggested in [1]. [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html [2] https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/ Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- device-types/net/description.tex | 38 +++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/device-types/net/description.tex b/device-types/net/description.tex index 76585b0..a63d497 100644 --- a/device-types/net/description.tex +++ b/device-types/net/description.tex @@ -88,6 +88,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control channel. +\item[VIRTIO_NET_F_GUEST_FULL_CSUM (50)] Driver handles packets with full checksum. + \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header hash for encapsulated packets. \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification coalescing. @@ -133,6 +135,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM. \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM. \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM. +\item[VIRTIO_NET_F_GUEST_FULL_CSUM] Requires VIRTIO_NET_F_GUEST_CSUM. \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM. \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM. @@ -390,6 +393,11 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets}~\nameref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets} below. + +\item The VIRTIO_NET_F_GUEST_FULL_CSUM feature indicates that the driver handles + packets with full checksum and does not handle packets with partial checksum, + which means that partially checksummed packets can not be received by the driver + and the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can not be set by the device. \end{enumerate} A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore @@ -720,7 +728,8 @@ \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 +\item If the VIRTIO_NET_F_GUEST_CSUM feature (regardless of whether + VIRTIO_NET_F_GUEST_FULL_CSUM was negotiated) was negotiated, 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 @@ -744,7 +753,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network number of coalesced TCP segments in \field{csum_start} field and number of duplicated ACK segments in \field{csum_offset} field and sets bit VIRTIO_NET_HDR_F_RSC_INFO in \field{flags}. -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the +\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated but the + VIRTIO_NET_F_GUEST_FULL_CSUM feature was not negotiated, the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can be set: if so, the packet checksum at offset \field{csum_offset} from \field{csum_start} and any preceding checksums @@ -802,8 +812,9 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network device MUST set the VIRTIO_NET_HDR_GSO_ECN bit in \field{gso_type}. -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the -device MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in +If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated but +the VIRTIO_NET_F_GUEST_FULL_CSUM feature has not been negotiated, +the device MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit inSo FULL_CSUM is mutually exclusive with GUEST_GSO_XXX?
They are not mutually exclusive. If there are packetsÂthat carry gso information and have partial checksum, the purpose of this patch is to calculate full checksum for these packets.
But if the capability of this patch changes to the dynamic enabling you suggested, then I think this feature conflicts with GUEST_GSO_xx. Because XDP currently cannot coexist with GRO_HW etc.
Thanksï
Thanks
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]