Received-SPF: pass (google.com: domain of linux-kernel+bounces-35790-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 23 Jan 2024 17:40:02 +0000
From: Simon Horman <horms@kernel.org>
To: Michal =?utf-8?Q?Koutn=C3=BD?= <mkoutny@suse.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, cake@lists.bufferbloat.net,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Cong Wang <xiyou.wangcong@gmail.com>, Jiri Pirko <jiri@resnulli.us>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>,
	Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
	Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Petr Pavlu <ppavlu@suse.cz>, Michal Kubecek <mkubecek@suse.cz>,
	Martin Wilck <mwilck@suse.com>,
	Pedro Tammela <pctammela@mojatatu.com>
Subject: Re: [PATCH v4 3/4] net/sched: Load modules via their alias
Message-ID: <20240123174002.GN254773@kernel.org>
References: <20240123135242.11430-1-mkoutny@suse.com>
 <20240123135242.11430-4-mkoutny@suse.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240123135242.11430-4-mkoutny@suse.com>

On Tue, Jan 23, 2024 at 02:52:41PM +0100, Michal Koutný wrote:
> The cls_,sch_,act_ modules may be loaded lazily during network
> configuration but without user's awareness and control.
> 
> Switch the lazy loading from canonical module names to a module alias.
> This allows finer control over lazy loading, the precedent from
> commit 7f78e0351394 ("fs: Limit sys_mount to only request filesystem
> modules.") explains it already:
> 
> 	Using aliases means user space can control the policy of which
> 	filesystem^W net/sched modules are auto-loaded by editing
> 	/etc/modprobe.d/*.conf with blacklist and alias directives.
> 	Allowing simple, safe, well understood work-arounds to known
> 	problematic software.
> 
> By default, nothing changes. However, if a specific module is
> blacklisted (its canonical name), it won't be modprobe'd when requested
> under its alias (i.e. kernel auto-loading). It would appear as if the
> given module was unknown.
> 
> The module can still be loaded under its canonical name, which is an
> explicit (privileged) user action.
> 
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>  net/sched/act_api.c | 2 +-
>  net/sched/cls_api.c | 2 +-
>  net/sched/sch_api.c | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 3e30d7260493..60c0fadfac6d 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1363,7 +1363,7 @@ struct tc_action_ops *tc_action_load_ops(struct nlattr *nla, u32 flags,
>  
>  		if (rtnl_held)
>  			rtnl_unlock();
> -		request_module("act_%s", act_name);
> +		request_module(NET_ACT_ALIAS_PREFIX "%s", name);

Hi Michal,

name doesn't exist in this context, perhaps the line above should be:

		request_module(NET_ACT_ALIAS_PREFIX "%s", act_name);

>  		if (rtnl_held)
>  			rtnl_lock();
>  
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 92a12e3d0fe6..b31b832598e7 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(NET_CLS_ALIAS_PREFIX "%s", name);

Likewise, perhaps the line above should be:

	request_module(NET_CLS_ALIAS_PREFIX "%s", kind);

>  	if (rtnl_held)
>  		rtnl_lock();
>  	ops = __tcf_proto_lookup_ops(kind);

..

-- 
pw-bot: changes-requested