We tested the patch and it works. After NATed, the packet already carries the CS_SRC_NAT or CS_DST_NAT flag, there is no need to check nat_action_info, and the original check logic is incorrect. Reviewed-by: wangze <[email protected]>
<[email protected]> 于2021年8月6日周五 下午4:38写道： > > From: wenxu <[email protected]> > > 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 <[email protected]> > --- > 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 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
