| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAL+tcoBp4qj-Q3w3tb3U-qHnDpmjhmcJ7xBpDXYj+q-SHcf+Nw@mail.gmail.com>
Date: Tue, 11 Feb 2025 19:37:33 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, dsahern@...nel.org, willemdebruijn.kernel@...il.com,
willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, horms@...nel.org, bpf@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v9 12/12] selftests/bpf: add simple bpf tests in
the tx path for timestamping feature
On Tue, Feb 11, 2025 at 4:05 PM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
> On 2/8/25 2:32 AM, Jason Xing wrote:
> > ---
> > .../bpf/prog_tests/so_timestamping.c | 79 +++++
> > .../selftests/bpf/progs/so_timestamping.c | 312 ++++++++++++++++++
>
> A bike shedding. s/so_timestamping.c/net_timestamping.c/
Will rename them.
>
> > diff --git a/tools/testing/selftests/bpf/progs/so_timestamping.c b/tools/testing/selftests/bpf/progs/so_timestamping.c
> > new file mode 100644
> > index 000000000000..4974552cdecb
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/so_timestamping.c
> > @@ -0,0 +1,312 @@
> > +#include "vmlinux.h"
> > +#include "bpf_tracing_net.h"
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include "bpf_misc.h"
> > +#include "bpf_kfuncs.h"
> > +#define BPF_PROG_TEST_TCP_HDR_OPTIONS
> > +#include "test_tcp_hdr_options.h"
> > +#include <errno.h>
> > +
> > +#define SK_BPF_CB_FLAGS 1009
> > +#define SK_BPF_CB_TX_TIMESTAMPING 1
> > +
> > +int nr_active;
> > +int nr_snd;
> > +int nr_passive;
> > +int nr_sched;
> > +int nr_txsw;
> > +int nr_ack;
> > +
> > +struct sockopt_test {
> > + int opt;
> > + int new;
> > +};
> > +
> > +static const struct sockopt_test sol_socket_tests[] = {
> > + { .opt = SK_BPF_CB_FLAGS, .new = SK_BPF_CB_TX_TIMESTAMPING, },
> > + { .opt = 0, },
> > +};
> > +
> > +struct loop_ctx {
> > + void *ctx;
> > + const struct sock *sk;
> > +};
> > +
> > +struct sk_stg {
> > + __u64 sendmsg_ns; /* record ts when sendmsg is called */
> > +};
> > +
> > +struct sk_tskey {
> > + u64 cookie;
> > + u32 tskey;
> > +};
> > +
> > +struct delay_info {
> > + u64 sendmsg_ns; /* record ts when sendmsg is called */
> > + u32 sched_delay; /* SCHED_OPT_CB - sendmsg_ns */
> > + u32 sw_snd_delay; /* SW_OPT_CB - SCHED_OPT_CB */
> > + u32 ack_delay; /* ACK_OPT_CB - SW_OPT_CB */
> > +};
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_SK_STORAGE);
> > + __uint(map_flags, BPF_F_NO_PREALLOC);
> > + __type(key, int);
> > + __type(value, struct sk_stg);
> > +} sk_stg_map SEC(".maps");
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_HASH);
> > + __type(key, struct sk_tskey);
> > + __type(value, struct delay_info);
> > + __uint(max_entries, 1024);
> > +} time_map SEC(".maps");
> > +
> > +static u64 delay_tolerance_nsec = 10000000000; /* 10 second as an example */
> > +
> > +extern int bpf_sock_ops_enable_tx_tstamp(struct bpf_sock_ops_kern *skops) __ksym;
> > +
> > +static int bpf_test_sockopt_int(void *ctx, const struct sock *sk,
> > + const struct sockopt_test *t,
> > + int level)
>
> This should be the only one that is needed even when supporting the future RX
> timestamping.
>
> TX and RX timestamping need to be tested independently. Looping it will either
> enabling them together or disabling them together. It cannot test whether RX
> will work by itself.
>
> Thus, the bpf_loop won't help. Lets remove it to simplify the test.
Got it. Will remove it.
>
> > +{
> > + int new, opt, tmp;
> > +
> > + opt = t->opt;
> > + new = t->new;
> > +
> > + if (bpf_setsockopt(ctx, level, opt, &new, sizeof(new)))
> > + return 1;
> > +
> > + if (bpf_getsockopt(ctx, level, opt, &tmp, sizeof(tmp)) ||
> > + tmp != new)
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +static int bpf_test_socket_sockopt(__u32 i, struct loop_ctx *lc)
> > +{
> > + const struct sockopt_test *t;
> > +
> > + if (i >= ARRAY_SIZE(sol_socket_tests))
> > + return 1;
> > +
> > + t = &sol_socket_tests[i];
> > + if (!t->opt)
> > + return 1;
> > +
> > + return bpf_test_sockopt_int(lc->ctx, lc->sk, t, SOL_SOCKET);
> > +}
> > +
> > +static int bpf_test_sockopt(void *ctx, const struct sock *sk)
> > +{
> > + struct loop_ctx lc = { .ctx = ctx, .sk = sk, };
> > + int n;
> > +
> > + n = bpf_loop(ARRAY_SIZE(sol_socket_tests), bpf_test_socket_sockopt, &lc, 0);
> > + if (n != ARRAY_SIZE(sol_socket_tests))
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +static bool bpf_test_access_sockopt(void *ctx)
> > +{
> > + const struct sockopt_test *t;
> > + int tmp, ret, i = 0;
> > + int level = SOL_SOCKET;
> > +
> > + t = &sol_socket_tests[i];
> > +
> > + for (; t->opt;) {
>
> It really does not need a loop here. It only needs to test "one" optname to
> ensure it is -EOPNOTSUPP.
>
> > + ret = bpf_setsockopt(ctx, level, t->opt, (void *)&t->new, sizeof(t->new));
> > + if (ret != -EOPNOTSUPP)
> > + return true;
> > +
> > + ret = bpf_getsockopt(ctx, level, t->opt, &tmp, sizeof(tmp));
> > + if (ret != -EOPNOTSUPP)
> > + return true;
> > +
> > + if (++i >= ARRAY_SIZE(sol_socket_tests))
> > + break;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +/* Adding a simple test to see if we can get an expected value */
> > +static bool bpf_test_access_load_hdr_opt(struct bpf_sock_ops *skops)
> > +{
> > + struct tcp_opt reg_opt;
>
> Just noticed this one. Use a plain u8 array. Then no need to include the
> "test_tcp_hdr_options.h" from an unrelated test.
Will update it.
Thanks,
Jason
>
> > + int load_flags = 0;
> > + int ret;
> > +
> > + reg_opt.kind = TCPOPT_EXP;
>
> The kind could be any integer, e.g. 2.
>
> > + reg_opt.len = 0;
> > + reg_opt.data32 = 0;
> > + ret = bpf_load_hdr_opt(skops, ®_opt, sizeof(reg_opt), load_flags);
> > + if (ret != -EOPNOTSUPP)
> > + return true;
> > +
> > + return false;
> > +}
Powered by blists - more mailing lists