Commit e6ff92f4 authored by Martin KaFai Lau's avatar Martin KaFai Lau Committed by Andrii Nakryiko

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: c803475f ("bpf: selftests: test skb->tstamp in redirect_neigh")
Reported-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Signed-off-by: default avatarMartin KaFai Lau <kafai@fb.com>
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Acked-by: default avatarSong Liu <songliubraving@fb.com>
Link: https://lore.kernel.org/bpf/20220601234050.2572671-1-kafai@fb.com
parent 988d0d58
...@@ -646,7 +646,7 @@ static void test_tcp_clear_dtime(struct test_tc_dtime *skel) ...@@ -646,7 +646,7 @@ static void test_tcp_clear_dtime(struct test_tc_dtime *skel)
__u32 *errs = skel->bss->errs[t]; __u32 *errs = skel->bss->errs[t];
skel->bss->test = t; skel->bss->test = t;
test_inet_dtime(AF_INET6, SOCK_STREAM, IP6_DST, 0); test_inet_dtime(AF_INET6, SOCK_STREAM, IP6_DST, 50000 + t);
ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0, ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0,
dtime_cnt_str(t, INGRESS_FWDNS_P100)); 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) ...@@ -683,7 +683,7 @@ static void test_tcp_dtime(struct test_tc_dtime *skel, int family, bool bpf_fwd)
errs = skel->bss->errs[t]; errs = skel->bss->errs[t];
skel->bss->test = t; skel->bss->test = t;
test_inet_dtime(family, SOCK_STREAM, addr, 0); test_inet_dtime(family, SOCK_STREAM, addr, 50000 + t);
/* fwdns_prio100 prog does not read delivery_time_type, so /* fwdns_prio100 prog does not read delivery_time_type, so
* kernel puts the (rcv) timetamp in __sk_buff->tstamp * 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) ...@@ -715,13 +715,13 @@ static void test_udp_dtime(struct test_tc_dtime *skel, int family, bool bpf_fwd)
errs = skel->bss->errs[t]; errs = skel->bss->errs[t];
skel->bss->test = t; skel->bss->test = t;
test_inet_dtime(family, SOCK_DGRAM, addr, 0); test_inet_dtime(family, SOCK_DGRAM, addr, 50000 + t);
ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0, ASSERT_EQ(dtimes[INGRESS_FWDNS_P100], 0,
dtime_cnt_str(t, INGRESS_FWDNS_P100)); dtime_cnt_str(t, INGRESS_FWDNS_P100));
/* non mono delivery time is not forwarded */ /* non mono delivery time is not forwarded */
ASSERT_EQ(dtimes[INGRESS_FWDNS_P101], 0, ASSERT_EQ(dtimes[INGRESS_FWDNS_P101], 0,
dtime_cnt_str(t, INGRESS_FWDNS_P100)); dtime_cnt_str(t, INGRESS_FWDNS_P101));
for (i = EGRESS_FWDNS_P100; i < SET_DTIME; i++) for (i = EGRESS_FWDNS_P100; i < SET_DTIME; i++)
ASSERT_GT(dtimes[i], 0, dtime_cnt_str(t, i)); ASSERT_GT(dtimes[i], 0, dtime_cnt_str(t, i));
......
...@@ -11,6 +11,8 @@ ...@@ -11,6 +11,8 @@
#include <linux/in.h> #include <linux/in.h>
#include <linux/ip.h> #include <linux/ip.h>
#include <linux/ipv6.h> #include <linux/ipv6.h>
#include <linux/tcp.h>
#include <linux/udp.h>
#include <bpf/bpf_helpers.h> #include <bpf/bpf_helpers.h>
#include <bpf/bpf_endian.h> #include <bpf/bpf_endian.h>
#include <sys/socket.h> #include <sys/socket.h>
...@@ -115,6 +117,19 @@ static bool bpf_fwd(void) ...@@ -115,6 +117,19 @@ static bool bpf_fwd(void)
return test < TCP_IP4_RT_FWD; return test < TCP_IP4_RT_FWD;
} }
static __u8 get_proto(void)
{
switch (test) {
case UDP_IP4:
case UDP_IP6:
case UDP_IP4_RT_FWD:
case UDP_IP6_RT_FWD:
return IPPROTO_UDP;
default:
return IPPROTO_TCP;
}
}
/* -1: parse error: TC_ACT_SHOT /* -1: parse error: TC_ACT_SHOT
* 0: not testing traffic: TC_ACT_OK * 0: not testing traffic: TC_ACT_OK
* >0: first byte is the inet_proto, second byte has the netns * >0: first byte is the inet_proto, second byte has the netns
...@@ -122,11 +137,16 @@ static bool bpf_fwd(void) ...@@ -122,11 +137,16 @@ static bool bpf_fwd(void)
*/ */
static int skb_get_type(struct __sk_buff *skb) static int skb_get_type(struct __sk_buff *skb)
{ {
__u16 dst_ns_port = __bpf_htons(50000 + test);
void *data_end = ctx_ptr(skb->data_end); void *data_end = ctx_ptr(skb->data_end);
void *data = ctx_ptr(skb->data); void *data = ctx_ptr(skb->data);
__u8 inet_proto = 0, ns = 0; __u8 inet_proto = 0, ns = 0;
struct ipv6hdr *ip6h; struct ipv6hdr *ip6h;
__u16 sport, dport;
struct iphdr *iph; struct iphdr *iph;
struct tcphdr *th;
struct udphdr *uh;
void *trans;
switch (skb->protocol) { switch (skb->protocol) {
case __bpf_htons(ETH_P_IP): case __bpf_htons(ETH_P_IP):
...@@ -138,6 +158,7 @@ static int skb_get_type(struct __sk_buff *skb) ...@@ -138,6 +158,7 @@ static int skb_get_type(struct __sk_buff *skb)
else if (iph->saddr == ip4_dst) else if (iph->saddr == ip4_dst)
ns = DST_NS; ns = DST_NS;
inet_proto = iph->protocol; inet_proto = iph->protocol;
trans = iph + 1;
break; break;
case __bpf_htons(ETH_P_IPV6): case __bpf_htons(ETH_P_IPV6):
ip6h = data + sizeof(struct ethhdr); ip6h = data + sizeof(struct ethhdr);
...@@ -148,15 +169,43 @@ static int skb_get_type(struct __sk_buff *skb) ...@@ -148,15 +169,43 @@ static int skb_get_type(struct __sk_buff *skb)
else if (v6_equal(ip6h->saddr, (struct in6_addr)ip6_dst)) else if (v6_equal(ip6h->saddr, (struct in6_addr)ip6_dst))
ns = DST_NS; ns = DST_NS;
inet_proto = ip6h->nexthdr; inet_proto = ip6h->nexthdr;
trans = ip6h + 1;
break; break;
default: default:
return 0; return 0;
} }
if ((inet_proto != IPPROTO_TCP && inet_proto != IPPROTO_UDP) || !ns) /* skb is not from src_ns or dst_ns.
* skb is not the testing IPPROTO.
*/
if (!ns || inet_proto != get_proto())
return 0;
switch (inet_proto) {
case IPPROTO_TCP:
th = trans;
if (th + 1 > data_end)
return -1;
sport = th->source;
dport = th->dest;
break;
case IPPROTO_UDP:
uh = trans;
if (uh + 1 > data_end)
return -1;
sport = uh->source;
dport = uh->dest;
break;
default:
return 0; return 0;
}
/* The skb is the testing traffic */
if ((ns == SRC_NS && dport == dst_ns_port) ||
(ns == DST_NS && sport == dst_ns_port))
return (ns << 8 | inet_proto); return (ns << 8 | inet_proto);
return 0;
} }
/* format: direction@iface@netns /* format: direction@iface@netns
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment