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

[wenxu at ucloud.cn]

From: wenxu <wenxu at 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 at ucloud.cn>
---
 lib/conntrack.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

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) {
-- 
1.8.3.1