Re: [ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
On Fri, Aug 25, 2023 at 3:46 PM Ilya Maximets 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
> > ---
> > 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(>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
Re: [ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
On 25 Aug 2023, at 15:46, Ilya Maximets 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
>> ---
>> 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(>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.
>
>> +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 did not test this with TSO other than the make check’s. Mike can you verify,
have any comments on this change?
//Eelco
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
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
> ---
> 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(>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.
> +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 ?
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
On 24 Aug 2023, at 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
Thanks for the patch, this one looks good to me, and tested it in combination
with the ‘dpdk test patch series’.
Acked-by: Eelco Chaudron
> ---
> 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(>up));
> +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;
> +}
> +
> if (info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM) {
> dev->hw_ol_features |= NETDEV_TX_IPV4_CKSUM_OFFLOAD;
> } else {
> --
> 2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] netdev-dpdk: Disable net/tap Tx L4 checksum offloads.
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
---
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(>up));
+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;
+}
+
if (info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM) {
dev->hw_ol_features |= NETDEV_TX_IPV4_CKSUM_OFFLOAD;
} else {
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
