diff mbox series

[ovs-dev,v3,6/6] conntrack: Fix L4 csum for V6 extension hdr pkts.

Message ID 1550679439-95810-6-git-send-email-dlu998@gmail.com
State Changes Requested
Headers show
Series [ovs-dev,v3,1/6] ipf: Misc Cleanup. | expand

Commit Message

Darrell Ball Feb. 20, 2019, 4:17 p.m. UTC
It is a day one issue that got copied to the FTP handling code.

Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
CC: Daniele Di Proietto <diproiettod@ovn.org>
Signed-off-by: Darrell Ball <dlu998@gmail.com>
---

v3: New patch.

 lib/conntrack.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Comments

Ben Pfaff Feb. 22, 2019, 10:18 p.m. UTC | #1
On Wed, Feb 20, 2019 at 08:17:19AM -0800, Darrell Ball wrote:
> It is a day one issue that got copied to the FTP handling code.
> 
> Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
> Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> CC: Daniele Di Proietto <diproiettod@ovn.org>
> Signed-off-by: Darrell Ball <dlu998@gmail.com>

Thanks for working on this!

There's something goofy with the types here:

../lib/conntrack.c:1598:14: error: incorrect type in assignment (different base types)
../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned] [usertype] csum
../lib/conntrack.c:1598:14:    got restricted ovs_be16
../lib/conntrack.c:1598:14: error: incorrect type in assignment (different base types)
../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned] [usertype] csum
../lib/conntrack.c:1598:14:    got restricted ovs_be16
../lib/conntrack.c:1598:14: error: incorrect type in assignment (different base types)
../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned] [usertype] csum
../lib/conntrack.c:1598:14:    got restricted ovs_be16
../lib/conntrack.c:3264:22: error: incorrect type in assignment (different base types)
../lib/conntrack.c:3264:22:    expected restricted ovs_be16 [usertype] tcp_csum
../lib/conntrack.c:3264:22:    got unsigned short
../lib/conntrack.c:690:39: error: incorrect type in assignment (different base types)
../lib/conntrack.c:690:39:    expected restricted ovs_be16 [usertype] icmp6_cksum
../lib/conntrack.c:690:39:    got unsigned short

Thanks,

Ben.
Darrell Ball Feb. 22, 2019, 10:34 p.m. UTC | #2
oops, thanks for pointing that out

I'll fix and resend

Darrell

On Fri, Feb 22, 2019 at 2:18 PM Ben Pfaff <blp@ovn.org> wrote:

> On Wed, Feb 20, 2019 at 08:17:19AM -0800, Darrell Ball wrote:
> > It is a day one issue that got copied to the FTP handling code.
> >
> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
> > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> > CC: Daniele Di Proietto <diproiettod@ovn.org>
> > Signed-off-by: Darrell Ball <dlu998@gmail.com>
>
> Thanks for working on this!
>
> There's something goofy with the types here:
>
> ../lib/conntrack.c:1598:14: error: incorrect type in assignment (different
> base types)
> ../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned]
> [usertype] csum
> ../lib/conntrack.c:1598:14:    got restricted ovs_be16
> ../lib/conntrack.c:1598:14: error: incorrect type in assignment (different
> base types)
> ../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned]
> [usertype] csum
> ../lib/conntrack.c:1598:14:    got restricted ovs_be16
> ../lib/conntrack.c:1598:14: error: incorrect type in assignment (different
> base types)
> ../lib/conntrack.c:1598:14:    expected unsigned int [unsigned] [assigned]
> [usertype] csum
> ../lib/conntrack.c:1598:14:    got restricted ovs_be16
> ../lib/conntrack.c:3264:22: error: incorrect type in assignment (different
> base types)
> ../lib/conntrack.c:3264:22:    expected restricted ovs_be16 [usertype]
> tcp_csum
> ../lib/conntrack.c:3264:22:    got unsigned short
> ../lib/conntrack.c:690:39: error: incorrect type in assignment (different
> base types)
> ../lib/conntrack.c:690:39:    expected restricted ovs_be16 [usertype]
> icmp6_cksum
> ../lib/conntrack.c:690:39:    got unsigned short
>
> Thanks,
>
> Ben.
>
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4d76552..d605382 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -686,10 +686,10 @@  reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
                                  &conn->key.dst.addr.ipv6, true);
         }
         reverse_pat_packet(pkt, conn);
-        uint32_t icmp6_csum = packet_csum_pseudoheader6(nh6);
         icmp6->icmp6_base.icmp6_cksum = 0;
-        icmp6->icmp6_base.icmp6_cksum = csum_finish(
-            csum_continue(icmp6_csum, icmp6, tail - (char *) icmp6 - pad));
+        icmp6->icmp6_base.icmp6_cksum = packet_csum_upperlayer6(
+                                            nh6, icmp6, IPPROTO_ICMPV6,
+                                            tail - (char *) icmp6 - pad);
     }
     pkt->l3_ofs = orig_l3_ofs;
     pkt->l4_ofs = orig_l4_ofs;
@@ -1595,15 +1595,14 @@  checksum_valid(const struct conn_key *key, const void *data, size_t size,
 
     if (key->dl_type == htons(ETH_TYPE_IP)) {
         csum = packet_csum_pseudoheader(l3);
+        csum = csum_finish(csum_continue(csum, data, size));
     } else if (key->dl_type == htons(ETH_TYPE_IPV6)) {
-        csum = packet_csum_pseudoheader6(l3);
+        csum = packet_csum_upperlayer6(l3, data, key->nw_proto, size);
     } else {
         return false;
     }
 
-    csum = csum_continue(csum, data, size);
-
-    return csum_finish(csum) == 0;
+    return csum == 0;
 }
 
 static inline bool
@@ -3261,16 +3260,14 @@  handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
     }
 
     th->tcp_csum = 0;
-    uint32_t tcp_csum;
     if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
-        tcp_csum = packet_csum_pseudoheader6(nh6);
+        th->tcp_csum = packet_csum_upperlayer6(nh6, th, ctx->key.nw_proto,
+                                               dp_packet_l4_size(pkt));
     } else {
-        tcp_csum = packet_csum_pseudoheader(l3_hdr);
+        uint32_t tcp_csum = packet_csum_pseudoheader(l3_hdr);
+        th->tcp_csum = csum_finish(
+             csum_continue(tcp_csum, th, dp_packet_l4_size(pkt)));
     }
-    const char *tail = dp_packet_tail(pkt);
-    uint8_t pad = dp_packet_l2_pad_size(pkt);
-    th->tcp_csum = csum_finish(
-        csum_continue(tcp_csum, th, tail - (char *) th - pad));
 
     if (seq_skew) {
         conn_seq_skew_set(ct, &ec->key, now, seq_skew + ec->seq_skew,