bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling
@ 2020-04-07  5:09 Andrey Ignatov
  2020-04-07  5:09 ` [PATCH bpf 1/2] " Andrey Ignatov
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrey Ignatov @ 2020-04-07  5:09 UTC (permalink / raw)
  To: bpf; +Cc: Andrey Ignatov, ast, daniel, kernel-team, toke

This patch set fixes bpf_get_link_xdp_id() behavior for non-zero flags and
adds selftest that verifies the fix and can be used to reproduce the
problem.


Andrey Ignatov (2):
  libbpf: Fix bpf_get_link_xdp_id flags handling
  selftests/bpf: Add test for bpf_get_link_xdp_id

 tools/lib/bpf/netlink.c                       |  2 +-
 .../selftests/bpf/prog_tests/xdp_info.c       | 68 +++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_info.c

-- 
2.24.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf 1/2] libbpf: Fix bpf_get_link_xdp_id flags handling
  2020-04-07  5:09 [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Andrey Ignatov
@ 2020-04-07  5:09 ` Andrey Ignatov
  2020-04-07  9:04   ` Toke Høiland-Jørgensen
  2020-04-07  5:09 ` [PATCH bpf 2/2] selftests/bpf: Add test for bpf_get_link_xdp_id Andrey Ignatov
  2020-04-07 23:44 ` [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Daniel Borkmann
  2 siblings, 1 reply; 5+ messages in thread
From: Andrey Ignatov @ 2020-04-07  5:09 UTC (permalink / raw)
  To: bpf; +Cc: Andrey Ignatov, ast, daniel, kernel-team, toke

Currently if one of XDP_FLAGS_{DRV,HW,SKB}_MODE flags is passed to
bpf_get_link_xdp_id() and there is a single XDP program attached to
ifindex, that program's id will be returned by bpf_get_link_xdp_id() in
prog_id argument no matter what mode the program is attached in, i.e.
flags argument is not taken into account.

For example, if there is a single program attached with
XDP_FLAGS_SKB_MODE but user calls bpf_get_link_xdp_id() with flags =
XDP_FLAGS_DRV_MODE, that skb program will be returned.

Fix it by returning info->prog_id only if user didn't specify flags. If
flags is specified then return corresponding mode-specific-field from
struct xdp_link_info.

The initial error was introduced in commit 50db9f073188 ("libbpf: Add a
support for getting xdp prog id on ifindex") and then refactored in
473f4e133a12 so 473f4e133a12 is used in the Fixes tag.

Fixes: 473f4e133a12 ("libbpf: Add bpf_get_link_xdp_info() function to get more XDP information")
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/lib/bpf/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 18b5319025e1..f5732d35930d 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -321,7 +321,7 @@ int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 
 static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
 {
-	if (info->attach_mode != XDP_ATTACHED_MULTI)
+	if (info->attach_mode != XDP_ATTACHED_MULTI && !flags)
 		return info->prog_id;
 	if (flags & XDP_FLAGS_DRV_MODE)
 		return info->drv_prog_id;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf 2/2] selftests/bpf: Add test for bpf_get_link_xdp_id
  2020-04-07  5:09 [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Andrey Ignatov
  2020-04-07  5:09 ` [PATCH bpf 1/2] " Andrey Ignatov
