All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.