[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-GNBeCX0dg-rxgQ@calendula>
Date: Mon, 24 Mar 2025 17:49:09 +0100
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Michal Koutný <mkoutny@...e.com>
Cc: netfilter-devel@...r.kernel.org, coreteam@...filter.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Jozsef Kadlecsik <kadlec@...filter.org>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, cgroups@...r.kernel.org,
Jan Engelhardt <ej@...i.de>, Florian Westphal <fw@...len.de>
Subject: Re: [PATCH v2] netfilter: Make xt_cgroup independent from net_cls
On Mon, Mar 24, 2025 at 01:56:07PM +0100, Michal Koutný wrote:
> Hello Pablo.
>
> On Sun, Mar 23, 2025 at 10:20:10AM +0100, Pablo Neira Ayuso <pablo@...filter.org> wrote:
> > why classid != 0 is accepted for cgroup_mt_check_v0()?
>
> It is opposite, only classid == 0 is accepted (that should be same for
> all of v0..v2). (OTOH, there should be no change in validation with
> CONFIG_CGROUP_NET_CLASSID.)
Thanks for clarifying this, more questions below.
> > cgroup_mt_check_v0 represents revision 0 of this match, and this match
> > only supports for clsid (groupsv1).
> >
> > History of revisions of cgroupsv2:
> >
> > - cgroup_mt_check_v0 added to match on clsid (initial version of this match)
> > - cgroup_mt_check_v1 is added to support cgroupsv2 matching
> > - cgroup_mt_check_v2 is added to make cgroupsv2 matching more flexible
>
> > I mean, if !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID) then xt_cgroup
> > should fail for cgroup_mt_check_v0.
>
>
> I considered classid == 0 valid (regardless of CONFIG_*) as counterpart
> to implementation of sock_cgroup_classid() that collapses to 0 when
> !CONFIG_CGROUP_NET_CLASSID (thus at least rules with classid=0 remain
> acceptable).
That is, 0 is the default value when !CONFIG_CGROUP_NET_CLASSID.
> > But a more general question: why this check for classid == 0 in
> > cgroup_mt_check_v1 and cgroup_mt_check_v2?
>
> cgroup_mt_check_v1 is for cgroupv2 OR classid matching. Similar with
> cgroup_mt_check_v2.
Yes, and cgroup_mt_check_v0 only supports for classid matching.
> IOW, all three versions accept classid=0 with !CONFIG_CGROUP_NET_CLASSID
> equally because that is the value that sockets reported classid falls
> back to.
If !CONFIG_CGROUP_NET_CLASSID, then no classid matching is possible.
So why allow a rule to match on cgroup with classid == 0?
Maybe simply do this instead?
static bool possible_classid(u32 classid)
{
return IS_ENABLED(CONFIG_CGROUP_NET_CLASSID);
}
Thanks.
Powered by blists - more mailing lists