diff mbox series

[ovs-dev,v2] conntrack: fix incorrect check nat_action_info in check_orig_tuple

Message ID 1625809032-436-1-git-send-email-wenxu@ucloud.cn
State New
Headers show
Series [ovs-dev,v2] conntrack: fix incorrect check nat_action_info in check_orig_tuple | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot success github build: passed

Commit Message

wenxu July 9, 2021, 5:37 a.m. UTC 
From: wenxu <wenxu@ucloud.cn>

A case for client A 10.0.0.2 snat to 1.1.1.2 with following flows.

rule1: ovs-ofctl add-flow manbr "table=0,ct_state=-trk,ip,in_port=dpdk2, actions=ct(table=1, nat)"
rule2: ovs-ofctl add-flow manbr "table=0,table=1,ct_state=+trk+new,ip,in_port=dpdk2, actions=ct(commit, nat(src=1.1.1.2)),dpdk3"

When client A tcp connect to a non-exist server 1.1.1.3

The first syn packet will create the following conntrack 1
But the second syn packet will wrongly create the conntrack 2

tcp,orig=(src=10.0.0.2,dst=1.1.1.3,sport=15690,dport=5001),reply=(src=1.1.1.3,dst=1.1.1.2,sport=5001,dport=15690),protoinfo=(state=SYN_SENT)   #conntrack 1
tcp,orig=(src=1.1.1.2,dst=1.1.1.3,sport=15690,dport=5001),reply=(src=1.1.1.3,dst=1.1.1.2,sport=5001,dport=1024),protoinfo=(state=SYN_SENT)     #conntrack 2

The second syn packet gothrough rule1 and find the conntrack1 and
do nat. Then gothrough the rule2 will not find the only conntrack
for packet nated in the rule1
The check_orig_tuple is used to fix for this situation(packet
nated in the first rule). But it should't check the nat_action_info
in the rule2. It should only check the CS_SRC_NAT and CS_DST_NAT.

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 lib/conntrack.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 2e803ca..ace3e9a 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1156,15 +1156,13 @@  handle_nat(struct dp_packet *pkt, struct conn *conn,
 static bool
 check_orig_tuple(struct conntrack *ct, struct dp_packet *pkt,
                  struct conn_lookup_ctx *ctx_in, long long now,
-                 struct conn **conn,
-                 const struct nat_action_info_t *nat_action_info)
+                 struct conn **conn)
 {
     if (!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
         (ctx_in->key.dl_type == htons(ETH_TYPE_IP) &&
          !pkt->md.ct_orig_tuple.ipv4.ipv4_proto) ||
         (ctx_in->key.dl_type == htons(ETH_TYPE_IPV6) &&
-         !pkt->md.ct_orig_tuple.ipv6.ipv6_proto) ||
-        nat_action_info) {
+         !pkt->md.ct_orig_tuple.ipv6.ipv6_proto)) {
         return false;
     }
 
@@ -1343,7 +1341,7 @@  process_one(struct conntrack *ct, struct dp_packet *pkt,
             handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related);
         }
 
-    } else if (check_orig_tuple(ct, pkt, ctx, now, &conn, nat_action_info)) {
+    } else if (check_orig_tuple(ct, pkt, ctx, now, &conn)) {
         create_new_conn = conn_update_state(ct, pkt, ctx, conn, now);
     } else {
         if (ctx->icmp_related) {