@ 2020-04-07  5:09 ` Andrey Ignatov
  2020-04-07 23:44 ` [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Daniel Borkmann
  2 siblings, 0 replies; 5+ messages in thread
From: Andrey Ignatov @ 2020-04-07  5:09 UTC (permalink / raw)
  To: bpf; +Cc: Andrey Ignatov, ast, daniel, kernel-team, toke

Add xdp_info selftest that makes sure that bpf_get_link_xdp_id returns
valid prog_id for different input modes:
* w/ and w/o flags when no program is attached;
* w/ and w/o flags when one program is attached.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 .../selftests/bpf/prog_tests/xdp_info.c       | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_info.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_info.c b/tools/testing/selftests/bpf/prog_tests/xdp_info.c
new file mode 100644
index 000000000000..d2d7a283d72f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_info.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/if_link.h>
+#include <test_progs.h>
+
+#define IFINDEX_LO 1
+
+void test_xdp_info(void)
+{
+	__u32 len = sizeof(struct bpf_prog_info), duration = 0, prog_id;
+	const char *file = "./xdp_dummy.o";
+	struct bpf_prog_info info = {};
+	struct bpf_object *obj;
+	int err, prog_fd;
+
+	/* Get prog_id for XDP_ATTACHED_NONE mode */
+
+	err = bpf_get_link_xdp_id(IFINDEX_LO, &prog_id, 0);
+	if (CHECK(err, "get_xdp_none", "errno=%d\n", errno))
+		return;
+	if (CHECK(prog_id, "prog_id_none", "unexpected prog_id=%u\n", prog_id))
+		return;
+
+	err = bpf_get_link_xdp_id(IFINDEX_LO, &prog_id, XDP_FLAGS_SKB_MODE);
+	if (CHECK(err, "get_xdp_none_skb", "errno=%d\n", errno))
+		return;
+	if (CHECK(prog_id, "prog_id_none_skb", "unexpected prog_id=%u\n",
+		  prog_id))
+		return;
+
+	/* Setup prog */
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
+	if (CHECK_FAIL(err))
+		return;
+
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &len);
+	if (CHECK(err, "get_prog_info", "errno=%d\n", errno))
+		goto out_close;
+
+	err = bpf_set_link_xdp_fd(IFINDEX_LO, prog_fd, XDP_FLAGS_SKB_MODE);
+	if (CHECK(err, "set_xdp_skb", "errno=%d\n", errno))
+		goto out_close;
+
+	/* Get prog_id for single prog mode */
+
+	err = bpf_get_link_xdp_id(IFINDEX_LO, &prog_id, 0);
+	if (CHECK(err, "get_xdp", "errno=%d\n", errno))
+		goto out;
+	if (CHECK(prog_id != info.id, "prog_id", "prog_id not available\n"))
+		goto out;
+
+	err = bpf_get_link_xdp_id(IFINDEX_LO, &prog_id, XDP_FLAGS_SKB_MODE);
+	if (CHECK(err, "get_xdp_skb", "errno=%d\n", errno))
+		goto out;
+	if (CHECK(prog_id != info.id, "prog_id_skb", "prog_id not available\n"))
+		goto out;
+
+	err = bpf_get_link_xdp_id(IFINDEX_LO, &prog_id, XDP_FLAGS_DRV_MODE);
+	if (CHECK(err, "get_xdp_drv", "errno=%d\n", errno))
+		goto out;
+	if (CHECK(prog_id, "prog_id_drv", "unexpected prog_id=%u\n", prog_id))
+		goto out;
+
+out:
+	bpf_set_link_xdp_fd(IFINDEX_LO, -1, 0);
+out_close:
+	bpf_object__close(obj);
+}
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf 1/2] libbpf: Fix bpf_get_link_xdp_id flags handling
  2020-04-07  5:09 ` [PATCH bpf 1/2] " Andrey Ignatov
@ 2020-04-07  9:04   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-04-07  9:04 UTC (permalink / raw)
  To: Andrey Ignatov, bpf; +Cc: Andrey Ignatov, ast, daniel, kernel-team

Andrey Ignatov <rdna@fb.com> writes:

> Currently if one of XDP_FLAGS_{DRV,HW,SKB}_MODE flags is passed to
> bpf_get_link_xdp_id() and there is a single XDP program attached to
> ifindex, that program's id will be returned by bpf_get_link_xdp_id() in
> prog_id argument no matter what mode the program is attached in, i.e.
> flags argument is not taken into account.
>
> For example, if there is a single program attached with
> XDP_FLAGS_SKB_MODE but user calls bpf_get_link_xdp_id() with flags =
> XDP_FLAGS_DRV_MODE, that skb program will be returned.
>
> Fix it by returning info->prog_id only if user didn't specify flags. If
> flags is specified then return corresponding mode-specific-field from
> struct xdp_link_info.
>
> The initial error was introduced in commit 50db9f073188 ("libbpf: Add a
> support for getting xdp prog id on ifindex") and then refactored in
> 473f4e133a12 so 473f4e133a12 is used in the Fixes tag.
>
> Fixes: 473f4e133a12 ("libbpf: Add bpf_get_link_xdp_info() function to get more XDP information")
> Signed-off-by: Andrey Ignatov <rdna@fb.com>

Makes sense

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling
  2020-04-07  5:09 [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Andrey Ignatov
  2020-04-07  5:09 ` [PATCH bpf 1/2] " Andrey Ignatov
  2020-04-07  5:09 ` [PATCH bpf 2/2] selftests/bpf: Add test for bpf_get_link_xdp_id Andrey Ignatov
@ 2020-04-07 23:44 ` Daniel Borkmann
  2 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2020-04-07 23:44 UTC (permalink / raw)
  To: Andrey Ignatov, bpf; +Cc: ast, kernel-team, toke

On 4/7/20 7:09 AM, Andrey Ignatov wrote:
> This patch set fixes bpf_get_link_xdp_id() behavior for non-zero flags and
> adds selftest that verifies the fix and can be used to reproduce the
> problem.
> 
> 
> Andrey Ignatov (2):
>    libbpf: Fix bpf_get_link_xdp_id flags handling
>    selftests/bpf: Add test for bpf_get_link_xdp_id
> 
>   tools/lib/bpf/netlink.c                       |  2 +-
>   .../selftests/bpf/prog_tests/xdp_info.c       | 68 +++++++++++++++++++
>   2 files changed, 69 insertions(+), 1 deletion(-)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_info.c

LGTM, applied, thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-04-07 23:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07  5:09 [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling Andrey Ignatov
2020-04-07  5:09 ` [PATCH bpf 1/2] " Andrey Ignatov
2020-04-07  9:04   ` Toke Høiland-Jørgensen
2020-04-07  5:09 ` [PATCH bpf 2/2] selftests/bpf: Add test for bpf_get_link_xdp_id Andrey Ignatov
2020-04-07 23:44 ` [PATCH bpf 0/2] libbpf: Fix bpf_get_link_xdp_id flags handling 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).