Skip to content

Commit f7dd4cf

Browse files
iamkafaianakryiko
authored andcommitted
selftests/bpf: Fix tc_redirect_dtime
tc_redirect_dtime was reported flaky from time to time. It always fails at the udp test and complains about the bpf@tc-ingress got a skb->tstamp when handling udp packet. It is unexpected because the skb->tstamp should have been cleared when crossing different netns. The most likely cause is that the skb is actually a tcp packet from the earlier tcp test. It could be the final TCP_FIN handling. This patch tightens the skb->tstamp check in the bpf prog. It ensures the skb is the current testing traffic. First, it checks that skb matches the IPPROTO of the running test (i.e. tcp vs udp). Second, it checks the server port (dst_ns_port). The server port is unique for each test (50000 + test_enum). Also fixed a typo in test_udp_dtime(): s/P100/P101/ Fixes: c803475 ("bpf: selftests: test skb->tstamp in redirect_neigh") Reported-by: Andrii Nakryiko <[email protected]> Signed-off-by: Martin KaFai Lau <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Acked-by: Song Liu <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 330eb2a commit f7dd4cf

File tree

2 files changed

+55
-6
lines changed

2 files changed

+55
-6
lines changed

tools/testing/selftests/bpf/prog_tests/tc_redirect.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ static void test_tcp_clear_dtime(struct test_tc_dtime *skel)
646646
__u32 *errs = skel->bss->errs[t];
647647

648648
skel->bss->test = t;
649-
test_inet_dtime(AF_INET6, SOCK_STREAM, IP6_DST, 0);
649+
test_inet_dtime(AF_INET6, SOCK_STREAM, IP6_DST, 50000 + t);
650650

651651
ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0,
652652
dtime_cnt_str(t, INGRESS_FWDNS_P100));
@@ -683,7 +683,7 @@ static void test_tcp_dtime(struct test_tc_dtime *skel, int family, bool bpf_fwd)
683683
errs = skel->bss->errs[t];
684684

685685
skel->bss->test = t;
686-
test_inet_dtime(family, SOCK_STREAM, addr, 0);
686+
test_inet_dtime(family, SOCK_STREAM, addr, 50000 + t);
687687

688688
/* fwdns_prio100 prog does not read delivery_time_type, so
689689
* kernel puts the (rcv) timetamp in __sk_buff->tstamp
@@ -715,13 +715,13 @@ static void test_udp_dtime(struct test_tc_dtime *skel, int family, bool bpf_fwd)
715715
errs = skel->bss->errs[t];
716716

717717
skel->bss->test = t;
718-
test_inet_dtime(family, SOCK_DGRAM, addr, 0);
718+
test_inet_dtime(family, SOCK_DGRAM, addr, 50000 + t);
719719

720720
ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0,
721721
dtime_cnt_str(t, INGRESS_FWDNS_P100));
722722
/* non mono delivery time is not forwarded */
723723
ASSERT_EQ(dtimes[INGRESS_FWDNS_P101], 0,
724-
dtime_cnt_str(t, INGRESS_FWDNS_P100));
724+
dtime_cnt_str(t, INGRESS_FWDNS_P101));
725725
for (i = EGRESS_FWDNS_P100; i < SET_DTIME; i++)
726726
ASSERT_GT(dtimes[i], 0, dtime_cnt_str(t, i));
727727

tools/testing/selftests/bpf/progs/test_tc_dtime.c

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#include <linux/in.h>
1212
#include <linux/ip.h>
1313
#include <linux/ipv6.h>
14+
#include <linux/tcp.h>
15+
#include <linux/udp.h>
1416
#include <bpf/bpf_helpers.h>
1517
#include <bpf/bpf_endian.h>
1618
#include <sys/socket.h>
@@ -115,18 +117,36 @@ static bool bpf_fwd(void)
115117
return test < TCP_IP4_RT_FWD;
116118
}
117119

120+
static __u8 get_proto(void)
121+
{
122+
switch (test) {
123+
case UDP_IP4:
124+
case UDP_IP6:
125+
case UDP_IP4_RT_FWD:
126+
case UDP_IP6_RT_FWD:
127+
return IPPROTO_UDP;
128+
default:
129+
return IPPROTO_TCP;
130+
}
131+
}
132+
118133
/* -1: parse error: TC_ACT_SHOT
119134
* 0: not testing traffic: TC_ACT_OK
120135
* >0: first byte is the inet_proto, second byte has the netns
121136
* of the sender
122137
*/
123138
static int skb_get_type(struct __sk_buff *skb)
124139
{
140+
__u16 dst_ns_port = __bpf_htons(50000 + test);
125141
void *data_end = ctx_ptr(skb->data_end);
126142
void *data = ctx_ptr(skb->data);
127143
__u8 inet_proto = 0, ns = 0;
128144
struct ipv6hdr *ip6h;
145+
__u16 sport, dport;
129146
struct iphdr *iph;
147+
struct tcphdr *th;
148+
struct udphdr *uh;
149+
void *trans;
130150

131151
switch (skb->protocol) {
132152
case __bpf_htons(ETH_P_IP):
@@ -138,6 +158,7 @@ static int skb_get_type(struct __sk_buff *skb)
138158
else if (iph->saddr == ip4_dst)
139159
ns = DST_NS;
140160
inet_proto = iph->protocol;
161+
trans = iph + 1;
141162
break;
142163
case __bpf_htons(ETH_P_IPV6):
143164
ip6h = data + sizeof(struct ethhdr);
@@ -148,15 +169,43 @@ static int skb_get_type(struct __sk_buff *skb)
148169
else if (v6_equal(ip6h->saddr, (struct in6_addr)ip6_dst))
149170
ns = DST_NS;
150171
inet_proto = ip6h->nexthdr;
172+
trans = ip6h + 1;
151173
break;
152174
default:
153175
return 0;
154176
}
155177

156-
if ((inet_proto != IPPROTO_TCP && inet_proto != IPPROTO_UDP) || !ns)
178+
/* skb is not from src_ns or dst_ns.
179+
* skb is not the testing IPPROTO.
180+
*/
181+
if (!ns || inet_proto != get_proto())
157182
return 0;
158183

159-
return (ns << 8 | inet_proto);
184+
switch (inet_proto) {
185+
case IPPROTO_TCP:
186+
th = trans;
187+
if (th + 1 > data_end)
188+
return -1;
189+
sport = th->source;
190+
dport = th->dest;
191+
break;
192+
case IPPROTO_UDP:
193+
uh = trans;
194+
if (uh + 1 > data_end)
195+
return -1;
196+
sport = uh->source;
197+
dport = uh->dest;
198+
break;
199+
default:
200+
return 0;
201+
}
202+
203+
/* The skb is the testing traffic */
204+
if ((ns == SRC_NS && dport == dst_ns_port) ||
205+
(ns == DST_NS && sport == dst_ns_port))
206+
return (ns << 8 | inet_proto);
207+
208+
return 0;
160209
}
161210

162211
/* format: direction@iface@netns

0 commit comments

Comments
 (0)