Add the return code CT_REOPENED for conntrack table lookup.

When a tcp SYN packet hits a conntrack entry that is in close-wait state, the entry timeout will be modified and the entry will be reopened. But because the return code is CT_ESTABLISHED, policy-verdict event will not be generated even this is a new connection. The patch fix the issue by returning the code CT_REOPENED instead in this case. Except for policy verdict events, all other handlings are the same as if CT_ESTABLISHED is returned. Signed-off-by: Zang Li <zangli@google.com>
cilium · Oct 6, 2020 · ada53a9 · ada53a9
1 parent e505dad
commit ada53a9
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 5 deletions.
diff --git a/bpf/bpf_lxc.c b/bpf/bpf_lxc.c
@@ -46,7 +46,7 @@
 static __always_inline bool redirect_to_proxy(int verdict, __u8 dir)
 {
 	return is_defined(ENABLE_HOST_REDIRECT) && verdict > 0 &&
-	       (dir == CT_NEW || dir == CT_ESTABLISHED);
+	       (dir == CT_NEW || dir == CT_ESTABLISHED ||  dir == CT_REOPENED);
 }
 #endif
 
@@ -201,6 +201,10 @@ static __always_inline int ipv6_l3_from_lxc(struct __ctx_buff *ctx,
 		monitor = TRACE_PAYLOAD_LEN;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *dstID, tuple->dport,
+					   tuple->nexthdr, POLICY_EGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 		/* Did we end up at a stale non-service entry? Recreate if so. */
 		if (unlikely(ct_state.rev_nat_index != ct_state_new.rev_nat_index))
@@ -580,6 +584,10 @@ static __always_inline int handle_ipv4_from_lxc(struct __ctx_buff *ctx,
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *dstID, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 		/* Did we end up at a stale non-service entry? Recreate if so. */
 		if (unlikely(ct_state.rev_nat_index != ct_state_new.rev_nat_index))
@@ -946,10 +954,13 @@ ipv6_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, __u8 *reason,
 	if (skip_ingress_proxy)
 		verdict = 0;
 
-	if (ret == CT_NEW) {
+	if (ret == CT_NEW || ret == CT_REOPENED) {
 		send_policy_verdict_notify(ctx, src_label, tuple.dport,
 					   tuple.nexthdr, POLICY_INGRESS, 1,
 					   verdict, policy_match_type, audited);
+	}
+
+	if (ret == CT_NEW) {
 #ifdef ENABLE_DSR
 	{
 		bool dsr = false;
@@ -1181,10 +1192,13 @@ ipv4_policy(struct __ctx_buff *ctx, int ifindex, __u32 src_label, __u8 *reason,
 	if (skip_ingress_proxy)
 		verdict = 0;
 
-	if (ret == CT_NEW) {
+	if (ret == CT_NEW || ret == CT_REOPENED) {
 		send_policy_verdict_notify(ctx, src_label, tuple.dport,
 					   tuple.nexthdr, POLICY_INGRESS, 0,
 					   verdict, policy_match_type, audited);
+	}
+
+	if (ret == CT_NEW) {
 #ifdef ENABLE_DSR
 	{
 		bool dsr = false;

diff --git a/bpf/lib/common.h b/bpf/lib/common.h
@@ -546,6 +546,7 @@ enum {
 	CT_ESTABLISHED,
 	CT_REPLY,
 	CT_RELATED,
+	CT_REOPENED,
 };
 
 /* Service flags (lb{4,6}_service->flags) */

diff --git a/bpf/lib/conntrack.h b/bpf/lib/conntrack.h
@@ -245,6 +245,7 @@ static __always_inline __u8 __ct_lookup(const void *map, struct __ctx_buff *ctx,
 			if (unlikely(reopen == (TCP_FLAG_SYN|0x1))) {
 				ct_reset_closing(entry);
 				*monitor = ct_update_timeout(entry, is_tcp, dir, seen_flags);
+				return CT_REOPENED;
 			}
 			break;
 		case ACTION_CLOSE:
@@ -435,7 +436,7 @@ static __always_inline int ct_lookup6(const void *map,
 	ret = __ct_lookup(map, ctx, tuple, action, dir, ct_state, is_tcp,
 			  tcp_flags, monitor);
 	if (ret != CT_NEW) {
-		if (likely(ret == CT_ESTABLISHED)) {
+		if (likely(ret == CT_ESTABLISHED || ret == CT_REOPENED)) {
 			if (unlikely(tuple->flags & TUPLE_F_RELATED))
 				ret = CT_RELATED;
 			else
@@ -647,7 +648,7 @@ static __always_inline int ct_lookup4(const void *map,
 	ret = __ct_lookup(map, ctx, tuple, action, dir, ct_state, is_tcp,
 			  tcp_flags, monitor);
 	if (ret != CT_NEW) {
-		if (likely(ret == CT_ESTABLISHED)) {
+		if (likely(ret == CT_ESTABLISHED || ret == CT_REOPENED)) {
 			if (unlikely(tuple->flags & TUPLE_F_RELATED))
 				ret = CT_RELATED;
 			else

diff --git a/bpf/lib/host_firewall.h b/bpf/lib/host_firewall.h
@@ -79,6 +79,10 @@ ipv6_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id)
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, dst_id, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -167,6 +171,10 @@ ipv6_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id)
 		if (IS_ERR(ret))
 			return ret;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *src_id, tuple.dport,
+					   tuple.nexthdr, POLICY_INGRESS, 1,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -298,6 +306,10 @@ ipv4_host_policy_egress(struct __ctx_buff *ctx, __u32 src_id,
 			return ret;
 		break;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, dst_id, tuple.dport,
+					   tuple.nexthdr, POLICY_EGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -389,6 +401,10 @@ ipv4_host_policy_ingress(struct __ctx_buff *ctx, __u32 *src_id)
 		if (IS_ERR(ret))
 			return ret;
 
+	case CT_REOPENED:
+		send_policy_verdict_notify(ctx, *src_id, tuple.dport,
+					   tuple.nexthdr, POLICY_INGRESS, 0,
+					   verdict, policy_match_type, audited);
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:

diff --git a/bpf/lib/lb.h b/bpf/lib/lb.h
@@ -809,6 +809,7 @@ static __always_inline int lb6_local(const void *map, struct __ctx_buff *ctx,
 		if (IS_ERR(ret))
 			goto drop_no_service;
 		goto update_state;
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:
@@ -1343,6 +1344,7 @@ static __always_inline int lb4_local(const void *map, struct __ctx_buff *ctx,
 		if (IS_ERR(ret))
 			goto drop_no_service;
 		goto update_state;
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_RELATED:
 	case CT_REPLY:

diff --git a/bpf/lib/nodeport.h b/bpf/lib/nodeport.h
@@ -601,6 +601,7 @@ static __always_inline int nodeport_lb6(struct __ctx_buff *ctx,
 		}
 		break;
 
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_REPLY:
 		if (unlikely(ct_state.rev_nat_index != svc->rev_nat_index))
@@ -1296,6 +1297,7 @@ static __always_inline int nodeport_lb4(struct __ctx_buff *ctx,
 		}
 		break;
 
+	case CT_REOPENED:
 	case CT_ESTABLISHED:
 	case CT_REPLY:
 		/* Recreate CT entries, as the existing one is stale and belongs

diff --git a/bpf/lib/trace.h b/bpf/lib/trace.h
@@ -40,6 +40,7 @@ enum {
 	TRACE_REASON_CT_ESTABLISHED = CT_ESTABLISHED,
 	TRACE_REASON_CT_REPLY = CT_REPLY,
 	TRACE_REASON_CT_RELATED = CT_RELATED,
+	TRACE_REASON_CT_REOPENED = CT_REOPENED,
 };
 
 #define TRACE_REASON_ENCRYPTED	    0x80

diff --git a/bpf/tests/conntrack_test.h b/bpf/tests/conntrack_test.h
@@ -166,6 +166,16 @@ static void test___ct_lookup(void)
 	assert(monitor == 0);
 	assert(timeout_in(entry, CT_CLOSE_TIMEOUT - 1));
 
+	/* A connection is reopened due to a newly seen SYN.*/
+	advance_time();
+	monitor = 0;
+	seen_flags.value = TCP_FLAG_SYN;
+	res = __ct_lookup(map, &ctx, tuple, ACTION_CREATE, CT_EGRESS,
+			  &ct_state, true, seen_flags, &monitor);
+	assert(res == CT_REOPENED);
+	assert(monitor == TRACE_PAYLOAD_LEN);
+	assert(timeout_in(entry, CT_CONNECTION_LIFETIME_TCP));
+
 	/* Label connection as new if the tuple wasn't previously tracked */
 	tuple = (void *)__TUPLE_NOEXIST;
 	seen_flags.value = TCP_FLAG_SYN;

diff --git a/pkg/monitor/datapath_trace.go b/pkg/monitor/datapath_trace.go
@@ -90,13 +90,15 @@ const (
 	TraceReasonCtEstablished
 	TraceReasonCtReply
 	TraceReasonCtRelated
+	TraceReasonCtReopened
 )
 
 var traceReasons = map[uint8]string{
 	TraceReasonPolicy:        "new",
 	TraceReasonCtEstablished: "established",
 	TraceReasonCtReply:       "reply",
 	TraceReasonCtRelated:     "related",
+	TraceReasonCtReopened:    "reopened",
 }
 
 func connState(reason uint8) string {