diff mbox series

[bpf-next,v3,18/18] selftests/bpf: Test attaching bpf qdisc to mq and non root

Message ID 20250131192912.133796-19-ameryhung@gmail.com (mailing list archive)
State New
Delegated to: BPF
Headers show
Series bpf qdisc | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 5 this patch: 5
netdev/build_tools success Errors and warnings before: 26 (+1) this patch: 26 (+1)
netdev/cc_maintainers warning 12 maintainers not CCed: martin.lau@linux.dev kpsingh@kernel.org sdf@fomichev.me jolsa@kernel.org yonghong.song@linux.dev song@kernel.org shuah@kernel.org john.fastabend@gmail.com linux-kselftest@vger.kernel.org haoluo@google.com mykolal@fb.com eddyz87@gmail.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 107 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-5 fail Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-10 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-16 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / GCC BPF
bpf/vmtest-bpf-next-VM_Test-18 fail Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-llvm-17 / GCC BPF
bpf/vmtest-bpf-next-VM_Test-24 fail Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-25 fail Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-17 / test
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-18 / GCC BPF
bpf/vmtest-bpf-next-VM_Test-30 fail Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-31 fail Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-18 / test
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / veristat-meta

Commit Message

Amery Hung Jan. 31, 2025, 7:28 p.m. UTC 
Until we are certain that existing classful qdiscs work with bpf qdisc,
make sure we don't allow attaching a bpf qdisc to non root. Meanwhile,
attaching to mq is allowed.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 tools/testing/selftests/bpf/config            |   1 +
 .../selftests/bpf/prog_tests/bpf_qdisc.c      | 111 +++++++++++++++++-
 2 files changed, 110 insertions(+), 2 deletions(-)

Comments

Amery Hung Feb. 1, 2025, 12:45 a.m. UTC | #1 
On Fri, Jan 31, 2025 at 11:29 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> Until we are certain that existing classful qdiscs work with bpf qdisc,
> make sure we don't allow attaching a bpf qdisc to non root. Meanwhile,
> attaching to mq is allowed.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
>  tools/testing/selftests/bpf/config            |   1 +
>  .../selftests/bpf/prog_tests/bpf_qdisc.c      | 111 +++++++++++++++++-
>  2 files changed, 110 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index 6b0cab55bd2d..3201a962b3dc 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -74,6 +74,7 @@ CONFIG_NET_MPLS_GSO=y
>  CONFIG_NET_SCH_BPF=y
>  CONFIG_NET_SCH_FQ=y
>  CONFIG_NET_SCH_INGRESS=y
> +CONFIG_NET_SCH_HTB=y
>  CONFIG_NET_SCHED=y
>  CONFIG_NETDEVSIM=y
>  CONFIG_NETFILTER=y
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> index 7e8e3170e6b6..f3158170edff 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> @@ -86,18 +86,125 @@ static void test_fq(void)
>         bpf_qdisc_fq__destroy(fq_skel);
>  }
>
> +static int netdevsim_write_cmd(const char *path, const char *cmd)
> +{
> +       FILE *fp;
> +
> +       fp = fopen(path, "w");
> +       if (!ASSERT_OK_PTR(fp, "write_netdevsim_cmd"))
> +               return -errno;
> +
> +       fprintf(fp, cmd);
> +       fclose(fp);
> +       return 0;
> +}
> +

I will replace netdevsim with veth for attaching mq. The function
above that failed to compile in CI will also go.


