diff mbox series

net/sched: cls: Load net classifier modules via alias

Message ID 20231121175640.9981-1-mkoutny@suse.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series net/sched: cls: Load net classifier modules via alias | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/codegen success Generated files up to date
netdev/tree_selection success Guessed tree name to be net-next
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 success Errors and warnings before: 1182 this patch: 1182
netdev/cc_maintainers success CCed 19 of 19 maintainers
netdev/build_clang success Errors and warnings before: 1155 this patch: 1155
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: 1236 this patch: 1236
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 78 lines checked
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-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-3 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-16 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-16 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-16 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-16 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-10 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc

Commit Message

Michal Koutný Nov. 21, 2023, 5:56 p.m. UTC 
The classifier modules may be loaded lazily without user's awareness and
control. Add respective aliases to modules and request them under these
aliases so that modprobe's blacklisting mechanism works also for
classifier modules. (The same pattern exists e.g. for filesystem
modules.)

Original module names remain unchanged.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 include/net/pkt_cls.h    | 1 +
 net/sched/cls_api.c      | 2 +-
 net/sched/cls_basic.c    | 1 +
 net/sched/cls_bpf.c      | 1 +
 net/sched/cls_cgroup.c   | 1 +
 net/sched/cls_flow.c     | 1 +
 net/sched/cls_flower.c   | 1 +
 net/sched/cls_fw.c       | 1 +
 net/sched/cls_matchall.c | 1 +
 net/sched/cls_route.c    | 1 +
 net/sched/cls_u32.c      | 1 +
 11 files changed, 11 insertions(+), 1 deletion(-)

This is primarily for TC subsystem maintainers where the
request_module() resides but Cc list is large because of touches in
various classifier modules.

Comments

Jamal Hadi Salim Nov. 21, 2023, 10:37 p.m. UTC | #1 
On Tue, Nov 21, 2023 at 12:56 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> The classifier modules may be loaded lazily without user's awareness and
> control. Add respective aliases to modules and request them under these
> aliases so that modprobe's blacklisting mechanism works also for
> classifier modules. (The same pattern exists e.g. for filesystem
> modules.)
>

Hi Michal,
Dumb question: What's speacial about the "tcf- '' that makes it work
better for filtering than existing "cls_" prefix? What about actions
(prefix "act_") etc?

cheers,
jamal

