On Fri, Aug 25, 2023 at 3:46 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
>
> On 8/24/23 17:19, David Marchand wrote:
> > As reported by Ales when doing some OVN integration tests with OVS 3.2,
> > net/tap has broken L4 checksum offloads.
> >
> > Fixes are pending on DPDK side.
> > Until they get in a LTS release used by OVS, disable those Tx offloads.
> >
> > Signed-off-by: David Marchand <david.march...@redhat.com>
> > ---
> > lib/netdev-dpdk.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 8f1361e21f..fc7225cba1 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1312,6 +1312,14 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> > dev->hw_ol_features &= ~NETDEV_RX_HW_SCATTER;
> > }
> >
> > + if (!strcmp(info.driver_name, "net_tap")) {
> > + VLOG_WARN("%s: disabled Tx L4 checksum offloads for a net/tap
> > port.",
> > + netdev_get_name(&dev->up));
>
> Maybe convert this one to INFO? I'm not sure we need to warn users every
> time the tap interface is reconfigured. It's not a high performance port
> anyway.
>
> Also, would be nice to have an XXX/FIXME comment here explaining the
> situation, so we do not forget to remove this hack in the future.
Ok.
>
> > + info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_UDP_CKSUM;
> > + info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
> > + info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_TCP_TSO;
>
> Did someone test this with userspace-tso enabled? I mean, do we need to
> backport this to 3.1 as well? Or even maybe 2.17 ?
I had a try with TSO enabled (virtio-net -> vhost-user -> ovs -> tap).
Without the patch, a tcp connection does not get established, since
TCP checksums are KO and we don't get to TSO packets.
I then tested by only filtering TCP/UDP checksum offloading.
Keeping TSO feature for net/tap works fine and TSO'd packets have
valid checksums...
Which (kind of) makes sense, when you look at the bug in the net/tap
driver: it was expecting l4_len to compute l4 checksum, and OVS sets
it when for TSO is requested.
I can update the patch and only filter RTE_ETH_TX_OFFLOAD_UDP_CKSUM
and RTE_ETH_TX_OFFLOAD_TCP_CKSUM.
As for earlier OVS versions, the TSO feature is experimental.
The code changed a bit when compared to 3.2.
I am not sure it is worth spending time fixing in a corner case like net/tap.
--
David Marchand
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
