bpf: dsr: only require DSR-info on SYN packet

The DSR ingress path on a remote node currently expects DSR-info on both
the SYN *and* the SYN-ACK. And clears the connection's DSR status if either
of those packets doesn't carry DSR info.

But it should actually be fine for the LB node to only send the DSR info
on the SYN packet. So let's relax this check accordingly, and not require
DSR info on the SYN-ACK.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>

Author: julianwiedmann

bpf/lib/l4.h

@@ -29,6 +29,12 @@ static __always_inline __u8 tcp_flags_to_u8(__be32 value)
 	return ((union tcp_flags)value).lower_bits;
 }
 
+static __always_inline bool tcp_is_syn(union tcp_flags flags)
+{
+	/* Match SYN, but not SYN-ACK. */
+	return (flags.value & (TCP_FLAG_SYN | TCP_FLAG_ACK)) == TCP_FLAG_SYN;
+}
+
 static __always_inline int
 l4_store_port(struct __ctx_buff *ctx, int l4_off, int port_off, __be16 port)
 {

bpf/lib/nodeport.h

@@ -583,7 +583,7 @@ nodeport_extract_dsr_v6(struct __ctx_buff *ctx,
 		tmp.flags = TUPLE_F_OUT;
 		__ipv6_ct_tuple_reverse(&tmp);
 
-		if (tcp_flags.value & TCP_FLAG_SYN) {
+		if (tcp_is_syn(tcp_flags)) {
 			/* SYN for a new connection that's not / no longer DSR.
 			 * If it's reopened, avoid sending subsequent traffic down the DSR path.
 			 */
@@ -1962,7 +1962,7 @@ nodeport_extract_dsr_v4(struct __ctx_buff *ctx,
 		tmp.flags = TUPLE_F_OUT;
 		__ipv4_ct_tuple_reverse(&tmp);
 
-		if (tcp_flags.value & TCP_FLAG_SYN) {
+		if (tcp_is_syn(tcp_flags)) {
 			/* SYN for a new connection that's not / no longer DSR.
 			 * If it's reopened, avoid sending subsequent traffic down the DSR path.
 			 */

bpf/tests/kpr_dsr_remote_node.h

@@ -262,6 +262,27 @@ int kpr_v4_dsr_remote_node_3synack_check(__maybe_unused const struct __ctx_buff
 			   kpr_v4_dsr_remote_node_synack,
 			   sizeof(kpr_v4_dsr_remote_node_synack));
 
+	struct ipv4_ct_tuple tuple;
+	struct ct_entry *ct_entry;
+
+	tuple.flags = TUPLE_F_IN;
+	tuple.nexthdr = IPPROTO_TCP;
+	tuple.daddr = v4_pod_one;
+	tuple.saddr = v4_ext_one;
+	tuple.sport = tcp_dst_one;
+	tuple.dport = tcp_src_one;
+	ipv4_ct_tuple_reverse(&tuple);
+
+	ct_entry = map_lookup_elem(get_ct_map4(&tuple), &tuple);
+	if (!ct_entry)
+		test_fatal("no CT entry for DSR found");
+	if (!ct_entry->dsr_internal)
+		test_fatal("CT entry doesn't have the .dsr_internal flag set");
+	if (ct_entry->nat_addr.p4 != v4_svc_one)
+		test_fatal("CT entry doesn't have the RevDNAT addr set");
+	if (ct_entry->nat_port != tcp_svc_one)
+		test_fatal("CT entry doesn't have the RevDNAT port set");
+
 	test_finish();
 }
 
@@ -619,6 +640,30 @@ int kpr_v6_dsr_remote_node_3synack_check(__maybe_unused const struct __ctx_buff
 			   kpr_v6_dsr_remote_node_synack,
 			   sizeof(kpr_v6_dsr_remote_node_synack));
 
+	struct ipv6_ct_tuple tuple __align_stack_8;
+	struct ct_entry *ct_entry;
+	union v6addr frontend_ip = { v6_svc_one_addr };
+	union v6addr backend_ip = { v6_pod_one_addr };
+	union v6addr client_ip = { v6_ext_node_one_addr };
+
+	tuple.flags = TUPLE_F_IN;
+	tuple.nexthdr = IPPROTO_TCP;
+	ipv6_addr_copy(&tuple.daddr, &backend_ip);
+	ipv6_addr_copy(&tuple.saddr, &client_ip);
+	tuple.sport = tcp_dst_one;
+	tuple.dport = tcp_src_one;
+	ipv6_ct_tuple_reverse(&tuple);
+
+	ct_entry = map_lookup_elem(get_ct_map6(&tuple), &tuple);
+	if (!ct_entry)
+		test_fatal("no CT entry for DSR found");
+	if (!ct_entry->dsr_internal)
+		test_fatal("CT entry doesn't have the .dsr_internal flag set");
+	if (!ipv6_addr_equals(&ct_entry->nat_addr, &frontend_ip))
+		test_fatal("CT entry doesn't have the RevDNAT addr set");
+	if (ct_entry->nat_port != tcp_svc_one)
+		test_fatal("CT entry doesn't have the RevDNAT port set");
+
 	test_finish();
 }