* [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts
@ 2020-04-14 14:50 Toke Høiland-Jørgensen
2020-04-14 14:50 ` [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test Toke Høiland-Jørgensen
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-04-14 14:50 UTC (permalink / raw)
To: daniel, ast; +Cc: Toke Høiland-Jørgensen, bpf, netdev, David Ahern
The 'old_fd' parameter used for atomic replacement of XDP programs is
supposed to be an FD, but was left as a u32 from an earlier iteration of
the patch that added it. It was converted to an int when read, so things
worked correctly even with negative values, but better change the
definition to correctly reflect the intention.
Fixes: bd5ca3ef93cd ("libbpf: Add function to set link XDP fd while specifying old program")
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/lib/bpf/libbpf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 44df1d3e7287..f1dacecb1619 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -458,7 +458,7 @@ struct xdp_link_info {
struct bpf_xdp_set_link_opts {
size_t sz;
- __u32 old_fd;
+ int old_fd;
};
#define bpf_xdp_set_link_opts__last_field old_fd
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test
2020-04-14 14:50 [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts Toke Høiland-Jørgensen
@ 2020-04-14 14:50 ` Toke Høiland-Jørgensen
2020-04-14 22:30 ` Song Liu
2020-04-14 19:47 ` [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts David Ahern
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-04-14 14:50 UTC (permalink / raw)
To: daniel, ast; +Cc: Toke Høiland-Jørgensen, bpf, netdev, David Ahern
David Ahern noticed that there was a bug in the EXPECTED_FD code so
programs did not get detached properly when that parameter was supplied.
This case was not included in the xdp_attach tests; so let's add it to be
sure that such a bug does not sneak back in down.
Fixes: 87854a0b57b3 ("selftests/bpf: Add tests for attaching XDP programs")
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
.../selftests/bpf/prog_tests/xdp_attach.c | 30 ++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
index 05b294d6b923..15ef3531483e 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_attach.c
@@ -6,19 +6,34 @@
void test_xdp_attach(void)
{
+ __u32 duration = 0, id1, id2, id0 = 0, len;
struct bpf_object *obj1, *obj2, *obj3;
const char *file = "./test_xdp.o";
+ struct bpf_prog_info info = {};
int err, fd1, fd2, fd3;
- __u32 duration = 0;
DECLARE_LIBBPF_OPTS(bpf_xdp_set_link_opts, opts,
.old_fd = -1);
+ len = sizeof(info);
+
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj1, &fd1);
if (CHECK_FAIL(err))
return;
+ err = bpf_obj_get_info_by_fd(fd1, &info, &len);
+ if (CHECK_FAIL(err))
+ goto out_1;
+ id1 = info.id;
+
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj2, &fd2);
if (CHECK_FAIL(err))
goto out_1;
+
+ memset(&info, 0, sizeof(info));
+ err = bpf_obj_get_info_by_fd(fd2, &info, &len);
+ if (CHECK_FAIL(err))
+ goto out_2;
+ id2 = info.id;
+
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj3, &fd3);
if (CHECK_FAIL(err))
goto out_2;
@@ -28,6 +43,11 @@ void test_xdp_attach(void)
if (CHECK(err, "load_ok", "initial load failed"))
goto out_close;
+ err = bpf_get_link_xdp_id(IFINDEX_LO, &id0, 0);
+ if (CHECK(err || id0 != id1, "id1_check",
+ "loaded prog id %u != id1 %u, err %d", id0, id1, err))
+ goto out_close;
+
err = bpf_set_link_xdp_fd_opts(IFINDEX_LO, fd2, XDP_FLAGS_REPLACE,
&opts);
if (CHECK(!err, "load_fail", "load with expected id didn't fail"))
@@ -37,6 +57,10 @@ void test_xdp_attach(void)
err = bpf_set_link_xdp_fd_opts(IFINDEX_LO, fd2, 0, &opts);
if (CHECK(err, "replace_ok", "replace valid old_fd failed"))
goto out;
+ err = bpf_get_link_xdp_id(IFINDEX_LO, &id0, 0);
+ if (CHECK(err || id0 != id2, "id2_check",
+ "loaded prog id %u != id2 %u, err %d", id0, id2, err))
+ goto out_close;
err = bpf_set_link_xdp_fd_opts(IFINDEX_LO, fd3, 0, &opts);
if (CHECK(!err, "replace_fail", "replace invalid old_fd didn't fail"))
@@ -51,6 +75,10 @@ void test_xdp_attach(void)
if (CHECK(err, "remove_ok", "remove valid old_fd failed"))
goto out;
+ err = bpf_get_link_xdp_id(IFINDEX_LO, &id0, 0);
+ if (CHECK(err || id0 != 0, "unload_check",
+ "loaded prog id %u != 0, err %d", id0, err))
+ goto out_close;
out:
bpf_set_link_xdp_fd(IFINDEX_LO, -1, 0);
out_close:
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts
2020-04-14 14:50 [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts Toke Høiland-Jørgensen
2020-04-14 14:50 ` [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test Toke Høiland-Jørgensen
@ 2020-04-14 19:47 ` David Ahern
2020-04-14 22:24 ` Song Liu
2020-04-15 12:34 ` Daniel Borkmann
3 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2020-04-14 19:47 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, daniel, ast; +Cc: bpf, netdev
On 4/14/20 8:50 AM, Toke Høiland-Jørgensen wrote:
> The 'old_fd' parameter used for atomic replacement of XDP programs is
> supposed to be an FD, but was left as a u32 from an earlier iteration of
> the patch that added it. It was converted to an int when read, so things
> worked correctly even with negative values, but better change the
> definition to correctly reflect the intention.
>
> Fixes: bd5ca3ef93cd ("libbpf: Add function to set link XDP fd while specifying old program")
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> tools/lib/bpf/libbpf.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 44df1d3e7287..f1dacecb1619 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -458,7 +458,7 @@ struct xdp_link_info {
>
> struct bpf_xdp_set_link_opts {
> size_t sz;
> - __u32 old_fd;
> + int old_fd;
> };
> #define bpf_xdp_set_link_opts__last_field old_fd
>
>
int is much better. Thanks,
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts
2020-04-14 14:50 [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts Toke Høiland-Jørgensen
2020-04-14 14:50 ` [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test Toke Høiland-Jørgensen
2020-04-14 19:47 ` [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts David Ahern
@ 2020-04-14 22:24 ` Song Liu
2020-04-15 12:34 ` Daniel Borkmann
3 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2020-04-14 22:24 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Networking, David Ahern
On Tue, Apr 14, 2020 at 9:20 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> The 'old_fd' parameter used for atomic replacement of XDP programs is
> supposed to be an FD, but was left as a u32 from an earlier iteration of
> the patch that added it. It was converted to an int when read, so things
> worked correctly even with negative values, but better change the
> definition to correctly reflect the intention.
>
> Fixes: bd5ca3ef93cd ("libbpf: Add function to set link XDP fd while specifying old program")
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test
2020-04-14 14:50 ` [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test Toke Høiland-Jørgensen
@ 2020-04-14 22:30 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2020-04-14 22:30 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Networking, David Ahern
On Tue, Apr 14, 2020 at 9:20 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> David Ahern noticed that there was a bug in the EXPECTED_FD code so
> programs did not get detached properly when that parameter was supplied.
> This case was not included in the xdp_attach tests; so let's add it to be
> sure that such a bug does not sneak back in down.
>
> Fixes: 87854a0b57b3 ("selftests/bpf: Add tests for attaching XDP programs")
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts
2020-04-14 14:50 [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2020-04-14 22:24 ` Song Liu
@ 2020-04-15 12:34 ` Daniel Borkmann
3 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2020-04-15 12:34 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, ast; +Cc: bpf, netdev, David Ahern
On 4/14/20 4:50 PM, Toke Høiland-Jørgensen wrote:
> The 'old_fd' parameter used for atomic replacement of XDP programs is
> supposed to be an FD, but was left as a u32 from an earlier iteration of
> the patch that added it. It was converted to an int when read, so things
> worked correctly even with negative values, but better change the
> definition to correctly reflect the intention.
>
> Fixes: bd5ca3ef93cd ("libbpf: Add function to set link XDP fd while specifying old program")
> Reported-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Both applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-15 12:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14 14:50 [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts Toke Høiland-Jørgensen
2020-04-14 14:50 ` [PATCH bpf 2/2] selftests/bpf: Check for correct program attach/detach in xdp_attach test Toke Høiland-Jørgensen
2020-04-14 22:30 ` Song Liu
2020-04-14 19:47 ` [PATCH bpf 1/2] libbpf: Fix type of old_fd in bpf_xdp_set_link_opts David Ahern
2020-04-14 22:24 ` Song Liu
2020-04-15 12:34 ` Daniel Borkmann
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).