> +static void test_qdisc_attach_to_mq(void)
> +{
> +       DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook,
> +                           .attach_point = BPF_TC_QDISC,
> +                           .parent = 0x00010001,
> +                           .handle = 0x8000000,
> +                           .qdisc = "bpf_fifo");
> +       struct bpf_qdisc_fifo *fifo_skel;
> +       struct bpf_link *link;
> +       int err;
> +
> +       hook.ifindex = if_nametoindex("eni1np1");
> +       if (!ASSERT_NEQ(hook.ifindex, 0, "if_nametoindex"))
> +               return;
> +
> +       fifo_skel = bpf_qdisc_fifo__open_and_load();
> +       if (!ASSERT_OK_PTR(fifo_skel, "bpf_qdisc_fifo__open_and_load"))
> +               return;
> +
> +       link = bpf_map__attach_struct_ops(fifo_skel->maps.fifo);
> +       if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
> +               bpf_qdisc_fifo__destroy(fifo_skel);
> +               return;
> +       }
> +
> +       ASSERT_OK(system("tc qdisc add dev eni1np1 root handle 1: mq"), "create mq");
> +
> +       err = bpf_tc_hook_create(&hook);
> +       ASSERT_OK(err, "attach qdisc");
> +
> +       bpf_tc_hook_destroy(&hook);
> +
> +       ASSERT_OK(system("tc qdisc delete dev eni1np1 root mq"), "delete mq");
> +
> +       bpf_link__destroy(link);
> +       bpf_qdisc_fifo__destroy(fifo_skel);
> +}
> +
> +static void test_qdisc_attach_to_non_root(void)
> +{
> +       DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX,
> +                           .attach_point = BPF_TC_QDISC,
> +                           .parent = 0x00010001,
> +                           .handle = 0x8000000,
> +                           .qdisc = "bpf_fifo");
> +       struct bpf_qdisc_fifo *fifo_skel;
> +       struct bpf_link *link;
> +       int err;
> +
> +       fifo_skel = bpf_qdisc_fifo__open_and_load();
> +       if (!ASSERT_OK_PTR(fifo_skel, "bpf_qdisc_fifo__open_and_load"))
> +               return;
> +
> +       link = bpf_map__attach_struct_ops(fifo_skel->maps.fifo);
> +       if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
> +               bpf_qdisc_fifo__destroy(fifo_skel);
> +               return;
> +       }
> +
> +       ASSERT_OK(system("tc qdisc add dev lo root handle 1: htb"), "create htb");
> +       ASSERT_OK(system("tc class add dev lo parent 1: classid 1:1 htb rate 75Kbit"), "create htb class");
> +
> +       err = bpf_tc_hook_create(&hook);
> +       ASSERT_ERR(err, "attach qdisc");
> +
> +       bpf_tc_hook_destroy(&hook);
> +
> +       ASSERT_OK(system("tc qdisc delete dev lo root htb"), "delete htb");
> +
> +       bpf_link__destroy(link);
> +       bpf_qdisc_fifo__destroy(fifo_skel);
> +}
> +
>  void test_bpf_qdisc(void)
>  {
> +       struct nstoken *nstoken = NULL;
>         struct netns_obj *netns;
> +       int err;
>
> -       netns = netns_new("bpf_qdisc_ns", true);
> +       netns = netns_new("bpf_qdisc_ns", false);
>         if (!ASSERT_OK_PTR(netns, "netns_new"))
>                 return;
>
> +       err = netdevsim_write_cmd("/sys/bus/netdevsim/new_device", "1 1 4");
> +       if (!ASSERT_OK(err, "create netdevsim")) {
> +               netns_free(netns);
> +               return;
> +       }
> +
> +       ASSERT_OK(system("ip link set eni1np1 netns bpf_qdisc_ns"), "ip link set netdevsim");
> +
> +       nstoken = open_netns("bpf_qdisc_ns");
> +       if (!ASSERT_OK_PTR(nstoken, "open_netns"))
> +               goto out;
> +
>         if (test__start_subtest("fifo"))
>                 test_fifo();
>         if (test__start_subtest("fq"))
>                 test_fq();
> -
> +       if (test__start_subtest("attach to mq"))
> +               test_qdisc_attach_to_mq();
> +       if (test__start_subtest("attach to non root"))
> +               test_qdisc_attach_to_non_root();
> +
> +out:
> +       err = netdevsim_write_cmd("/sys/bus/netdevsim/del_device", "1");
> +       ASSERT_OK(err, "delete netdevsim");
>         netns_free(netns);
>  }
> --
> 2.47.1
>
Eduard Zingerman Feb. 4, 2025, 5:58 a.m. UTC | #2 
On Fri, 2025-01-31 at 11:28 -0800, Amery Hung wrote:

