On Thu, Feb 15, 2024 at 4:59 PM Mike Pattrick <[email protected]> wrote:
>
> Previously some packets were excluded from the tunnel mark if they
> weren't L4. However, this causes problems with multi encapsulated
> packets like arp.
>
> Due to these flags being set, additional checks are required in checksum
> modification code.
>
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> Reported-by: David Marchand <[email protected]>
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> lib/dp-packet.h | 19 +++++++++++++++++--
> lib/netdev-native-tnl.c | 10 ++++++++--
> lib/packets.c | 8 ++++----
> 3 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 770ddc1b9..2fa17d814 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -1386,8 +1386,8 @@ dp_packet_ip_checksum_bad(const struct dp_packet *p)
>
> /* Return 'true' is packet 'b' is not encapsulated and is marked for IPv4
> * checksum offload, or if 'b' is encapsulated and the outer layer is marked
> - * for IPv4 checksum offload. IPv6 packets and non offloaded packets return
> - * 'false'. */
> + * for IPv4 checksum offload. IPv6 packets, non offloaded packets, and IPv4
> + * packets that are marked as good return 'false'. */
> static inline bool
> dp_packet_hwol_l3_csum_ipv4_ol(const struct dp_packet *b)
> {
> @@ -1400,6 +1400,21 @@ dp_packet_hwol_l3_csum_ipv4_ol(const struct dp_packet
> *b)
> return false;
> }
>
> +/* Return 'true' is packet 'b' is not encapsulated and is marked for IPv4
> + * checksum offload, or if 'b' is encapsulated and the outer layer is marked
> + * for IPv4 checksum offload. IPv6 packets and non offloaded packets return
> + * 'false'. */
> +static inline bool
> +dp_packet_hwol_l3_ipv4(const struct dp_packet *b)
> +{
> + if (dp_packet_hwol_is_outer_ipv4(b)) {
> + return true;
> + } else if (!dp_packet_hwol_is_outer_ipv6(b)) {
> + return dp_packet_hwol_tx_ip_csum(b);
> + }
> + return false;
> +}
> +
> /* Calculate and set the IPv4 header checksum in packet 'p'. */
> static inline void
> dp_packet_ip_set_header_csum(struct dp_packet *p, bool inner)
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index 0d6d803fe..dee9ab344 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -91,8 +91,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet,
> struct flow_tnl *tnl,
>
> /* A packet coming from a network device might have the
> * csum already checked. In this case, skip the check. */
> - if (OVS_UNLIKELY(!dp_packet_ip_checksum_good(packet))
> - && !dp_packet_hwol_tx_ip_csum(packet)) {
> + if (OVS_UNLIKELY(!dp_packet_hwol_l3_csum_ipv4_ol(packet))) {
> if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum");
> return NULL;
> @@ -299,6 +298,13 @@ dp_packet_tnl_ol_process(struct dp_packet *packet,
> (char *) dp_packet_eth(packet) +
> VXLAN_HLEN);
> }
> + } else {
> + /* Mark non-l4 packets as tunneled. */
> + if (data->tnl_type == OVS_VPORT_TYPE_GENEVE) {
> + dp_packet_hwol_set_tunnel_geneve(packet);
> + } else if (data->tnl_type == OVS_VPORT_TYPE_VXLAN) {
> + dp_packet_hwol_set_tunnel_vxlan(packet);
> + }
> }
> }
>
> diff --git a/lib/packets.c b/lib/packets.c
> index 36c6692e5..ed185b4ec 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -1149,7 +1149,7 @@ packet_set_ipv4_addr(struct dp_packet *packet,
> }
> }
>
> - if (dp_packet_hwol_tx_ip_csum(packet)) {
> + if (dp_packet_hwol_l3_ipv4(packet)) {
> dp_packet_ol_reset_ip_csum_good(packet);
> } else {
> nh->ip_csum = recalc_csum32(nh->ip_csum, old_addr, new_addr);
> @@ -1328,7 +1328,7 @@ packet_set_ipv4(struct dp_packet *packet, ovs_be32 src,
> ovs_be32 dst,
> if (nh->ip_tos != tos) {
> uint8_t *field = &nh->ip_tos;
>
> - if (dp_packet_hwol_tx_ip_csum(packet)) {
> + if (dp_packet_hwol_l3_ipv4(packet)) {
> dp_packet_ol_reset_ip_csum_good(packet);
> } else {
> nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t)
> *field),
> @@ -1341,7 +1341,7 @@ packet_set_ipv4(struct dp_packet *packet, ovs_be32 src,
> ovs_be32 dst,
> if (nh->ip_ttl != ttl) {
> uint8_t *field = &nh->ip_ttl;
>
> - if (dp_packet_hwol_tx_ip_csum(packet)) {
> + if (dp_packet_hwol_l3_ipv4(packet)) {
> dp_packet_ol_reset_ip_csum_good(packet);
> } else {
> nh->ip_csum = recalc_csum16(nh->ip_csum, htons(*field << 8),
> @@ -1979,7 +1979,7 @@ IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6)
>
> tos |= IP_ECN_CE;
> if (nh->ip_tos != tos) {
> - if (dp_packet_hwol_tx_ip_csum(pkt)) {
> + if (dp_packet_hwol_l3_ipv4(packet)) {
Will resubmit with this change corrected.
-M
> dp_packet_ol_reset_ip_csum_good(pkt);
> } else {
> nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_tos),
> --
> 2.39.3
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
