* [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
@ 2019-12-11 17:53 Stanislav Fomichev
2019-12-11 17:53 ` [PATCH bpf-next 2/2] selftests/bpf: test wire_len/gso_segs in BPF_PROG_TEST_RUN Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Stanislav Fomichev @ 2019-12-11 17:53 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
gso_segs is capped by GSO_MAX_SEGS.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
net/bpf/test_run.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 85c8cbbada92..06cadba2e3b9 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -263,8 +263,10 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
return -EINVAL;
/* tstamp is allowed */
+ /* wire_len is allowed */
+ /* gso_segs is allowed */
- if (!range_is_zero(__skb, offsetofend(struct __sk_buff, tstamp),
+ if (!range_is_zero(__skb, offsetofend(struct __sk_buff, gso_segs),
sizeof(struct __sk_buff)))
return -EINVAL;
@@ -272,6 +274,14 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
skb->tstamp = __skb->tstamp;
memcpy(&cb->data, __skb->cb, QDISC_CB_PRIV_LEN);
+ if (__skb->wire_len < skb->len || __skb->wire_len > GSO_MAX_SIZE)
+ return -EINVAL;
+ cb->pkt_len = __skb->wire_len;
+
+ if (__skb->gso_segs > GSO_MAX_SEGS)
+ return -EINVAL;
+ skb_shinfo(skb)->gso_segs = __skb->gso_segs;
+
return 0;
}
@@ -285,6 +295,8 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
__skb->priority = skb->priority;
__skb->tstamp = skb->tstamp;
memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
+ __skb->wire_len = cb->pkt_len;
+ __skb->gso_segs = skb_shinfo(skb)->gso_segs;
}
int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
--
2.24.0.525.g8f36a354ae-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next 2/2] selftests/bpf: test wire_len/gso_segs in BPF_PROG_TEST_RUN
2019-12-11 17:53 [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Stanislav Fomichev
@ 2019-12-11 17:53 ` Stanislav Fomichev
2019-12-12 18:44 ` Martin Lau
2019-12-12 18:44 ` [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Martin Lau
2019-12-13 20:55 ` Alexei Starovoitov
2 siblings, 1 reply; 8+ messages in thread
From: Stanislav Fomichev @ 2019-12-11 17:53 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
Make sure we can pass arbitrary data in wire_len/gso_segs.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/prog_tests/skb_ctx.c | 2 ++
tools/testing/selftests/bpf/progs/test_skb_ctx.c | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
index a2eb8db8dafb..edf5e8c7d400 100644
--- a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
+++ b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
@@ -11,6 +11,8 @@ void test_skb_ctx(void)
.cb[4] = 5,
.priority = 6,
.tstamp = 7,
+ .wire_len = 100,
+ .gso_segs = 8,
};
struct bpf_prog_test_run_attr tattr = {
.data_in = &pkt_v4,
diff --git a/tools/testing/selftests/bpf/progs/test_skb_ctx.c b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
index 2a9f4c736ebc..534fbf9a7344 100644
--- a/tools/testing/selftests/bpf/progs/test_skb_ctx.c
+++ b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
@@ -18,5 +18,10 @@ int process(struct __sk_buff *skb)
skb->priority++;
skb->tstamp++;
+ if (skb->wire_len != 100)
+ return 1;
+ if (skb->gso_segs != 8)
+ return 1;
+
return 0;
}
--
2.24.0.525.g8f36a354ae-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
2019-12-11 17:53 [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Stanislav Fomichev
2019-12-11 17:53 ` [PATCH bpf-next 2/2] selftests/bpf: test wire_len/gso_segs in BPF_PROG_TEST_RUN Stanislav Fomichev
@ 2019-12-12 18:44 ` Martin Lau
2019-12-13 20:55 ` Alexei Starovoitov
2 siblings, 0 replies; 8+ messages in thread
From: Martin Lau @ 2019-12-12 18:44 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, davem, ast, daniel
On Wed, Dec 11, 2019 at 09:53:48AM -0800, Stanislav Fomichev wrote:
> wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
> gso_segs is capped by GSO_MAX_SEGS.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
2019-12-11 17:53 [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Stanislav Fomichev
2019-12-11 17:53 ` [PATCH bpf-next 2/2] selftests/bpf: test wire_len/gso_segs in BPF_PROG_TEST_RUN Stanislav Fomichev
2019-12-12 18:44 ` [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Martin Lau
@ 2019-12-13 20:55 ` Alexei Starovoitov
2019-12-13 21:23 ` Stanislav Fomichev
2 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2019-12-13 20:55 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Network Development, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann
On Wed, Dec 11, 2019 at 9:53 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
> gso_segs is capped by GSO_MAX_SEGS.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
This change breaks tests:
./test_progs -n 16
test_kfree_skb:PASS:prog_load sched cls 0 nsec
test_kfree_skb:PASS:prog_load raw tp 0 nsec
test_kfree_skb:PASS:find_prog 0 nsec
test_kfree_skb:PASS:find_prog 0 nsec
test_kfree_skb:PASS:find_prog 0 nsec
test_kfree_skb:PASS:find global data 0 nsec
test_kfree_skb:PASS:attach_raw_tp 0 nsec
test_kfree_skb:PASS:attach fentry 0 nsec
test_kfree_skb:PASS:attach fexit 0 nsec
test_kfree_skb:PASS:find_perf_buf_map 0 nsec
test_kfree_skb:PASS:perf_buf__new 0 nsec
test_kfree_skb:FAIL:ipv6 err -1 errno 22 retval 0 duration 0
on_sample:PASS:check_size 0 nsec
on_sample:PASS:check_meta_ifindex 0 nsec
on_sample:PASS:check_cb8_0 0 nsec
on_sample:PASS:check_cb32_0 0 nsec
on_sample:PASS:check_eth 0 nsec
on_sample:PASS:check_ip 0 nsec
on_sample:PASS:check_tcp 0 nsec
test_kfree_skb:PASS:perf_buffer__poll 0 nsec
test_kfree_skb:PASS:get_result 0 nsec
#16 kfree_skb:FAIL
Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
2019-12-13 20:55 ` Alexei Starovoitov
@ 2019-12-13 21:23 ` Stanislav Fomichev
2019-12-13 21:30 ` Alexei Starovoitov
0 siblings, 1 reply; 8+ messages in thread
From: Stanislav Fomichev @ 2019-12-13 21:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, Network Development, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On 12/13, Alexei Starovoitov wrote:
> On Wed, Dec 11, 2019 at 9:53 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
> > gso_segs is capped by GSO_MAX_SEGS.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> This change breaks tests:
> ./test_progs -n 16
> test_kfree_skb:PASS:prog_load sched cls 0 nsec
> test_kfree_skb:PASS:prog_load raw tp 0 nsec
> test_kfree_skb:PASS:find_prog 0 nsec
> test_kfree_skb:PASS:find_prog 0 nsec
> test_kfree_skb:PASS:find_prog 0 nsec
> test_kfree_skb:PASS:find global data 0 nsec
> test_kfree_skb:PASS:attach_raw_tp 0 nsec
> test_kfree_skb:PASS:attach fentry 0 nsec
> test_kfree_skb:PASS:attach fexit 0 nsec
> test_kfree_skb:PASS:find_perf_buf_map 0 nsec
> test_kfree_skb:PASS:perf_buf__new 0 nsec
> test_kfree_skb:FAIL:ipv6 err -1 errno 22 retval 0 duration 0
> on_sample:PASS:check_size 0 nsec
> on_sample:PASS:check_meta_ifindex 0 nsec
> on_sample:PASS:check_cb8_0 0 nsec
> on_sample:PASS:check_cb32_0 0 nsec
> on_sample:PASS:check_eth 0 nsec
> on_sample:PASS:check_ip 0 nsec
> on_sample:PASS:check_tcp 0 nsec
> test_kfree_skb:PASS:perf_buffer__poll 0 nsec
> test_kfree_skb:PASS:get_result 0 nsec
> #16 kfree_skb:FAIL
> Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
Ugh, it's probably because of '__skb->wire_len < skb->len' check.
Let me take a look.
(sorry, I'm still not running/looking at full test_progs because BTF support
is WIP in our toolchain and some subtests fail because of that,
generating a bunch of noise).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
2019-12-13 21:23 ` Stanislav Fomichev
@ 2019-12-13 21:30 ` Alexei Starovoitov
2019-12-13 22:06 ` Stanislav Fomichev
0 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2019-12-13 21:30 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, Network Development, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On Fri, Dec 13, 2019 at 1:23 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 12/13, Alexei Starovoitov wrote:
> > On Wed, Dec 11, 2019 at 9:53 AM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
> > > gso_segs is capped by GSO_MAX_SEGS.
> > >
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >
> > This change breaks tests:
> > ./test_progs -n 16
> > test_kfree_skb:PASS:prog_load sched cls 0 nsec
> > test_kfree_skb:PASS:prog_load raw tp 0 nsec
> > test_kfree_skb:PASS:find_prog 0 nsec
> > test_kfree_skb:PASS:find_prog 0 nsec
> > test_kfree_skb:PASS:find_prog 0 nsec
> > test_kfree_skb:PASS:find global data 0 nsec
> > test_kfree_skb:PASS:attach_raw_tp 0 nsec
> > test_kfree_skb:PASS:attach fentry 0 nsec
> > test_kfree_skb:PASS:attach fexit 0 nsec
> > test_kfree_skb:PASS:find_perf_buf_map 0 nsec
> > test_kfree_skb:PASS:perf_buf__new 0 nsec
> > test_kfree_skb:FAIL:ipv6 err -1 errno 22 retval 0 duration 0
> > on_sample:PASS:check_size 0 nsec
> > on_sample:PASS:check_meta_ifindex 0 nsec
> > on_sample:PASS:check_cb8_0 0 nsec
> > on_sample:PASS:check_cb32_0 0 nsec
> > on_sample:PASS:check_eth 0 nsec
> > on_sample:PASS:check_ip 0 nsec
> > on_sample:PASS:check_tcp 0 nsec
> > test_kfree_skb:PASS:perf_buffer__poll 0 nsec
> > test_kfree_skb:PASS:get_result 0 nsec
> > #16 kfree_skb:FAIL
> > Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
> Ugh, it's probably because of '__skb->wire_len < skb->len' check.
> Let me take a look.
>
> (sorry, I'm still not running/looking at full test_progs because BTF support
> is WIP in our toolchain and some subtests fail because of that,
> generating a bunch of noise).
I thought all bpf-next developers are developing against that tree ?
Are you saying you cannot install the latest clang/pahole on your
development system?
git pull llvm;ninja;ninja install;
git pull pahole; cmake;make
why is it not possible?
Now your complains about skeleton make more sense,
but it's an issue with your particular setup.
Anyway I'm not sure that this test issue is actually an issue with your patch.
It could be that this test is flaky in a weird way. Just with and without your
kernel patch it's 100% reproducible for me and I need to keep the rest
of the patches
moving without introducing failures in my test setup.
All that will get resolved when we have kernel CI.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN
2019-12-13 21:30 ` Alexei Starovoitov
@ 2019-12-13 22:06 ` Stanislav Fomichev
0 siblings, 0 replies; 8+ messages in thread
From: Stanislav Fomichev @ 2019-12-13 22:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, Network Development, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
On 12/13, Alexei Starovoitov wrote:
> On Fri, Dec 13, 2019 at 1:23 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 12/13, Alexei Starovoitov wrote:
> > > On Wed, Dec 11, 2019 at 9:53 AM Stanislav Fomichev <sdf@google.com> wrote:
> > > >
> > > > wire_len should not be less than real len and is capped by GSO_MAX_SIZE.
> > > > gso_segs is capped by GSO_MAX_SEGS.
> > > >
> > > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > >
> > > This change breaks tests:
> > > ./test_progs -n 16
> > > test_kfree_skb:PASS:prog_load sched cls 0 nsec
> > > test_kfree_skb:PASS:prog_load raw tp 0 nsec
> > > test_kfree_skb:PASS:find_prog 0 nsec
> > > test_kfree_skb:PASS:find_prog 0 nsec
> > > test_kfree_skb:PASS:find_prog 0 nsec
> > > test_kfree_skb:PASS:find global data 0 nsec
> > > test_kfree_skb:PASS:attach_raw_tp 0 nsec
> > > test_kfree_skb:PASS:attach fentry 0 nsec
> > > test_kfree_skb:PASS:attach fexit 0 nsec
> > > test_kfree_skb:PASS:find_perf_buf_map 0 nsec
> > > test_kfree_skb:PASS:perf_buf__new 0 nsec
> > > test_kfree_skb:FAIL:ipv6 err -1 errno 22 retval 0 duration 0
> > > on_sample:PASS:check_size 0 nsec
> > > on_sample:PASS:check_meta_ifindex 0 nsec
> > > on_sample:PASS:check_cb8_0 0 nsec
> > > on_sample:PASS:check_cb32_0 0 nsec
> > > on_sample:PASS:check_eth 0 nsec
> > > on_sample:PASS:check_ip 0 nsec
> > > on_sample:PASS:check_tcp 0 nsec
> > > test_kfree_skb:PASS:perf_buffer__poll 0 nsec
> > > test_kfree_skb:PASS:get_result 0 nsec
> > > #16 kfree_skb:FAIL
> > > Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
> > Ugh, it's probably because of '__skb->wire_len < skb->len' check.
> > Let me take a look.
> >
> > (sorry, I'm still not running/looking at full test_progs because BTF support
> > is WIP in our toolchain and some subtests fail because of that,
> > generating a bunch of noise).
>
> I thought all bpf-next developers are developing against that tree ?
I am developing against that tree, but I have a wrapper around
make that points it to the proper version of cc/tools that we
use for prod builds (for consistency).
> Are you saying you cannot install the latest clang/pahole on your
> development system?
> git pull llvm;ninja;ninja install;
> git pull pahole; cmake;make
> why is it not possible?
Yeah, I'll do that now, it just requires some manual movements :-)
> Now your complains about skeleton make more sense,
> but it's an issue with your particular setup.
>
> Anyway I'm not sure that this test issue is actually an issue with your patch.
> It could be that this test is flaky in a weird way. Just with and without your
> kernel patch it's 100% reproducible for me and I need to keep the rest
> of the patches
> moving without introducing failures in my test setup.
> All that will get resolved when we have kernel CI.
No, I'm pretty sure that's that wire_len check. I think I need to add
a special case to set it to skb->len if it's zero.
Will follow up with a v2!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-13 22:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 17:53 [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Stanislav Fomichev
2019-12-11 17:53 ` [PATCH bpf-next 2/2] selftests/bpf: test wire_len/gso_segs in BPF_PROG_TEST_RUN Stanislav Fomichev
2019-12-12 18:44 ` Martin Lau
2019-12-12 18:44 ` [PATCH bpf-next 1/2] bpf: expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN Martin Lau
2019-12-13 20:55 ` Alexei Starovoitov
2019-12-13 21:23 ` Stanislav Fomichev
2019-12-13 21:30 ` Alexei Starovoitov
2019-12-13 22:06 ` Stanislav Fomichev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).