[PATCH v5 0/4] net/sched: Load modules via alias

Michal Koutný posted 4 patches 1 year, 10 months ago
include/net/act_api.h      | 2 ++
include/net/pkt_cls.h      | 2 ++
include/net/pkt_sched.h    | 2 ++
net/sched/act_api.c        | 2 +-
net/sched/act_bpf.c        | 1 +
net/sched/act_connmark.c   | 1 +
net/sched/act_csum.c       | 1 +
net/sched/act_ct.c         | 1 +
net/sched/act_ctinfo.c     | 1 +
net/sched/act_gact.c       | 1 +
net/sched/act_gate.c       | 1 +
net/sched/act_ife.c        | 1 +
net/sched/act_mirred.c     | 1 +
net/sched/act_mpls.c       | 1 +
net/sched/act_nat.c        | 1 +
net/sched/act_pedit.c      | 1 +
net/sched/act_police.c     | 1 +
net/sched/act_sample.c     | 1 +
net/sched/act_simple.c     | 1 +
net/sched/act_skbedit.c    | 1 +
net/sched/act_skbmod.c     | 1 +
net/sched/act_tunnel_key.c | 1 +
net/sched/act_vlan.c       | 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 +
net/sched/sch_api.c        | 4 ++--
net/sched/sch_cake.c       | 1 +
net/sched/sch_cbs.c        | 1 +
net/sched/sch_choke.c      | 1 +
net/sched/sch_codel.c      | 1 +
net/sched/sch_drr.c        | 1 +
net/sched/sch_etf.c        | 1 +
net/sched/sch_ets.c        | 1 +
net/sched/sch_fq.c         | 1 +
net/sched/sch_fq_codel.c   | 1 +
net/sched/sch_gred.c       | 1 +
net/sched/sch_hfsc.c       | 1 +
net/sched/sch_hhf.c        | 1 +
net/sched/sch_htb.c        | 1 +
net/sched/sch_ingress.c    | 3 ++-
net/sched/sch_mqprio.c     | 1 +
net/sched/sch_multiq.c     | 1 +
net/sched/sch_netem.c      | 1 +
net/sched/sch_pie.c        | 1 +
net/sched/sch_plug.c       | 1 +
net/sched/sch_prio.c       | 1 +
net/sched/sch_qfq.c        | 1 +
net/sched/sch_red.c        | 1 +
net/sched/sch_sfb.c        | 1 +
net/sched/sch_sfq.c        | 1 +
net/sched/sch_skbprio.c    | 1 +
net/sched/sch_taprio.c     | 1 +
net/sched/sch_tbf.c        | 1 +
61 files changed, 66 insertions(+), 5 deletions(-)
[PATCH v5 0/4] net/sched: Load modules via alias
Posted by Michal Koutný 1 year, 10 months ago
These 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 (through aliases)
works for them. (The same pattern exists e.g. for filesystem
modules.)

For example (before the change):
  $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
  # cls_cgroup module is loaded despite a `blacklist cls_cgroup` entry
  # in /etc/modprobe.d/*.conf

After the change:
  $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
  Error: TC classifier not found.
  We have an error talking to the kernel
  # explicit/acknowledged (privileged) action is needed
  $ modprobe cls_cgroup
  # blacklist entry won't apply to this direct modprobe, module is
  # loaded with awareness