> Original module names remain unchanged.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>  include/net/pkt_cls.h    | 1 +
>  net/sched/cls_api.c      | 2 +-
>  net/sched/cls_basic.c    | 1 +
>  net/sched/cls_bpf.c      | 1 +
>  net/sched/cls_cgroup.c   | 1 +
>  net/sched/cls_flow.c     | 1 +
>  net/sched/cls_flower.c   | 1 +
>  net/sched/cls_fw.c       | 1 +
>  net/sched/cls_matchall.c | 1 +
>  net/sched/cls_route.c    | 1 +
>  net/sched/cls_u32.c      | 1 +
>  11 files changed, 11 insertions(+), 1 deletion(-)
>
> This is primarily for TC subsystem maintainers where the
> request_module() resides but Cc list is large because of touches in
> various classifier modules.
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index a76c9171db0e..424b4f889feb 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -24,6 +24,7 @@ struct tcf_walker {
>
>  int register_tcf_proto_ops(struct tcf_proto_ops *ops);
>  void unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
> +#define MODULE_ALIAS_TCF(kind) MODULE_ALIAS("tcf-" __stringify(kind))
>
>  struct tcf_block_ext_info {
>         enum flow_block_binder_type binder_type;
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 1976bd163986..02fdcceee083 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -257,7 +257,7 @@ tcf_proto_lookup_ops(const char *kind, bool rtnl_held,
>  #ifdef CONFIG_MODULES
>         if (rtnl_held)
>                 rtnl_unlock();
> -       request_module("cls_%s", kind);
> +       request_module("tcf-%s", kind);
>         if (rtnl_held)
>                 rtnl_lock();
>         ops = __tcf_proto_lookup_ops(kind);
> diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
> index a1f56931330c..a3500ac7fc1a 100644
> --- a/net/sched/cls_basic.c
> +++ b/net/sched/cls_basic.c
> @@ -328,6 +328,7 @@ static struct tcf_proto_ops cls_basic_ops __read_mostly = {
>         .bind_class     =       basic_bind_class,
>         .owner          =       THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("basic");
>
>  static int __init init_basic(void)
>  {
> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 382c7a71f81f..8d57ac155c0c 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -693,6 +693,7 @@ static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
>         .dump           =       cls_bpf_dump,
>         .bind_class     =       cls_bpf_bind_class,
>  };
> +MODULE_ALIAS_TCF("bpf");
>
>  static int __init cls_bpf_init_mod(void)
>  {
> diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
> index 7ee8dbf49ed0..0ded7d79894c 100644
> --- a/net/sched/cls_cgroup.c
> +++ b/net/sched/cls_cgroup.c
> @@ -209,6 +209,7 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
>         .dump           =       cls_cgroup_dump,
>         .owner          =       THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("cgroup");
>
>  static int __init init_cgroup_cls(void)
>  {
> diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
> index 6ab317b48d6c..2806aa1254e1 100644
> --- a/net/sched/cls_flow.c
> +++ b/net/sched/cls_flow.c
> @@ -702,6 +702,7 @@ static struct tcf_proto_ops cls_flow_ops __read_mostly = {
>         .walk           = flow_walk,
>         .owner          = THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("flow");
>
>  static int __init cls_flow_init(void)
>  {
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index e5314a31f75a..739e09e0fa57 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -3633,6 +3633,7 @@ static struct tcf_proto_ops cls_fl_ops __read_mostly = {
>         .owner          = THIS_MODULE,
>         .flags          = TCF_PROTO_OPS_DOIT_UNLOCKED,
>  };
> +MODULE_ALIAS_TCF("flower");
>
>  static int __init cls_fl_init(void)
>  {
> diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
> index afc534ee0a18..86c833885a2d 100644
> --- a/net/sched/cls_fw.c
> +++ b/net/sched/cls_fw.c
> @@ -433,6 +433,7 @@ static struct tcf_proto_ops cls_fw_ops __read_mostly = {
>         .bind_class     =       fw_bind_class,
>         .owner          =       THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("fw");
>
>  static int __init init_fw(void)
>  {
> diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
> index c4ed11df6254..21ba73978c6a 100644
> --- a/net/sched/cls_matchall.c
> +++ b/net/sched/cls_matchall.c
> @@ -398,6 +398,7 @@ static struct tcf_proto_ops cls_mall_ops __read_mostly = {
>         .bind_class     = mall_bind_class,
>         .owner          = THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("matchall");
>
>  static int __init cls_mall_init(void)
>  {
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 12a505db4183..a4701c0752df 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -671,6 +671,7 @@ static struct tcf_proto_ops cls_route4_ops __read_mostly = {
>         .bind_class     =       route4_bind_class,
>         .owner          =       THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("route");
>
>  static int __init init_route4(void)
>  {
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index d5bdfd4a7655..a969adbd7423 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -1453,6 +1453,7 @@ static struct tcf_proto_ops cls_u32_ops __read_mostly = {
>         .bind_class     =       u32_bind_class,
>         .owner          =       THIS_MODULE,
>  };
> +MODULE_ALIAS_TCF("u32");
>
>  static int __init init_u32(void)
>  {
> --
> 2.42.1
>
Michal Koutný Nov. 22, 2023, 10:41 a.m. UTC | #2 
On Tue, Nov 21, 2023 at 05:37:37PM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> What's speacial about the "tcf- '' that makes it work
> better for filtering than existing "cls_" prefix?

tcf-foo is an alias.
cls_foo is the canonical name of the kernel module.

request_module() + blacklist (as described in modprobe.d(5)) works only
when calling with the alias. The actual string is not important, being
an alias is the crux.

> What about actions (prefix "act_") etc?

I focused only on "cls_" for the first iteration. Do you want me to look
at other analogous loads?

Thanks,
Michal
Jamal Hadi Salim Nov. 22, 2023, 3:33 p.m. UTC | #3 
On Wed, Nov 22, 2023 at 5:41 AM Michal Koutný <mkoutny@suse.com> wrote:
>
> On Tue, Nov 21, 2023 at 05:37:37PM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > What's speacial about the "tcf- '' that makes it work
> > better for filtering than existing "cls_" prefix?
>
> tcf-foo is an alias.
> cls_foo is the canonical name of the kernel module.
>
> request_module() + blacklist (as described in modprobe.d(5)) works only
> when calling with the alias. The actual string is not important, being
> an alias is the crux.
>

Thanks for the explanation.

> > What about actions (prefix "act_") etc?
>
> I focused only on "cls_" for the first iteration. Do you want me to look
> at other analogous loads?

Yes, look at act_ and sch_

cheers,
jamal
> Thanks,
> Michal
Jamal Hadi Salim Nov. 22, 2023, 3:55 p.m. UTC | #4 
On Wed, Nov 22, 2023 at 10:33 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Wed, Nov 22, 2023 at 5:41 AM Michal Koutný <mkoutny@suse.com> wrote:
> >
> > On Tue, Nov 21, 2023 at 05:37:37PM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > What's speacial about the "tcf- '' that makes it work
> > > better for filtering than existing "cls_" prefix?
> >
> > tcf-foo is an alias.
> > cls_foo is the canonical name of the kernel module.
> >
> > request_module() + blacklist (as described in modprobe.d(5)) works only
> > when calling with the alias. The actual string is not important, being
> > an alias is the crux.
> >
>
> Thanks for the explanation.
>

Out of curiosity - how did you end up looking at this? Was there
someone who complained or is it just standard procedure to add aliases
to all modules and it was on a todo list somewhere?

cheers,
jamal

> > > What about actions (prefix "act_") etc?
> >
> > I focused only on "cls_" for the first iteration. Do you want me to look
> > at other analogous loads?
>
> Yes, look at act_ and sch_
>
> cheers,
> jamal
> > Thanks,
> > Michal
Michal Koutný Nov. 22, 2023, 4:11 p.m. UTC | #5 
On Wed, Nov 22, 2023 at 10:55:29AM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> Out of curiosity - how did you end up looking at this?

The trigger for this case was tcindex module -- removed in upstream, so
we don't want to have implicit autoloads for it while we retain it in
our distro.

The user has to modprobe explicitly or un-blacklist the module.

(For instance, we've been leveraging this for the fs-* modules due to
various support status of filesystem modules in SLE.)


Michal
diff mbox series

Patch

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index a76c9171db0e..424b4f889feb 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -24,6 +24,7 @@  struct tcf_walker {
 
 int register_tcf_proto_ops(struct tcf_proto_ops *ops);
 void unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
+#define MODULE_ALIAS_TCF(kind)	MODULE_ALIAS("tcf-" __stringify(kind))
 
 struct tcf_block_ext_info {
 	enum flow_block_binder_type binder_type;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 1976bd163986..02fdcceee083 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -257,7 +257,7 @@  tcf_proto_lookup_ops(const char *kind, bool rtnl_held,
 #ifdef CONFIG_MODULES
 	if (rtnl_held)
 		rtnl_unlock();
-	request_module("cls_%s", kind);
+	request_module("tcf-%s", kind);
 	if (rtnl_held)
 		rtnl_lock();
 	ops = __tcf_proto_lookup_ops(kind);
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index a1f56931330c..a3500ac7fc1a 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -328,6 +328,7 @@  static struct tcf_proto_ops cls_basic_ops __read_mostly = {
 	.bind_class	=	basic_bind_class,
 	.owner		=	THIS_MODULE,
 };
+MODULE_ALIAS_TCF("basic");
 
 static int __init init_basic(void)
 {
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 382c7a71f81f..8d57ac155c0c 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -693,6 +693,7 @@  static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
 	.dump		=	cls_bpf_dump,
 	.bind_class	=	cls_bpf_bind_class,
 };
+MODULE_ALIAS_TCF("bpf");
 
 static int __init cls_bpf_init_mod(void)
 {
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 7ee8dbf49ed0..0ded7d79894c 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -209,6 +209,7 @@  static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
 	.dump		=	cls_cgroup_dump,
 	.owner		=	THIS_MODULE,
 };
+MODULE_ALIAS_TCF("cgroup");
 
 static int __init init_cgroup_cls(void)
 {
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 6ab317b48d6c..2806aa1254e1 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -702,6 +702,7 @@  static struct tcf_proto_ops cls_flow_ops __read_mostly = {
 	.walk		= flow_walk,
 	.owner		= THIS_MODULE,
 };
+MODULE_ALIAS_TCF("flow");
 
 static int __init cls_flow_init(void)
 {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e5314a31f75a..739e09e0fa57 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -3633,6 +3633,7 @@  static struct tcf_proto_ops cls_fl_ops __read_mostly = {
 	.owner		= THIS_MODULE,
 	.flags		= TCF_PROTO_OPS_DOIT_UNLOCKED,
 };
+MODULE_ALIAS_TCF("flower");
 
 static int __init cls_fl_init(void)
 {
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index afc534ee0a18..86c833885a2d 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -433,6 +433,7 @@  static struct tcf_proto_ops cls_fw_ops __read_mostly = {
 	.bind_class	=	fw_bind_class,
 	.owner		=	THIS_MODULE,
 };
+MODULE_ALIAS_TCF("fw");
 
 static int __init init_fw(void)
 {
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index c4ed11df6254..21ba73978c6a 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -398,6 +398,7 @@  static struct tcf_proto_ops cls_mall_ops __read_mostly = {
 	.bind_class	= mall_bind_class,
 	.owner		= THIS_MODULE,
 };
+MODULE_ALIAS_TCF("matchall");
 
 static int __init cls_mall_init(void)
 {
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 12a505db4183..a4701c0752df 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -671,6 +671,7 @@  static struct tcf_proto_ops cls_route4_ops __read_mostly = {
 	.bind_class	=	route4_bind_class,
 	.owner		=	THIS_MODULE,
 };
+MODULE_ALIAS_TCF("route");
 
 static int __init init_route4(void)
 {
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index d5bdfd4a7655..a969adbd7423 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1453,6 +1453,7 @@  static struct tcf_proto_ops cls_u32_ops __read_mostly = {
 	.bind_class	=	u32_bind_class,
 	.owner		=	THIS_MODULE,
 };
+MODULE_ALIAS_TCF("u32");
 
 static int __init init_u32(void)
 {