On 6/21/23 00:14, Ilya Maximets wrote:
> 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?
Also, disabling TSO4/6 should implicitly disable ECN as well.
So, I'm confused by the 'NOTE'.
>
> 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
