Formerly when userspace TSO was enabled but with a non-DKDK interface
without support IP checksum offloading, FTP NAT connections would fail
if the packet length changed. This can happen if the packets length
changes during L7 NAT translation, predominantly with FTP. This is why
the "conntrack - IPv4 FTP Passive with DNAT" test would fail when run
when run with check-system-tso.

However, that isn't the only location with this type of issue. Any time
that we had checked for dp_packet_hwol_is_ipv4() there was a possibility
that the packet was either destined for an interface with TCP but not IP
offload, or originating from an interface with any level of offload but
destined for an interface with no checksum offload at all.

A patch series for a more robust solution is in progress.

Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
Reported-by: Ilya Maximets <i.maxim...@ovn.org>
Signed-off-by: Mike Pattrick <m...@redhat.com>
---
 lib/conntrack.c |  8 +++-----
 lib/ipf.c       | 36 ++++++++++++++++--------------------
 2 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 33a1a9295..dfb2606a6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3402,11 +3402,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
conn_lookup_ctx *ctx,
                 }
                 if (seq_skew) {
                     ip_len = ntohs(l3_hdr->ip_tot_len) + seq_skew;
-                    if (!dp_packet_hwol_is_ipv4(pkt)) {
-                        l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
-                                                        l3_hdr->ip_tot_len,
-                                                        htons(ip_len));
-                    }
+                    l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
+                                                    l3_hdr->ip_tot_len,
+                                                    htons(ip_len));
                     l3_hdr->ip_tot_len = htons(ip_len);
                 }
             }
diff --git a/lib/ipf.c b/lib/ipf.c
index 507db2aea..e1dee5907 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -433,11 +433,9 @@ ipf_reassemble_v4_frags(struct ipf_list *ipf_list)
     len += rest_len;
     l3 = dp_packet_l3(pkt);
     ovs_be16 new_ip_frag_off = l3->ip_frag_off & ~htons(IP_MORE_FRAGMENTS);
-    if (!dp_packet_hwol_is_ipv4(pkt)) {
-        l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_frag_off,
-                                    new_ip_frag_off);
-        l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_tot_len, htons(len));
-    }
+    l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_frag_off,
+                                new_ip_frag_off);
+    l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_tot_len, htons(len));
     l3->ip_tot_len = htons(len);
     l3->ip_frag_off = new_ip_frag_off;
     dp_packet_set_l2_pad_size(pkt, 0);
@@ -1185,21 +1183,19 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
                     } else {
                         struct ip_header *l3_frag = dp_packet_l3(frag_i->pkt);
                         struct ip_header *l3_reass = dp_packet_l3(pkt);
-                        if (!dp_packet_hwol_is_ipv4(frag_i->pkt)) {
-                            ovs_be32 reass_ip =
-                                get_16aligned_be32(&l3_reass->ip_src);
-                            ovs_be32 frag_ip =
-                                get_16aligned_be32(&l3_frag->ip_src);
-
-                            l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
-                                                             frag_ip,
-                                                             reass_ip);
-                            reass_ip = get_16aligned_be32(&l3_reass->ip_dst);
-                            frag_ip = get_16aligned_be32(&l3_frag->ip_dst);
-                            l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
-                                                             frag_ip,
-                                                             reass_ip);
-                        }
+                        ovs_be32 reass_ip =
+                            get_16aligned_be32(&l3_reass->ip_src);
+                        ovs_be32 frag_ip =
+                            get_16aligned_be32(&l3_frag->ip_src);
+
+                        l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
+                                                         frag_ip,
+                                                         reass_ip);
+                        reass_ip = get_16aligned_be32(&l3_reass->ip_dst);
+                        frag_ip = get_16aligned_be32(&l3_frag->ip_dst);
+                        l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
+                                                         frag_ip,
+                                                         reass_ip);
 
                         l3_frag->ip_src = l3_reass->ip_src;
                         l3_frag->ip_dst = l3_reass->ip_dst;
-- 
2.27.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to