[PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[Michal Koutný]

Changes from v2 (https://lore.kernel.org/r/20250305170935.80558-1-mkoutny@suse.com):
- don't accept zero classid neither (Pablo N. A.)
- eliminate code that might rely on comparison against zero with
  !CONFIG_CGROUP_NET_CLASSID

Michal Koutný (3):
  netfilter: Make xt_cgroup independent from net_cls
  cgroup: Guard users of sock_cgroup_classid()
  cgroup: Drop sock_cgroup_classid() dummy implementation

 include/linux/cgroup-defs.h | 10 ++++------
 net/ipv4/inet_diag.c        |  2 +-
 net/netfilter/Kconfig       |  2 +-
 net/netfilter/xt_cgroup.c   | 26 ++++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 8 deletions(-)


base-commit: dd83757f6e686a2188997cb58b5975f744bb7786
-- 
2.48.1

[PATCH v3 1/3] netfilter: Make xt_cgroup independent from net_cls

[Michal Koutný]

The xt_group matching supports the default hierarchy since commit
c38c4597e4bf3 ("netfilter: implement xt_cgroup cgroup2 path match").
The cgroup v1 matching (based on clsid) and cgroup v2 matching (based on
path) are rather independent. Downgrade the Kconfig dependency to
mere CONFIG_SOCK_GROUP_DATA so that xt_group can be built even without
CONFIG_NET_CLS_CGROUP for path matching.
Also add a message for users when they attempt to specify any clsid.

Link: https://lists.opensuse.org/archives/list/kernel@lists.opensuse.org/thread/S23NOILB7MUIRHSKPBOQKJHVSK26GP6X/
Cc: Jan Engelhardt <ej@inai.de>
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 net/netfilter/Kconfig     |  2 +-
 net/netfilter/xt_cgroup.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index df2dc21304efb..346ac2152fa18 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -1180,7 +1180,7 @@ config NETFILTER_XT_MATCH_CGROUP
 	tristate '"control group" match support'
 	depends on NETFILTER_ADVANCED
 	depends on CGROUPS
-	select CGROUP_NET_CLASSID
+	select SOCK_CGROUP_DATA
 	help
 	Socket/process control group matching allows you to match locally
 	generated packets based on which net_cls control group processes
diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index c0f5e9a4f3c65..66915bf0d89ad 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -23,6 +23,8 @@ MODULE_DESCRIPTION("Xtables: process control group matching");
 MODULE_ALIAS("ipt_cgroup");
 MODULE_ALIAS("ip6t_cgroup");
 
+#define NET_CLS_CLASSID_INVALID_MSG "xt_cgroup: classid invalid without net_cls cgroups\n"
+
 static int cgroup_mt_check_v0(const struct xt_mtchk_param *par)
 {
 	struct xt_cgroup_info_v0 *info = par->matchinfo;
@@ -30,6 +32,11 @@ static int cgroup_mt_check_v0(const struct xt_mtchk_param *par)
 	if (info->invert & ~1)
 		return -EINVAL;
 
+	if (!IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
+		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -51,6 +58,11 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 		return -EINVAL;
 	}
 
+	if (info->has_classid && !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
+		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		return -EINVAL;
+	}
+
 	info->priv = NULL;
 	if (info->has_path) {
 		cgrp = cgroup_get_from_path(info->path);
@@ -83,6 +95,11 @@ static int cgroup_mt_check_v2(const struct xt_mtchk_param *par)
 		return -EINVAL;
 	}
 
+	if (info->has_classid && !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
+		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		return -EINVAL;
+	}
+
 	info->priv = NULL;
 	if (info->has_path) {
 		cgrp = cgroup_get_from_path(info->path);
-- 
2.48.1

[PATCH v3 2/3] cgroup: Guard users of sock_cgroup_classid()

[Michal Koutný]

Exclude code that relies on sock_cgroup_classid() as preparation of
removal of the function.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 net/ipv4/inet_diag.c      | 2 +-
 net/netfilter/xt_cgroup.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 321acc8abf17e..886dbe65ed9e8 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -160,7 +160,7 @@ int inet_diag_msg_attrs_fill(struct sock *sk, struct sk_buff *skb,
 	    ext & (1 << (INET_DIAG_TCLASS - 1))) {
 		u32 classid = 0;
 
-#ifdef CONFIG_SOCK_CGROUP_DATA
+#ifdef CONFIG_CGROUP_NET_CLASSID
 		classid = sock_cgroup_classid(&sk->sk_cgrp_data);
 #endif
 		/* Fallback to socket priority if class id isn't set.
diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index 66915bf0d89ad..c437fbd59ec13 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -117,6 +117,7 @@ static int cgroup_mt_check_v2(const struct xt_mtchk_param *par)
 static bool
 cgroup_mt_v0(const struct sk_buff *skb, struct xt_action_param *par)
 {
+#ifdef CONFIG_CGROUP_NET_CLASSID
 	const struct xt_cgroup_info_v0 *info = par->matchinfo;
 	struct sock *sk = skb->sk;
 
@@ -125,6 +126,8 @@ cgroup_mt_v0(const struct sk_buff *skb, struct xt_action_param *par)
 
 	return (info->id == sock_cgroup_classid(&skb->sk->sk_cgrp_data)) ^
 		info->invert;
+#endif
+	return false;
 }
 
 static bool cgroup_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
@@ -140,9 +143,12 @@ static bool cgroup_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
 	if (ancestor)
 		return cgroup_is_descendant(sock_cgroup_ptr(skcd), ancestor) ^
 			info->invert_path;
+#ifdef CONFIG_CGROUP_NET_CLASSID
 	else
 		return (info->classid == sock_cgroup_classid(skcd)) ^
 			info->invert_classid;
+#endif
+	return false;
 }
 
 static bool cgroup_mt_v2(const struct sk_buff *skb, struct xt_action_param *par)
@@ -158,9 +164,12 @@ static bool cgroup_mt_v2(const struct sk_buff *skb, struct xt_action_param *par)
 	if (ancestor)
 		return cgroup_is_descendant(sock_cgroup_ptr(skcd), ancestor) ^
 			info->invert_path;
+#ifdef CONFIG_CGROUP_NET_CLASSID
 	else
 		return (info->classid == sock_cgroup_classid(skcd)) ^
 			info->invert_classid;
+#endif
+	return false;
 }
 
 static void cgroup_mt_destroy_v1(const struct xt_mtdtor_param *par)
-- 
2.48.1

[PATCH v3 3/3] cgroup: Drop sock_cgroup_classid() dummy implementation

[Michal Koutný]

The semantic of returning 0 is unclear when !CONFIG_CGROUP_NET_CLASSID.
Since there are no callers of sock_cgroup_classid() with that config
anymore we can undefine the helper at all and enforce all (future)
callers to handle cases when !CONFIG_CGROUP_NET_CLASSID.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 include/linux/cgroup-defs.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 17960a1e858db..28f33b0807c9a 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -866,14 +866,12 @@ static inline u16 sock_cgroup_prioidx(const struct sock_cgroup_data *skcd)
 #endif
 }
 
+#ifdef CONFIG_CGROUP_NET_CLASSID
 static inline u32 sock_cgroup_classid(const struct sock_cgroup_data *skcd)
 {
-#ifdef CONFIG_CGROUP_NET_CLASSID
 	return READ_ONCE(skcd->classid);
-#else
-	return 0;
-#endif
 }
+#endif
 
 static inline void sock_cgroup_set_prioidx(struct sock_cgroup_data *skcd,
 					   u16 prioidx)
@@ -883,13 +881,13 @@ static inline void sock_cgroup_set_prioidx(struct sock_cgroup_data *skcd,
 #endif
 }
 
+#ifdef CONFIG_CGROUP_NET_CLASSID
 static inline void sock_cgroup_set_classid(struct sock_cgroup_data *skcd,
 					   u32 classid)
 {
-#ifdef CONFIG_CGROUP_NET_CLASSID
 	WRITE_ONCE(skcd->classid, classid);
-#endif
 }
+#endif
 
 #else	/* CONFIG_SOCK_CGROUP_DATA */
 
-- 
2.48.1

Re: [PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[Tejun Heo]

On Tue, Apr 01, 2025 at 01:57:29PM +0200, Michal Koutný wrote:
> Changes from v2 (https://lore.kernel.org/r/20250305170935.80558-1-mkoutny@suse.com):
> - don't accept zero classid neither (Pablo N. A.)
> - eliminate code that might rely on comparison against zero with
>   !CONFIG_CGROUP_NET_CLASSID
> 
> Michal Koutný (3):
>   netfilter: Make xt_cgroup independent from net_cls
>   cgroup: Guard users of sock_cgroup_classid()
>   cgroup: Drop sock_cgroup_classid() dummy implementation

From cgroup POV:

  Acked-by: Tejun Heo <tj@kernel.org>

Once folks are happy, please let me know how the patches should be routed.

Thanks.

-- 
tejun

Re: [PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On Tue, Apr 01, 2025 at 01:57:29PM +0200, Michal Koutný <mkoutny@suse.com> wrote:
> Changes from v2 (https://lore.kernel.org/r/20250305170935.80558-1-mkoutny@suse.com):
> - don't accept zero classid neither (Pablo N. A.)
> - eliminate code that might rely on comparison against zero with
>   !CONFIG_CGROUP_NET_CLASSID

Pablo, just to break possible dilemma with Tejun's routing [1], it makes
sense to me to route this series together via net(filter) git(s).

Also, let me (anyone) know should there be further remarks to this form.

Thanks,
Michal

[1] https://lore.kernel.org/all/Z-zqvmJFI3PkNl6R@slm.duckdns.org/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[Pablo Neira Ayuso]

On Wed, Apr 09, 2025 at 06:56:17PM +0200, Michal Koutný wrote:
> On Tue, Apr 01, 2025 at 01:57:29PM +0200, Michal Koutný <mkoutny@suse.com> wrote:
> > Changes from v2 (https://lore.kernel.org/r/20250305170935.80558-1-mkoutny@suse.com):
> > - don't accept zero classid neither (Pablo N. A.)
> > - eliminate code that might rely on comparison against zero with
> >   !CONFIG_CGROUP_NET_CLASSID
> 
> Pablo, just to break possible dilemma with Tejun's routing [1], it makes
> sense to me to route this series together via net(filter) git(s).
> 
> Also, let me (anyone) know should there be further remarks to this form.

I am going to apply 1/3 and 2/3 to nf-next.git

I suggest, then, you follow up to cgroups tree to submit 3/3.

3/3 does not show up in my patchwork for some reason.

Re: [PATCH v3 0/3] netfilter: Make xt_cgroup independent from net_cls

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Tue, Apr 15, 2025 at 05:09:35PM +0200, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I am going to apply 1/3 and 2/3 to nf-next.git

Thanks.

> I suggest, then, you follow up to cgroups tree to submit 3/3.

OK.

> 3/3 does not show up in my patchwork for some reason.

The reason is -- my invocation of get_maintainer.pl on the 3rd patch
excluded anything netdev. Sorry.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]