On 6/14/23 21:03, Mike Pattrick wrote:
> The netdev receiving packets is supposed to provide the flags
> indicating if the L4 checksum was verified and it is OK or BAD,
> otherwise the stack will check when appropriate by software.
>
> If the packet comes with good checksum, then postpone the
> checksum calculation to the egress device if needed.
>
> When encapsulate a packet with that flag, set the checksum
> of the inner L4 header since that is not yet supported.
>
> Calculate the L4 checksum when the packet is going to be sent
> over a device that doesn't support the feature.
>
> Linux tap devices allows enabling L3 and L4 offload, so this
> patch enables the feature. However, Linux socket interface
> remains disabled because the API doesn't allow enabling
> those two features without enabling TSO too.
>
> Signed-off-by: Flavio Leitner <f...@sysclose.org>
> Co-authored-by: Flavio Leitner <f...@sysclose.org>
> Signed-off-by: Mike Pattrick <m...@redhat.com>
> ---
> Since v9:
> - Extended miniflow_extract changes into avx512 code
> - Formatting changes
> - Note that we cannot currently enable checksum offloading in
> CONFIGURE_VETH_OFFLOADS for check-system-userspace as
> netdev-linux.c currently only parses the vnet header if TSO
> is enabled.
> Since v10:
> - No change
> Since v11:
> - Added AVX512 IPv6 checksum offload support.
> - Improved error messages and logging.
> Since v12:
> - Added missing mutex annotations
> Since v13:
> - Added TUNGETFEATURES check in netdev-linux
> Since v14:
> - Only check TUNGETFEATURES once
> - Respect FLOW_TNL_F_CSUM flag
> ---
> lib/conntrack.c | 15 +-
> lib/dp-packet.c | 25 +++
> lib/dp-packet.h | 78 +++++++++-
> lib/dpif-netdev-extract-avx512.c | 62 +++++++-
> lib/flow.c | 23 +++
> lib/netdev-dpdk.c | 172 +++++++++++++++------
> lib/netdev-linux.c | 258 ++++++++++++++++++++++---------
> lib/netdev-native-tnl.c | 32 +---
> lib/netdev.c | 46 ++----
> lib/odp-execute-avx512.c | 88 +++++++----
> lib/packets.c | 175 ++++++++++++++++-----
> lib/packets.h | 3 +
> 12 files changed, 710 insertions(+), 267 deletions(-)
<snip>
> @@ -5443,22 +5513,22 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
> *netdev)
> }
>
> if (userspace_tso_enabled()) {
> - netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
> - netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CKSUM;
> - netdev->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CKSUM;
> - netdev->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CKSUM;
> - netdev->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CKSUM;
> - vhost_unsup_flags = 1ULL << VIRTIO_NET_F_HOST_ECN
> - | 1ULL << VIRTIO_NET_F_HOST_UFO;
> + virtio_unsup_features = 1ULL << VIRTIO_NET_F_HOST_ECN
> + | 1ULL << VIRTIO_NET_F_HOST_UFO;
> + VLOG_DBG("%s: TSO enabled on vhost port",
> + netdev_get_name(&dev->up));
> } else {
> - /* This disables checksum offloading and all the features
> - * that depends on it (TSO, UFO, ECN) according to virtio
> - * specification. */
> - vhost_unsup_flags = 1ULL << VIRTIO_NET_F_CSUM;
> + /* Advertise checksum offloading to the guest, but explicitly
> + * disable TSO and friends.
> + * NOTE: we can't disable HOST_ECN which may have been wrongly
> + * negotiated by a running guest. */
> + virtio_unsup_features = 1ULL << VIRTIO_NET_F_HOST_TSO4
> + | 1ULL << VIRTIO_NET_F_HOST_TSO6
> + | 1ULL << VIRTIO_NET_F_HOST_UFO;
Hold on a second... Why exactly we can disable UFO, but can't disable ECN ?
Previously, this branch of code was disabling VIRTIO_NET_F_CSUM, so neither
ECN or UFO should be negotiated, right?
Or is it possible to have ECN negotiated without VIRTIO_NET_F_HOST_CSUM enabled?
In that case, why the same is not true for UFO as well?
What do I miss here?
I see that this was suggested by David in review of v6, but I can't find
discussion about this particular part.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
