[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH v2] virtio-net: support distinguishing between partial and full checksum
On Sat, Oct 28, 2023 at 10:36âAM Heng Qi <hengqi@linux.alibaba.com> wrote: > > > > å 2023/10/27 äå3:39, Michael S. Tsirkin åé: > > On Thu, Oct 19, 2023 at 02:17:20PM +0800, Heng Qi 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 enable the > >> full checksum offloading, then the driver can load XDP without clearing > >> VIRTIO_NET_F_GUEST_CSUM. > >> > >> A new feature bit VIRTIO_NET_F_GUEST_FULL_CSUM is added to solve the above > >> situation, which provides the driver with configurable receive full checksum > >> offload. If the offload is enabled, then the device must supply fully > >> checksummed packets to the driver. > >> > >> Use case example: > >> If VIRTIO_NET_F_GUEST_FULL_CSUM is negotiated and receive full checksum > >> offload is enabled, after XDP processes a packet 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: > >> 1. Bring the driver advantages such as cpu savings. > >> 2. For devices that do not generate partially checksummed packets themselves, > >> XDP can be loaded in the driver without modifying the hardware behavior. > >> > >> 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> > >> --- > >> v1->v2: > >> 1. Modify full checksum functionality as a configurable offload > >> that is initially turned off. @Jason > >> > >> device-types/net/description.tex | 54 ++++++++++++++++++++++++++++---- > >> 1 file changed, 48 insertions(+), 6 deletions(-) > >> > >> diff --git a/device-types/net/description.tex b/device-types/net/description.tex > >> index 76585b0..3c34f27 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 and VIRTIO_NET_F_CTRL_GUEST_OFFLOADS. > >> > >> \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM. > >> \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM. > > What about all of these: > > > > device-types/net/description.tex:\item[VIRTIO_NET_F_GUEST_TSO4] Requires VIRTIO_NET_F_GUEST_CSUM. > > device-types/net/description.tex:\item[VIRTIO_NET_F_GUEST_TSO6] Requires VIRTIO_NET_F_GUEST_CSUM. > > device-types/net/description.tex:\item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM. > > device-types/net/description.tex:\item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM. > > device-types/net/description.tex:\item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM. > > > > > > > > can TSO/UFO/USO work with VIRTIO_NET_F_GUEST_FULL_CSUM as opposed to VIRTIO_NET_F_GUEST_CSUM? > > Both GUEST_FULL_CSUM and GUEST_CSUM can work with GUEST_TSO/USO/UFO. Yes. For software devices I guess it will have a lot of performance penalty. So it should be disabled by default anyhow. The idea is to delay the csum as late as possible. > Their important difference is that if GUEST_CSUM is negotiated, the > driver can handle partial checksum. > > > > > > >> @@ -390,6 +393,13 @@ \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, > > > > So we need to change definition of VIRTIO_NET_F_GUEST_CSUM then. > > > > > > Also this is not exactly right. As defined driver must be able to handle > > partial checksum too. > > > > > > How about this: > > > > - change definition above to just "Driver handles packets with full checksum." > > > > - if VIRTIO_NET_F_GUEST_FULL_CSUM is set but VIRTIO_NET_F_GUEST_CSUM is > > clear driver requires full checksum > > > > - if VIRTIO_NET_F_GUEST_FULL_CSUM is clear but VIRTIO_NET_F_GUEST_CSUM is > > set driver supports partial checksum > > > > - if VIRTIO_NET_F_GUEST_FULL_CSUM and VIRTIO_NET_F_GUEST_CSUM are > > set then the behavior is as you describe: VIRTIO_NET_F_GUEST_CSUM > > takes preference, but you can disable it with VIRTIO_NET_F_CTRL_GUEST_OFFLOADS > > if that is supported. > > Jason wanted this feature to be enabled only when XDP is loading, > and this is the context in which this patch was proposed. > > How do you pay attention to this? I don't see any conflict, or anything I miss? Thanks
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]