[...]

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> index 7e8e3170e6b6..f3158170edff 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> @@ -86,18 +86,125 @@ static void test_fq(void)
>  	bpf_qdisc_fq__destroy(fq_skel);
>  }
>  
> +static int netdevsim_write_cmd(const char *path, const char *cmd)
> +{
> +	FILE *fp;
> +
> +	fp = fopen(path, "w");
> +	if (!ASSERT_OK_PTR(fp, "write_netdevsim_cmd"))
> +		return -errno;
> +
> +	fprintf(fp, cmd);

I get the following error message when compiling these tests using
clang 19.1.7:

<kernel>/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c:97:14: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
   97 |         fprintf(fp, cmd);
      |                     ^~~
<kernel>/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c:97:14: note: treat the string as an argument to avoid this
   97 |         fprintf(fp, cmd);
      |                     ^
      |   

> +	fclose(fp);
> +	return 0;
> +}
> +

[...]
Amery Hung Feb. 4, 2025, 6:17 p.m. UTC | #3 
On Mon, Feb 3, 2025 at 9:58 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2025-01-31 at 11:28 -0800, Amery Hung wrote:
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> > index 7e8e3170e6b6..f3158170edff 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
> > @@ -86,18 +86,125 @@ static void test_fq(void)
> >       bpf_qdisc_fq__destroy(fq_skel);
> >  }
> >
> > +static int netdevsim_write_cmd(const char *path, const char *cmd)
> > +{
> > +     FILE *fp;
> > +
> > +     fp = fopen(path, "w");
> > +     if (!ASSERT_OK_PTR(fp, "write_netdevsim_cmd"))
> > +             return -errno;
> > +
> > +     fprintf(fp, cmd);
>
> I get the following error message when compiling these tests using
> clang 19.1.7:
>
> <kernel>/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c:97:14: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
>    97 |         fprintf(fp, cmd);
>       |                     ^~~
> <kernel>/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c:97:14: note: treat the string as an argument to avoid this
>    97 |         fprintf(fp, cmd);
>       |                     ^
>       |
>

I am removing the use of netdevsim and along with this function in the
next version. The warning should also disappear. Thanks for the
review!

> > +     fclose(fp);
> > +     return 0;
> > +}
> > +
>
> [...]
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 6b0cab55bd2d..3201a962b3dc 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -74,6 +74,7 @@  CONFIG_NET_MPLS_GSO=y
 CONFIG_NET_SCH_BPF=y
 CONFIG_NET_SCH_FQ=y
 CONFIG_NET_SCH_INGRESS=y
+CONFIG_NET_SCH_HTB=y
 CONFIG_NET_SCHED=y
 CONFIG_NETDEVSIM=y
 CONFIG_NETFILTER=y
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
index 7e8e3170e6b6..f3158170edff 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
@@ -86,18 +86,125 @@  static void test_fq(void)
 	bpf_qdisc_fq__destroy(fq_skel);
 }
 
