On 2/26/24 14:38, Mike Pattrick wrote:
> Some network cards support inner checksum offloading but not outer
> checksum offloading. Currently OVS will resolve that outer checksum but
> allows the network card to resolve the inner checksum, invalidating the
> outer checksum in the process.
>
> Now if we can't offload outer checksums, we don't offload inner either.
>
> Reported-at: https://issues.redhat.com/browse/FDP-363
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> Signed-off-by: Mike Pattrick
> ---
> nb: I also tested a more complex patch that only resolved the inner
> checksum and offloaded the UDP layer. This didn't noticably improve
> performance.
> v2: Added IPv4 flag
> ---
> lib/dp-packet.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 305822293..df7bf8e6b 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -592,6 +592,18 @@ dp_packet_ol_send_prepare(struct dp_packet *p, uint64_t
> flags)
> if (dp_packet_hwol_is_tunnel_geneve(p) ||
> dp_packet_hwol_is_tunnel_vxlan(p)) {
> tnl_inner = true;
> +
> +/* If the TX interface doesn't support UDP tunnel offload but does
> + * support inner checksum offload and an outer UDP checksum is
> + * required, then we can't offload inner checksum either. As that
> would
> + * invalidate the outer checksum. */
> +if (!(flags & NETDEV_TX_OFFLOAD_OUTER_UDP_CKSUM) &&
> +dp_packet_hwol_is_outer_udp_cksum(p)) {
> +flags &= ~(NETDEV_TX_OFFLOAD_TCP_CKSUM |
> + NETDEV_TX_OFFLOAD_UDP_CKSUM |
> + NETDEV_TX_OFFLOAD_SCTP_CKSUM |
> + NETDEV_TX_OFFLOAD_IPV4_CKSUM);
Thanks, Mike. Applied and backported to 3.3.
Though I still think the logic of offload flags is backwards.
Unfortunately that is derivative of the kernel and DPDK APIs.
Maybe we can try to flip it in OVS? I think it might be good
if we could make all the basic flags always mean offloads of
the outermost headers and have another set of inner flags.
Maybe we could bitshift them on encapsulations/decapsulations.
This will require flag translation on receive and send for
each particular netdev type. But maybe it will be more clear
to implement OVS logic this way, as long as it doesn't give a
huge performance hit. I'd happily sacrifice a couple percents
of performance for a clearer internal logic.
Any other ideas are also welcome. But the fact that basic flags
mean innermost header, except for cases where they do not,
doesn't make a lot of sense to me and we'll keep hitting issues
like this one because of that.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
