On 9/10/25 5:25 PM, David Marchand wrote: > OVS checks a netdev checksum offload capabilities before calling its > transmit helper. > Yet the OVS transmit preparation helper for DPDK drivers requests IP > checksum (setting RTE_MBUF_F_TX_IP_CSUM) regardless of the DPDK driver > support if some L4 checksum is requested. > > This is problematic since net/virtio does support L4 checksum offload, > but does not support nor announce IP checksum offload and this DPDK > driver tx prepare helper (calling helper shared with other DPDK drivers) > clears the ip checksum in packet data if RTE_MBUF_F_TX_IP_CSUM is set. > > The DPDK mbuf API mandates IP csum when TCP segmentation is requested. > However, IP csum is not required when TCP or other L4 checksum is > requested. > > While most DPDK drivers implement both IP and L4 checksums (and this is > likely why the bug was never caught), let's avoid requesting an > unsupported offload.
I'd still consider the fact that IP_CSUM is supported when together with TCP_SEG, but not supported on its own, as a bug in the driver. Also, if it is not supported, then the driver should either ignore the flag or fail the send, it shouldn't just do random things. The fact that virtio driver is using "intel" variant of the packet preparation for offloading doesn't sound correct to me. All the intel drivers support IP_CSUM offload, so it makes sense for the preparation function to clear the field. If virtio driver doesn't support IP_CSUM offload, then it should work around this case, or not use the intel variant of the preparation function. At the same time, I agree that IP_CSUM is not required by the API for L4 checksum offloads. So this change in OVS makes sense. Applied and backported down to 3.3. Thanks! Best regards, Ilya Maximets. > > Reported-at: https://issues.redhat.com/browse/FDP-1666 > Link: > https://git.dpdk.org/dpdk-stable/tree/drivers/net/virtio/virtio_rxtx.c?h=v24.11.2#n1742 > Link: https://git.dpdk.org/dpdk-stable/tree/lib/net/rte_net.h?h=v24.11.2#n179 > Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.") > Signed-off-by: David Marchand <[email protected]> > --- > lib/netdev-dpdk.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 07fca319a1..45b9f48964 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2765,12 +2765,11 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, > struct rte_mbuf *mbuf) > return false; > } > mbuf->ol_flags |= RTE_MBUF_F_TX_TCP_SEG; > - } > > - /* If L4 checksum is requested, IPv4 should be requested as well. */ > - if (mbuf->ol_flags & RTE_MBUF_F_TX_L4_MASK > - && mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) { > - mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM; > + /* DPDK API mandates IPv4 checksum when requesting TSO. */ > + if (IP_VER(ip->ip_ihl_ver) == 4) { > + mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM; > + } > } > > return true; _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