+static int netdevsim_write_cmd(const char *path, const char *cmd)
+{
+	FILE *fp;
+
+	fp = fopen(path, "w");
+	if (!ASSERT_OK_PTR(fp, "write_netdevsim_cmd"))
+		return -errno;
+
+	fprintf(fp, cmd);
+	fclose(fp);
+	return 0;
+}
+
+static void test_qdisc_attach_to_mq(void)
+{
+	DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook,
+			    .attach_point = BPF_TC_QDISC,
+			    .parent = 0x00010001,
+			    .handle = 0x8000000,
+			    .qdisc = "bpf_fifo");
+	struct bpf_qdisc_fifo *fifo_skel;
+	struct bpf_link *link;
+	int err;
+
+	hook.ifindex = if_nametoindex("eni1np1");
+	if (!ASSERT_NEQ(hook.ifindex, 0, "if_nametoindex"))
+		return;
+
+	fifo_skel = bpf_qdisc_fifo__open_and_load();
+	if (!ASSERT_OK_PTR(fifo_skel, "bpf_qdisc_fifo__open_and_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(fifo_skel->maps.fifo);
+	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
+		bpf_qdisc_fifo__destroy(fifo_skel);
+		return;
+	}
+
+	ASSERT_OK(system("tc qdisc add dev eni1np1 root handle 1: mq"), "create mq");
+
+	err = bpf_tc_hook_create(&hook);
+	ASSERT_OK(err, "attach qdisc");
+
+	bpf_tc_hook_destroy(&hook);
+
+	ASSERT_OK(system("tc qdisc delete dev eni1np1 root mq"), "delete mq");
+
+	bpf_link__destroy(link);
+	bpf_qdisc_fifo__destroy(fifo_skel);
+}
+
+static void test_qdisc_attach_to_non_root(void)
+{
+	DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX,
+			    .attach_point = BPF_TC_QDISC,
+			    .parent = 0x00010001,
+			    .handle = 0x8000000,
+			    .qdisc = "bpf_fifo");
+	struct bpf_qdisc_fifo *fifo_skel;
+	struct bpf_link *link;
+	int err;
+
+	fifo_skel = bpf_qdisc_fifo__open_and_load();
+	if (!ASSERT_OK_PTR(fifo_skel, "bpf_qdisc_fifo__open_and_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(fifo_skel->maps.fifo);
+	if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops")) {
+		bpf_qdisc_fifo__destroy(fifo_skel);
+		return;
+	}
+
+	ASSERT_OK(system("tc qdisc add dev lo root handle 1: htb"), "create htb");
+	ASSERT_OK(system("tc class add dev lo parent 1: classid 1:1 htb rate 75Kbit"), "create htb class");
+
+	err = bpf_tc_hook_create(&hook);
+	ASSERT_ERR(err, "attach qdisc");
+
+	bpf_tc_hook_destroy(&hook);
+
+	ASSERT_OK(system("tc qdisc delete dev lo root htb"), "delete htb");
+
+	bpf_link__destroy(link);
+	bpf_qdisc_fifo__destroy(fifo_skel);
+}
+
 void test_bpf_qdisc(void)
 {
+	struct nstoken *nstoken = NULL;
 	struct netns_obj *netns;
+	int err;
 
-	netns = netns_new("bpf_qdisc_ns", true);
+	netns = netns_new("bpf_qdisc_ns", false);
 	if (!ASSERT_OK_PTR(netns, "netns_new"))
 		return;
 
+	err = netdevsim_write_cmd("/sys/bus/netdevsim/new_device", "1 1 4");
+	if (!ASSERT_OK(err, "create netdevsim")) {
+		netns_free(netns);
+		return;
+	}
+
+	ASSERT_OK(system("ip link set eni1np1 netns bpf_qdisc_ns"), "ip link set netdevsim");
+
+	nstoken = open_netns("bpf_qdisc_ns");
+	if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+		goto out;
+
 	if (test__start_subtest("fifo"))
 		test_fifo();
 	if (test__start_subtest("fq"))
 		test_fq();
-
+	if (test__start_subtest("attach to mq"))
+		test_qdisc_attach_to_mq();
+	if (test__start_subtest("attach to non root"))
+		test_qdisc_attach_to_non_root();
+
+out:
+	err = netdevsim_write_cmd("/sys/bus/netdevsim/del_device", "1");
+	ASSERT_OK(err, "delete netdevsim");
 	netns_free(netns);
 }