On Fri, 12 Sept 2025 at 11:58, Ilya Maximets <[email protected]> wrote: > > 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.
I don't understand why the Intel helper has slipped in the driver.. At the time TSO was added, the ip checksum and pseudo checksum were done explicitly in the driver itself. The reason for switching to the Intel helper is not that clear.. and it was not a good choice seeing how this helper has evolved over time. I also think it would be cleaner to have dedicated code in net/virtio (or introduce a helper shared with lib/vhost). In fact, I had taken the approach first of fixing the driver, but as you summarize below: > > 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. Yes, this is why I preferred working on fixing OVS first. I'll put the driver cleanup in my todolist and see when I can finish it. > > Applied and backported down to 3.3. Thanks! Thank you. -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