A considered alternative was invoking `modprobe -b` always from
request_module(), however, dismissed as too intrusive and slightly
confusing in favor of the precedented aliases (the commit 7f78e0351394
("fs: Limit sys_mount to only request filesystem modules.").

User experience suffers in both alternatives. Its improvement is
orthogonal to blacklist honoring.

Changes from v1 (https://lore.kernel.org/r/20231121175640.9981-1-mkoutny@suse.com)
- Treat sch_ and act_ modules analogously to cls_

Changes from v2 (https://lore.kernel.org/r/20231206192752.18989-1-mkoutny@suse.com)
- reorganized commits (one generated commit + manual pre-/post- work)
- used alias names more fitting the existing net- aliases
- more info in commit messages and cover letter
- rebased on current master

Changes from v3 (https://lore.kernel.org/r/20240112180646.13232-1-mkoutny@suse.com)
- rebase on netdev/net-next/main
- correct aliases in cls_* modules (wrong sed)
- replace repeated prefix strings with a macro
- patch also request_module call in qdisc_set_default()

Changes from v4 (https://lore.kernel.org/r/20240123135242.11430-1-mkoutny@suse.com)
- update example in cover letter to existing module (cls_tcindex->cls_cgroup)
  - tested that ':-)
- remove __stringify in alias macro, net-cls-cgroup instead of net-cls-"cgroup"
- pass correct argument to request_module() (Simon)
- rebased on netdev-next/main

Michal Koutný (4):
  net/sched: Add helper macros with module names
  net/sched: Add module aliases for cls_,sch_,act_ modules
  net/sched: Load modules via their alias
  net/sched: Remove alias of sch_clsact

 include/net/act_api.h      | 2 ++
 include/net/pkt_cls.h      | 2 ++
 include/net/pkt_sched.h    | 2 ++
 net/sched/act_api.c        | 2 +-
 net/sched/act_bpf.c        | 1 +
 net/sched/act_connmark.c   | 1 +
 net/sched/act_csum.c       | 1 +
 net/sched/act_ct.c         | 1 +
 net/sched/act_ctinfo.c     | 1 +
 net/sched/act_gact.c       | 1 +
 net/sched/act_gate.c       | 1 +
 net/sched/act_ife.c        | 1 +
 net/sched/act_mirred.c     | 1 +
 net/sched/act_mpls.c       | 1 +
 net/sched/act_nat.c        | 1 +
 net/sched/act_pedit.c      | 1 +
 net/sched/act_police.c     | 1 +
 net/sched/act_sample.c     | 1 +
 net/sched/act_simple.c     | 1 +
 net/sched/act_skbedit.c    | 1 +
 net/sched/act_skbmod.c     | 1 +
 net/sched/act_tunnel_key.c | 1 +
 net/sched/act_vlan.c       | 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 +
 net/sched/sch_api.c        | 4 ++--
 net/sched/sch_cake.c       | 1 +
 net/sched/sch_cbs.c        | 1 +
 net/sched/sch_choke.c      | 1 +
 net/sched/sch_codel.c      | 1 +
 net/sched/sch_drr.c        | 1 +
 net/sched/sch_etf.c        | 1 +
 net/sched/sch_ets.c        | 1 +
 net/sched/sch_fq.c         | 1 +
 net/sched/sch_fq_codel.c   | 1 +
 net/sched/sch_gred.c       | 1 +
 net/sched/sch_hfsc.c       | 1 +
 net/sched/sch_hhf.c        | 1 +
 net/sched/sch_htb.c        | 1 +
 net/sched/sch_ingress.c    | 3 ++-
 net/sched/sch_mqprio.c     | 1 +
 net/sched/sch_multiq.c     | 1 +
 net/sched/sch_netem.c      | 1 +
 net/sched/sch_pie.c        | 1 +
 net/sched/sch_plug.c       | 1 +
 net/sched/sch_prio.c       | 1 +
 net/sched/sch_qfq.c        | 1 +
 net/sched/sch_red.c        | 1 +
 net/sched/sch_sfb.c        | 1 +
 net/sched/sch_sfq.c        | 1 +
 net/sched/sch_skbprio.c    | 1 +
 net/sched/sch_taprio.c     | 1 +
 net/sched/sch_tbf.c        | 1 +
 61 files changed, 66 insertions(+), 5 deletions(-)


base-commit: 644c64318de0df5f4ea9f00e4b8cc262b343a93f
-- 
2.43.0

Re: [PATCH v5 0/4] net/sched: Load modules via alias
Posted by Jiri Pirko 1 year, 10 months ago
Thu, Feb 01, 2024 at 02:09:39PM CET, mkoutny@suse.com wrote:
>These 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 (through aliases)
>works for them. (The same pattern exists e.g. for filesystem
>modules.)
>
>For example (before the change):
>  $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
>  # cls_cgroup module is loaded despite a `blacklist cls_cgroup` entry
>  # in /etc/modprobe.d/*.conf
>
>After the change:
>  $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
>  Error: TC classifier not found.
>  We have an error talking to the kernel
>  # explicit/acknowledged (privileged) action is needed
>  $ modprobe cls_cgroup
>  # blacklist entry won't apply to this direct modprobe, module is
>  # loaded with awareness
>
>A considered alternative was invoking `modprobe -b` always from
>request_module(), however, dismissed as too intrusive and slightly
>confusing in favor of the precedented aliases (the commit 7f78e0351394
>("fs: Limit sys_mount to only request filesystem modules.").
>
>User experience suffers in both alternatives. Its improvement is
>orthogonal to blacklist honoring.
>
>Changes from v1 (https://lore.kernel.org/r/20231121175640.9981-1-mkoutny@suse.com)
>- Treat sch_ and act_ modules analogously to cls_
>
>Changes from v2 (https://lore.kernel.org/r/20231206192752.18989-1-mkoutny@suse.com)
>- reorganized commits (one generated commit + manual pre-/post- work)
>- used alias names more fitting the existing net- aliases
>- more info in commit messages and cover letter
>- rebased on current master
>
>Changes from v3 (https://lore.kernel.org/r/20240112180646.13232-1-mkoutny@suse.com)
>- rebase on netdev/net-next/main
>- correct aliases in cls_* modules (wrong sed)
>- replace repeated prefix strings with a macro
>- patch also request_module call in qdisc_set_default()
>
>Changes from v4 (https://lore.kernel.org/r/20240123135242.11430-1-mkoutny@suse.com)
>- update example in cover letter to existing module (cls_tcindex->cls_cgroup)
>  - tested that ':-)
>- remove __stringify in alias macro, net-cls-cgroup instead of net-cls-"cgroup"
>- pass correct argument to request_module() (Simon)
>- rebased on netdev-next/main
>
>Michal Koutný (4):
>  net/sched: Add helper macros with module names
>  net/sched: Add module aliases for cls_,sch_,act_ modules
>  net/sched: Load modules via their alias
>  net/sched: Remove alias of sch_clsact

Set looks fine to me:

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Re: [PATCH v5 0/4] net/sched: Load modules via alias
Posted by Jamal Hadi Salim 1 year, 10 months ago
On Thu, Feb 1, 2024 at 8:09 AM Michal Koutný <mkoutny@suse.com> wrote:
>
> These 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 (through aliases)
> works for them. (The same pattern exists e.g. for filesystem
> modules.)
>
> For example (before the change):
>   $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
>   # cls_cgroup module is loaded despite a `blacklist cls_cgroup` entry
>   # in /etc/modprobe.d/*.conf
>
> After the change:
>   $ tc filter add dev lo parent 10: protocol ip prio 10 handle 1: cgroup
>   Error: TC classifier not found.
>   We have an error talking to the kernel
>   # explicit/acknowledged (privileged) action is needed
>   $ modprobe cls_cgroup
>   # blacklist entry won't apply to this direct modprobe, module is
>   # loaded with awareness
>
> A considered alternative was invoking `modprobe -b` always from
> request_module(), however, dismissed as too intrusive and slightly
> confusing in favor of the precedented aliases (the commit 7f78e0351394
> ("fs: Limit sys_mount to only request filesystem modules.").
>
> User experience suffers in both alternatives. Its improvement is
> orthogonal to blacklist honoring.
>

Thanks Michal. We still didnt hear from Stephen, but i think looks
solid now. So for the patchset:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

> Changes from v1 (https://lore.kernel.org/r/20231121175640.9981-1-mkoutny@suse.com)
> - Treat sch_ and act_ modules analogously to cls_
>
> Changes from v2 (https://lore.kernel.org/r/20231206192752.18989-1-mkoutny@suse.com)
> - reorganized commits (one generated commit + manual pre-/post- work)
> - used alias names more fitting the existing net- aliases
> - more info in commit messages and cover letter
> - rebased on current master
>
> Changes from v3 (https://lore.kernel.org/r/20240112180646.13232-1-mkoutny@suse.com)
> - rebase on netdev/net-next/main
> - correct aliases in cls_* modules (wrong sed)
> - replace repeated prefix strings with a macro
> - patch also request_module call in qdisc_set_default()
>
> Changes from v4 (https://lore.kernel.org/r/20240123135242.11430-1-mkoutny@suse.com)
> - update example in cover letter to existing module (cls_tcindex->cls_cgroup)
>   - tested that ':-)
> - remove __stringify in alias macro, net-cls-cgroup instead of net-cls-"cgroup"
> - pass correct argument to request_module() (Simon)
> - rebased on netdev-next/main
>
> Michal Koutný (4):
>   net/sched: Add helper macros with module names
>   net/sched: Add module aliases for cls_,sch_,act_ modules
>   net/sched: Load modules via their alias
>   net/sched: Remove alias of sch_clsact
>
>  include/net/act_api.h      | 2 ++
>  include/net/pkt_cls.h      | 2 ++
>  include/net/pkt_sched.h    | 2 ++
>  net/sched/act_api.c        | 2 +-
>  net/sched/act_bpf.c        | 1 +
>  net/sched/act_connmark.c   | 1 +
>  net/sched/act_csum.c       | 1 +
>  net/sched/act_ct.c         | 1 +
>  net/sched/act_ctinfo.c     | 1 +
>  net/sched/act_gact.c       | 1 +
>  net/sched/act_gate.c       | 1 +
>  net/sched/act_ife.c        | 1 +
>  net/sched/act_mirred.c     | 1 +
>  net/sched/act_mpls.c       | 1 +
>  net/sched/act_nat.c        | 1 +
>  net/sched/act_pedit.c      | 1 +
>  net/sched/act_police.c     | 1 +
>  net/sched/act_sample.c     | 1 +
>  net/sched/act_simple.c     | 1 +
>  net/sched/act_skbedit.c    | 1 +
>  net/sched/act_skbmod.c     | 1 +
>  net/sched/act_tunnel_key.c | 1 +
>  net/sched/act_vlan.c       | 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 +
>  net/sched/sch_api.c        | 4 ++--
>  net/sched/sch_cake.c       | 1 +
>  net/sched/sch_cbs.c        | 1 +
>  net/sched/sch_choke.c      | 1 +
>  net/sched/sch_codel.c      | 1 +
>  net/sched/sch_drr.c        | 1 +
>  net/sched/sch_etf.c        | 1 +
>  net/sched/sch_ets.c        | 1 +
>  net/sched/sch_fq.c         | 1 +
>  net/sched/sch_fq_codel.c   | 1 +
>  net/sched/sch_gred.c       | 1 +
>  net/sched/sch_hfsc.c       | 1 +
>  net/sched/sch_hhf.c        | 1 +
>  net/sched/sch_htb.c        | 1 +
>  net/sched/sch_ingress.c    | 3 ++-
>  net/sched/sch_mqprio.c     | 1 +
>  net/sched/sch_multiq.c     | 1 +
>  net/sched/sch_netem.c      | 1 +
>  net/sched/sch_pie.c        | 1 +
>  net/sched/sch_plug.c       | 1 +
>  net/sched/sch_prio.c       | 1 +
>  net/sched/sch_qfq.c        | 1 +
>  net/sched/sch_red.c        | 1 +
>  net/sched/sch_sfb.c        | 1 +
>  net/sched/sch_sfq.c        | 1 +
>  net/sched/sch_skbprio.c    | 1 +
>  net/sched/sch_taprio.c     | 1 +
>  net/sched/sch_tbf.c        | 1 +
>  61 files changed, 66 insertions(+), 5 deletions(-)
>
>
> base-commit: 644c64318de0df5f4ea9f00e4b8cc262b343a93f
> --
> 2.43.0
>