| Message ID | 20200922070518.1923618-1-kafai@fb.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,bpf-next,01/11] bpf: Move the PTR_TO_BTF_ID check to check_reg_type() | expand |
On Tue, Sep 22, 2020 at 12:05:18AM -0700, Martin KaFai Lau wrote: > > @@ -165,9 +168,13 @@ int egress_read_sock_fields(struct __sk_buff *skb) > tpcpy(tp_ret, tp); > > if (sk_ret == &srv_sk) { > + ktp = bpf_skc_to_tcp_sock(sk); > + if (!ktp) > + RET_LOG(); > + lsndtime = ktp->lsndtime; If I understood the patches correctly they extend the use cases significantly, but this test only covers cgroup/egress type with one particular helper. Could you please expand the test cases to cover more of things from earlier patches. Patch 7 covers some, but imo not enough. Similarly the case of sk_release(sk) in one place and sk_release(ktp) in another branch (one you mention in commit log) needs to have C based selftest as well.
diff --git a/tools/testing/selftests/bpf/prog_tests/sock_fields.c b/tools/testing/selftests/bpf/prog_tests/sock_fields.c index 23d14e2d0d28..9b81600a1ef4 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_fields.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_fields.c @@ -144,6 +144,8 @@ static void check_result(void) "srv_sk", "Unexpected. Check srv_sk output. egress_linum:%u\n", egress_linum); + CHECK(!skel->bss->lsndtime, "srv_tp", "Unexpected lsndtime:0\n"); + CHECK(cli_sk.state == 10 || !cli_sk.state || cli_sk.family != AF_INET6 || diff --git a/tools/testing/selftests/bpf/progs/test_sock_fields.c b/tools/testing/selftests/bpf/progs/test_sock_fields.c index 370e33a858db..24fdf2b2747e 100644 --- a/tools/testing/selftests/bpf/progs/test_sock_fields.c +++ b/tools/testing/selftests/bpf/progs/test_sock_fields.c @@ -7,6 +7,7 @@ #include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> +#include "bpf_tcp_helpers.h" enum bpf_linum_array_idx { EGRESS_LINUM_IDX, @@ -47,6 +48,7 @@ struct bpf_tcp_sock srv_tp = {}; struct bpf_sock listen_sk = {}; struct bpf_sock srv_sk = {}; struct bpf_sock cli_sk = {}; +__u64 lsndtime = 0; static bool is_loopback6(__u32 *a6) { @@ -121,6 +123,7 @@ int egress_read_sock_fields(struct __sk_buff *skb) struct bpf_tcp_sock *tp, *tp_ret; struct bpf_sock *sk, *sk_ret; __u32 linum, linum_idx; + struct tcp_sock *ktp; linum_idx = EGRESS_LINUM_IDX; @@ -165,9 +168,13 @@ int egress_read_sock_fields(struct __sk_buff *skb) tpcpy(tp_ret, tp); if (sk_ret == &srv_sk) { + ktp = bpf_skc_to_tcp_sock(sk); + if (!ktp) + RET_LOG(); + lsndtime = ktp->lsndtime; /* The userspace has created it for srv sk */ - pkt_out_cnt = bpf_sk_storage_get(&sk_pkt_out_cnt, sk, 0, 0); - pkt_out_cnt10 = bpf_sk_storage_get(&sk_pkt_out_cnt10, sk, + pkt_out_cnt = bpf_sk_storage_get(&sk_pkt_out_cnt, ktp, 0, 0); + pkt_out_cnt10 = bpf_sk_storage_get(&sk_pkt_out_cnt10, ktp, 0, 0); } else { pkt_out_cnt = bpf_sk_storage_get(&sk_pkt_out_cnt, sk,
This test uses bpf_skc_to_tcp_sock() to get a kernel tcp_sock ptr "ktp". Access the ktp->lsndtime and also pass it to bpf_sk_storage_get(). Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- tools/testing/selftests/bpf/prog_tests/sock_fields.c | 2 ++ tools/testing/selftests/bpf/progs/test_sock_fields.c | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-)