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

From: Jamal Hadi Salim
Date: Fri Feb 02 2024 - 07:08:40 EST


On Thu, Feb 1, 2024 at 8:09 AM Michal Koutný <mkoutny@xxxxxxxx> 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@xxxxxxxxxxxx>

cheers,
jamal

> Changes from v1 (https://lore.kernel.org/r/20231121175640.9981-1-mkoutny@xxxxxxxx)
> - Treat sch_ and act_ modules analogously to cls_
>
> Changes from v2 (https://lore.kernel.org/r/20231206192752.18989-1-mkoutny@xxxxxxxx)
> - 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@xxxxxxxx)
> - 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@xxxxxxxx)
> - 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
>