mbox series

[v4,0/4] net/sched: Load modules via alias

Message ID 20240123135242.11430-1-mkoutny@suse.com (mailing list archive)
Headers show
Series net/sched: Load modules via alias | expand

Message

Michal Koutný Jan. 23, 2024, 1:52 p.m. UTC 
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 1: protocol ip prio 1 handle 10 tcindex ...
  # cls_tcindex module is loaded despite a `blacklist cls_tcindex` entry
  # in /etc/modprobe.d/*.conf

After the change:
  $ tc filter add dev lo parent 1: protocol ip prio 1 handle 10 tcindex ...
  Unknown filter "tcindex", hence option "..." is unparsable
  # explicit/acknowledged (privileged) action is needed
  $ modprobe cls_tcindex
  # 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. It's 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()

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: 736b5545d39ca59d4332a60e56cc8a1a5e264a8e

Comments

Jamal Hadi Salim Jan. 24, 2024, 12:17 p.m. UTC | #1 
Hi Michal,

On Tue, Jan 23, 2024 at 8:52 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 1: protocol ip prio 1 handle 10 tcindex ...
>   # cls_tcindex module is loaded despite a `blacklist cls_tcindex` entry
>   # in /etc/modprobe.d/*.conf
>
> After the change:
>   $ tc filter add dev lo parent 1: protocol ip prio 1 handle 10 tcindex ...
>   Unknown filter "tcindex", hence option "..." is unparsable
>   # explicit/acknowledged (privileged) action is needed
>   $ modprobe cls_tcindex
>   # blacklist entry won't apply to this direct modprobe, module is
>   # loaded with awareness
>

A small nit seeing Simon's comment which will have you respin.
cls_tcindex is no longer in the kernel. Can you use another example?
Also Stephen had some comments last time, not sure if you addressed
those (nothing on the logs says you did and i didnt see him say
anything).

cheers,
jamal

> 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. It's 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()
>
> 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: 736b5545d39ca59d4332a60e56cc8a1a5e264a8e
> --
> 2.43.0
>
Michal Koutný Jan. 24, 2024, 1:19 p.m. UTC | #2 
On Wed, Jan 24, 2024 at 07:17:27AM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> A small nit seeing Simon's comment which will have you respin.
> cls_tcindex is no longer in the kernel.
> Can you use another example?

I'd better do.

> Also Stephen had some comments last time, not sure if you addressed
> those (nothing on the logs says you did 

It's these lines:

| Changes from v2 (https://lore.kernel.org/r/20231206192752.18989-1-mkoutny@suse.com)
| ...
| - used alias names more fitting the existing net- aliases
| - more info in commit messages and cover letter

> and i didnt see him say anything).

Me neither. I may amend the patches more if I missed anything.

Thanks,
Michal
Michal Koutný Feb. 2, 2024, 11 a.m. UTC | #3 
On Wed, Jan 24, 2024 at 02:19:11PM +0100, Michal Koutný <mkoutny@suse.com> wrote:
> On Wed, Jan 24, 2024 at 07:17:27AM -0500, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>...
> > and i didnt see him say anything).
> 
> Me neither. I may amend the patches more if I missed anything.

FTR, v5 is at [1], changes are summed in cover letter's end.

Thanks,
Michal

[1] https://lore.kernel.org/r/20240201130943.19536-1-mkoutny@suse.com/