Re: [PATCH nf-next] netfilter: conntrack: make filtering by zone discoverable

[Ilya Maximets]

On 9/4/26 7:44 PM, Ilya Maximets wrote:
> Conntrack flush supports filtering by zone using CTA_ZONE, but this
> attribute is really hard to use from user space applications.  The
> reason is that it is not possible to tell if it's supported or not.
> 
> Older kernels silently ignore CTA_ZONE.  And in that case they just
> happily flush all the entries from all zones breaking all the existing
> connections.  So, applications have to infer support from the kernel
> version.  While it works in most cases, it's not a particularly
> reliable or desired way to check kernel capabilities from applications
> that aim to be portable.  There should be a better way to probe or
> discover features in the kernel.
> 
> The CTA_FILTER interface on the other hand is simple enough to probe.
> We can check for NLM_F_DUMP_FILTERED in the dump to see if filtering
> is supported.  And unknown sub-attributes in CTA_FILTER are rejected
> explicitly since strict validation is in use there.
> 
> Let's add new CTA_FILTER_ZONE that signals that CTA_ZONE should be
> filtered on.  It is a flag, since everything in the CTA_FILTER is a
> bit mask, i.e., a form of a flag.  If set, it means that CTA_ZONE must
> be present and be used for filtering.  If the flag is not set however,
> the filtering on CTA_ZONE will still take place to ensure backwards
> compatibility.  So, the flag doesn't really change the filtering
> behavior, but it allows user space applications to properly discover
> support for CTA_ZONE filtering without need to rely on kernel version
> parsing or risk accidental flushes of the entire conntrack table,
> and also without modifying the kernel state.
> 
> A new test variant is added to test with and without the new flag.
> Since the setup code is moved into a shared function, expectations
> replaced with assertions to bail early if the base setup fails to
> avoid the cascade of secondary failures that can be misleading.
> Error return is only for the SKIP cases.
> 
> Signed-off-by: Ilya Maximets <i.maximets@xxxxxxx>
> ---
> 
> One other alternative is to make CTA_FILTER_ZONE a U16 sub-attribute
> instead of a FLAG and prioritize it over the CTA_ZONE.  But it feels
> like a FLAG is better suited for the CTA_FILTER, even if it takes a
> bit of extra space in the request.  Though I'm OK with reworking this
> into U16 or some other mechanism that would make the feature
> discoverable, if there are better ideas.
> 
>  Documentation/netlink/specs/conntrack.yaml    |   5 +
>  .../linux/netfilter/nfnetlink_conntrack.h     |   1 +
>  net/netfilter/nf_conntrack_netlink.c          |  11 ++
>  .../net/netfilter/conntrack_dump_flush.c      | 143 ++++++++++++------
>  4 files changed, 115 insertions(+), 45 deletions(-)

[...]

> -FIXTURE_SETUP(conntrack_dump_flush)
> +static int conntrack_zone_setup(struct __test_metadata *_metadata,
> +				struct mnl_socket **sock)
>  {
>  	struct in6_addr src, dst;
>  	int ret;
>  
> -	self->sock = mnl_socket_open(NETLINK_NETFILTER);
> -	if (!self->sock) {
> +	*sock = mnl_socket_open(NETLINK_NETFILTER);
> +	if (!*sock) {
>  		perror("mnl_socket_open");
> -		SKIP(return, "cannot open netlink_netfilter socket");
> +		SKIP(return -1, "cannot open netlink_netfilter socket");
>  	}
>  
> -	ret = mnl_socket_bind(self->sock, 0, MNL_SOCKET_AUTOPID);
> +	ret = mnl_socket_bind(*sock, 0, MNL_SOCKET_AUTOPID);
>  	EXPECT_EQ(ret, 0);

Sashiko-gemini (sashiko-nipa didn't run, probably because the patch was
moved to changes-requested due to the CI issue) pointed out that I missed
converting this expectation into an assert.  I can do that in v2 once
the CI is working again.

Best regards, Ilya Maximets.