[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum
On Thu, Jan 11 2024, Heng Qi <hengqi@linux.alibaba.com> wrote: > å 2024/1/4 äå12:47, Heng Qi åé: >> >> >> å 2024/1/3 äå2:35, Heng Qi åé: >>> 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 Michael, Cornelia. >> >> Could you please open a voting for this fix? > > Hi Michael and Cornelia. > > Sincerely asking if you missed this message? I've been back from end-of-year PTO only this week... I'm currently waiting for a confirmed updated schedule for the infrastructure migration, as I don't want to risk having the platform out of order in the middle of the voting period. > > In addition, Jason recently responded with his reviewed-by tag. > Do I need to publish a new version carrying his tag (thanks Jason)? No, we can add his R-b when pushing to git.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]