[nft PATCH] table: Embed creating nft version into userdata

[nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

Upon listing a table which was created by a newer version of nftables,
warn about the potentially incomplete content.

Suggested-by: Florian Westphal <fw@strlen.de>
Cc: Dan Winship <danwinship@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Keep version and build timestamp separate in nftversion.h (separate
  variables) and netlink (separate attributes)
- Drop NFTNL_UDATA_TABLE_NFTVER define, rely upon updated libnftnl
  headers instead (we need a correct value in NFTNL_UDATA_TABLE_MAX as
  well, so no point in introducing hacks)
- Perform attribute existence checks in version_cmp() itself, kernel
  data may contain one or the other or both or none at all
- Minor warning adjustment as requested

Changes since RFC:
- Change NFTNL_UDATA_TABLE_NFTVER content from string (PACKAGE_VERSION)
  to 12Byte binary data consisting of:
  - the version 3-tuple
  - a stable release number specified at configure-time
  - the build time in seconds since epoch (a 64bit value - could scrap
    some bytes, but maybe worth leaving some space)
---
 .gitignore     |  1 +
 Makefile.am    |  3 +++
 configure.ac   | 24 ++++++++++++++++++++++++
 include/rule.h |  1 +
 src/mnl.c      | 21 +++++++++++++++------
 src/netlink.c  | 33 +++++++++++++++++++++++++++++++++
 src/rule.c     |  4 ++++
 7 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/.gitignore b/.gitignore
index a62e31f31c6b5..1e3bc5146b2f1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,7 @@ autom4te.cache
 build-aux/
 libnftables.pc
 libtool
+nftversion.h
 
 # cscope files
 /cscope.*
diff --git a/Makefile.am b/Makefile.am
index b5580b5451fca..ca6af2e393bed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -33,6 +33,7 @@ sbin_PROGRAMS =
 check_PROGRAMS =
 dist_man_MANS =
 CLEANFILES =
+DISTCLEANFILES =
 
 ###############################################################################
 
@@ -106,6 +107,8 @@ noinst_HEADERS = \
 	\
 	$(NULL)
 
+DISTCLEANFILES += nftversion.h
+
 ###############################################################################
 
 AM_CPPFLAGS = \
diff --git a/configure.ac b/configure.ac
index 550913ef04964..55fecbc56660a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -114,6 +114,30 @@ AC_CHECK_DECLS([getprotobyname_r, getprotobynumber_r, getservbyport_r], [], [],
 #include <netdb.h>
 ]])
 
+AC_ARG_WITH([stable-release], [AS_HELP_STRING([--with-stable-release],
+            [Stable release number])],
+            [], [with_stable_release=0])
+AC_CONFIG_COMMANDS([stable_release],
+                   [STABLE_RELEASE=$stable_release],
+                   [stable_release=$with_stable_release])
+AC_CONFIG_COMMANDS([nftversion.h], [
+(
+	echo "static char nftversion[[]] = {"
+	echo "	${VERSION}," | tr '.' ','
+	echo "	${STABLE_RELEASE}"
+	echo "};"
+	echo "static char nftbuildstamp[[]] = {"
+	for ((i = 56; i >= 0; i-= 8)); do
+		echo "	((uint64_t)MAKE_STAMP >> $i) & 0xff,"
+	done
+	echo "};"
+) >nftversion.h
+])
+# Current date should be fetched exactly once per build,
+# so have 'make' call date and pass the value to every 'gcc' call
+AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
+CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
+
 AC_CONFIG_FILES([					\
 		Makefile				\
 		libnftables.pc				\
diff --git a/include/rule.h b/include/rule.h
index 470ae10754ba5..319f9c39f1107 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -170,6 +170,7 @@ struct table {
 	uint32_t		owner;
 	const char		*comment;
 	bool			has_xt_stmts;
+	bool			is_from_future;
 };
 
 extern struct table *table_alloc(void);
diff --git a/src/mnl.c b/src/mnl.c
index 43229f2498e55..ec4d73d12460a 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -10,6 +10,7 @@
 
 #include <nft.h>
 #include <iface.h>
+#include <nftversion.h>
 
 #include <libmnl/libmnl.h>
 #include <libnftnl/common.h>
@@ -1074,24 +1075,32 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
 	if (nlt == NULL)
 		memory_allocation_error();
 
+	udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+	if (!udbuf)
+		memory_allocation_error();
+
 	nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, cmd->handle.family);
 	if (cmd->table) {
 		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, cmd->table->flags);
 
 		if (cmd->table->comment) {
-			udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
-			if (!udbuf)
-				memory_allocation_error();
 			if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_TABLE_COMMENT, cmd->table->comment))
 				memory_allocation_error();
-			nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA, nftnl_udata_buf_data(udbuf),
-					     nftnl_udata_buf_len(udbuf));
-			nftnl_udata_buf_free(udbuf);
 		}
 	} else {
 		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, 0);
 	}
 
+	if (!nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTVER,
+			     sizeof(nftversion), nftversion) ||
+	    !nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTBLD,
+	                     sizeof(nftbuildstamp), nftbuildstamp))
+		memory_allocation_error();
+	nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA,
+			     nftnl_udata_buf_data(udbuf),
+			     nftnl_udata_buf_len(udbuf));
+	nftnl_udata_buf_free(udbuf);
+
 	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(ctx->batch),
 				    NFT_MSG_NEWTABLE,
 				    cmd->handle.family,
diff --git a/src/netlink.c b/src/netlink.c
index 94cbcbfc6c094..b5da33e5a1e53 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -10,6 +10,7 @@
  */
 
 #include <nft.h>
+#include <nftversion.h>
 
 #include <errno.h>
 #include <libmnl/libmnl.h>
@@ -799,6 +800,14 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 			if (value[len - 1] != '\0')
 				return -1;
 			break;
+		case NFTNL_UDATA_TABLE_NFTVER:
+			if (len != sizeof(nftversion))
+				return -1;
+			break;
+		case NFTNL_UDATA_TABLE_NFTBLD:
+			if (len != sizeof(nftbuildstamp))
+				return -1;
+			break;
 		default:
 			return 0;
 	}
@@ -806,6 +815,29 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 	return 0;
 }
 
+static int version_cmp(const struct nftnl_udata **ud)
+{
+	const char *udbuf;
+	size_t i;
+
+	/* netlink attribute lengths checked by table_parse_udata_cb() */
+	if (ud[NFTNL_UDATA_TABLE_NFTVER]) {
+		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTVER]);
+		for (i = 0; i < sizeof(nftversion); i++) {
+			if (nftversion[i] != udbuf[i])
+				return nftversion[i] - udbuf[i];
+		}
+	}
+	if (ud[NFTNL_UDATA_TABLE_NFTBLD]) {
+		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTBLD]);
+		for (i = 0; i < sizeof(nftbuildstamp); i++) {
+			if (nftbuildstamp[i] != udbuf[i])
+				return nftbuildstamp[i] - udbuf[i];
+		}
+	}
+	return 0;
+}
+
 struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 					const struct nftnl_table *nlt)
 {
@@ -830,6 +862,7 @@ struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 		}
 		if (ud[NFTNL_UDATA_TABLE_COMMENT])
 			table->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_TABLE_COMMENT]));
+		table->is_from_future = version_cmp(ud) < 0;
 	}
 
 	return table;
diff --git a/src/rule.c b/src/rule.c
index 0ad948ea87f2f..398e5bdd4c64e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1298,6 +1298,10 @@ static void table_print(const struct table *table, struct output_ctx *octx)
 		fprintf(octx->error_fp,
 			"# Warning: table %s %s is managed by iptables-nft, do not touch!\n",
 			family, table->handle.table.name);
+	if (table->is_from_future)
+		fprintf(octx->error_fp,
+			"# Warning: table %s %s was created by a newer version of nftables? Content may be incomplete!\n",
+			family, table->handle.table.name);
 
 	nft_print(octx, "table %s %s {", family, table->handle.table.name);
 	if (nft_output_handle(octx) || table->flags & TABLE_F_OWNER)
-- 
2.49.0

Re: [nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
Subject: Re: [nft PATCH] table: Embed creating nft version into userdata

This is actually 'PATCH v2', sorry for the mess.

Re: [nft PATCH] table: Embed creating nft version into userdata

[Pablo Neira Ayuso]

Hi Phil,

On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
> Upon listing a table which was created by a newer version of nftables,
> warn about the potentially incomplete content.
> 
> Suggested-by: Florian Westphal <fw@strlen.de>
> Cc: Dan Winship <danwinship@redhat.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Just rebase this and apply, it clashes with my update for --unitdir.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks.

Re: [nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

On Thu, Aug 28, 2025 at 12:46:54AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
> > Upon listing a table which was created by a newer version of nftables,
> > warn about the potentially incomplete content.
> > 
> > Suggested-by: Florian Westphal <fw@strlen.de>
> > Cc: Dan Winship <danwinship@redhat.com>
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> Just rebase this and apply, it clashes with my update for --unitdir.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Also applied.

Re: [nft PATCH] table: Embed creating nft version into userdata

[Pablo Neira Ayuso]

Hi Phil,

I know this is applied, but one late question.

On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
> @@ -806,6 +815,29 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
>  	return 0;
>  }
>  
> +static int version_cmp(const struct nftnl_udata **ud)
> +{
> +	const char *udbuf;
> +	size_t i;
> +
> +	/* netlink attribute lengths checked by table_parse_udata_cb() */
> +	if (ud[NFTNL_UDATA_TABLE_NFTVER]) {
> +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTVER]);
> +		for (i = 0; i < sizeof(nftversion); i++) {
> +			if (nftversion[i] != udbuf[i])
> +				return nftversion[i] - udbuf[i];
> +		}
> +	}
> +	if (ud[NFTNL_UDATA_TABLE_NFTBLD]) {
> +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTBLD]);
> +		for (i = 0; i < sizeof(nftbuildstamp); i++) {
> +			if (nftbuildstamp[i] != udbuf[i])
> +				return nftbuildstamp[i] - udbuf[i];
> +		}
> +	}

One situation I was considering:

1.0.6.y (build today) in the host
1.1.5 (build n days ago) in the container

This will display the warning.

I suggested to use build time only when version is the same?

If the scenario is nftables in the host injects tables into container,
then host binary will likely be updated more often.

IIUC, the build time here will actually determine when the warning is
emitted, regardless the version.

Re: [nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

On Thu, Aug 28, 2025 at 02:53:29PM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> I know this is applied, but one late question.
> 
> On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
> > @@ -806,6 +815,29 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> >  	return 0;
> >  }
> >  
> > +static int version_cmp(const struct nftnl_udata **ud)
> > +{
> > +	const char *udbuf;
> > +	size_t i;
> > +
> > +	/* netlink attribute lengths checked by table_parse_udata_cb() */
> > +	if (ud[NFTNL_UDATA_TABLE_NFTVER]) {
> > +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTVER]);
> > +		for (i = 0; i < sizeof(nftversion); i++) {
> > +			if (nftversion[i] != udbuf[i])
> > +				return nftversion[i] - udbuf[i];
> > +		}
> > +	}
> > +	if (ud[NFTNL_UDATA_TABLE_NFTBLD]) {
> > +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTBLD]);
> > +		for (i = 0; i < sizeof(nftbuildstamp); i++) {
> > +			if (nftbuildstamp[i] != udbuf[i])
> > +				return nftbuildstamp[i] - udbuf[i];
> > +		}
> > +	}
> 
> One situation I was considering:
> 
> 1.0.6.y (build today) in the host
> 1.1.5 (build n days ago) in the container
> 
> This will display the warning.
> 
> I suggested to use build time only when version is the same?
> 
> If the scenario is nftables in the host injects tables into container,
> then host binary will likely be updated more often.
> 
> IIUC, the build time here will actually determine when the warning is
> emitted, regardless the version.

It should not:

Here's version_cmp() pseudo-code:

| for attr in NFTNL_UDATA_TABLE_NFTVER, NFTNL_UDATA_TABLE_NFTBLD:
| 	for idx in len(attr):
| 		if local_data[idx] != attr[idx]:
| 			return local_data[idx] - attr[idx];

This algorithm considers following bytes only if all previous ones were
identical. Precedence is from highest order version bytes to lowest
order build bytes (data is therefore stored in Big Endian).

So your version 1.1.5 will always be "newer" than 1.0.6.y, no matter the
build date, due to minor version 1 > 0.

Cheers, Phil

Re: [nft PATCH] table: Embed creating nft version into userdata

[Pablo Neira Ayuso]

On Thu, Aug 28, 2025 at 03:53:14PM +0200, Phil Sutter wrote:
> On Thu, Aug 28, 2025 at 02:53:29PM +0200, Pablo Neira Ayuso wrote:
> > Hi Phil,
> > 
> > I know this is applied, but one late question.
> > 
> > On Wed, Aug 13, 2025 at 07:07:19PM +0200, Phil Sutter wrote:
> > > @@ -806,6 +815,29 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> > >  	return 0;
> > >  }
> > >  
> > > +static int version_cmp(const struct nftnl_udata **ud)
> > > +{
> > > +	const char *udbuf;
> > > +	size_t i;
> > > +
> > > +	/* netlink attribute lengths checked by table_parse_udata_cb() */
> > > +	if (ud[NFTNL_UDATA_TABLE_NFTVER]) {
> > > +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTVER]);
> > > +		for (i = 0; i < sizeof(nftversion); i++) {
> > > +			if (nftversion[i] != udbuf[i])
> > > +				return nftversion[i] - udbuf[i];
> > > +		}
> > > +	}
> > > +	if (ud[NFTNL_UDATA_TABLE_NFTBLD]) {
> > > +		udbuf = nftnl_udata_get(ud[NFTNL_UDATA_TABLE_NFTBLD]);
> > > +		for (i = 0; i < sizeof(nftbuildstamp); i++) {
> > > +			if (nftbuildstamp[i] != udbuf[i])
> > > +				return nftbuildstamp[i] - udbuf[i];
> > > +		}
> > > +	}
> > 
> > One situation I was considering:
> > 
> > 1.0.6.y (build today) in the host
> > 1.1.5 (build n days ago) in the container
> > 
> > This will display the warning.
> > 
> > I suggested to use build time only when version is the same?
> > 
> > If the scenario is nftables in the host injects tables into container,
> > then host binary will likely be updated more often.
> > 
> > IIUC, the build time here will actually determine when the warning is
> > emitted, regardless the version.
> 
> It should not:
> 
> Here's version_cmp() pseudo-code:
> 
> | for attr in NFTNL_UDATA_TABLE_NFTVER, NFTNL_UDATA_TABLE_NFTBLD:
> | 	for idx in len(attr):
> | 		if local_data[idx] != attr[idx]:
> | 			return local_data[idx] - attr[idx];
> 
> This algorithm considers following bytes only if all previous ones were
> identical. Precedence is from highest order version bytes to lowest
> order build bytes (data is therefore stored in Big Endian).
> 
> So your version 1.1.5 will always be "newer" than 1.0.6.y, no matter the
> build date, due to minor version 1 > 0.

Ah, I misread this smart function, thanks for clarifying.

[nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

Upon listing a table which was created by a newer version of nftables,
warn about the potentially incomplete content.

Suggested-by: Florian Westphal <fw@strlen.de>
Cc: Dan Winship <danwinship@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since RFC:
- Change NFTNL_UDATA_TABLE_NFTVER content from string (PACKAGE_VERSION)
  to 12Byte binary data consisting of:
  - the version 3-tuple
  - a stable release number specified at configure-time
  - the build time in seconds since epoch (a 64bit value - could scrap
    some bytes, but maybe worth leaving some space)
---
 .gitignore     |  1 +
 Makefile.am    |  3 +++
 configure.ac   | 22 ++++++++++++++++++++++
 include/nft.h  |  2 ++
 include/rule.h |  1 +
 src/mnl.c      | 19 +++++++++++++------
 src/netlink.c  | 23 ++++++++++++++++++++++-
 src/rule.c     |  4 ++++
 8 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index a62e31f31c6b5..1e3bc5146b2f1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,7 @@ autom4te.cache
 build-aux/
 libnftables.pc
 libtool
+nftversion.h
 
 # cscope files
 /cscope.*
diff --git a/Makefile.am b/Makefile.am
index b5580b5451fca..ca6af2e393bed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -33,6 +33,7 @@ sbin_PROGRAMS =
 check_PROGRAMS =
 dist_man_MANS =
 CLEANFILES =
+DISTCLEANFILES =
 
 ###############################################################################
 
@@ -106,6 +107,8 @@ noinst_HEADERS = \
 	\
 	$(NULL)
 
+DISTCLEANFILES += nftversion.h
+
 ###############################################################################
 
 AM_CPPFLAGS = \
diff --git a/configure.ac b/configure.ac
index 550913ef04964..2c68c2b8e0560 100644
--- a/configure.ac
+++ b/configure.ac
@@ -114,6 +114,28 @@ AC_CHECK_DECLS([getprotobyname_r, getprotobynumber_r, getservbyport_r], [], [],
 #include <netdb.h>
 ]])
 
+AC_ARG_WITH([stable-release], [AS_HELP_STRING([--with-stable-release],
+            [Stable release number])],
+            [], [with_stable_release=0])
+AC_CONFIG_COMMANDS([stable_release],
+                   [STABLE_RELEASE=$stable_release],
+                   [stable_release=$with_stable_release])
+AC_CONFIG_COMMANDS([nftversion.h], [
+(
+	echo "static char nftversion[[]] = {"
+	echo "	${VERSION}," | tr '.' ','
+	echo "	${STABLE_RELEASE},"
+	for ((i = 56; i >= 0; i-= 8)); do
+		echo "	((uint64_t)MAKE_STAMP >> $i) & 0xff,"
+	done
+	echo "};"
+) >nftversion.h
+])
+# Current date should be fetched exactly once per build,
+# so have 'make' call date and pass the value to every 'gcc' call
+AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
+CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
+
 AC_CONFIG_FILES([					\
 		Makefile				\
 		libnftables.pc				\
diff --git a/include/nft.h b/include/nft.h
index a2d62dbf4808a..b406a68ffeb18 100644
--- a/include/nft.h
+++ b/include/nft.h
@@ -15,4 +15,6 @@
  * something we frequently need to do and it's intentional. */
 #define free_const(ptr) free((void *)(ptr))
 
+#define NFTNL_UDATA_TABLE_NFTVER 1
+
 #endif /* NFTABLES_NFT_H */
diff --git a/include/rule.h b/include/rule.h
index 470ae10754ba5..319f9c39f1107 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -170,6 +170,7 @@ struct table {
 	uint32_t		owner;
 	const char		*comment;
 	bool			has_xt_stmts;
+	bool			is_from_future;
 };
 
 extern struct table *table_alloc(void);
diff --git a/src/mnl.c b/src/mnl.c
index 43229f2498e55..67ec60a6fee00 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -10,6 +10,7 @@
 
 #include <nft.h>
 #include <iface.h>
+#include <nftversion.h>
 
 #include <libmnl/libmnl.h>
 #include <libnftnl/common.h>
@@ -1074,24 +1075,30 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
 	if (nlt == NULL)
 		memory_allocation_error();
 
+	udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+	if (!udbuf)
+		memory_allocation_error();
+
 	nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, cmd->handle.family);
 	if (cmd->table) {
 		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, cmd->table->flags);
 
 		if (cmd->table->comment) {
-			udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
-			if (!udbuf)
-				memory_allocation_error();
 			if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_TABLE_COMMENT, cmd->table->comment))
 				memory_allocation_error();
-			nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA, nftnl_udata_buf_data(udbuf),
-					     nftnl_udata_buf_len(udbuf));
-			nftnl_udata_buf_free(udbuf);
 		}
 	} else {
 		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, 0);
 	}
 
+	if (!nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTVER,
+			     sizeof(nftversion), nftversion))
+		memory_allocation_error();
+	nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA,
+			     nftnl_udata_buf_data(udbuf),
+			     nftnl_udata_buf_len(udbuf));
+	nftnl_udata_buf_free(udbuf);
+
 	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(ctx->batch),
 				    NFT_MSG_NEWTABLE,
 				    cmd->handle.family,
diff --git a/src/netlink.c b/src/netlink.c
index 94cbcbfc6c094..97a49c08b1e82 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -10,6 +10,7 @@
  */
 
 #include <nft.h>
+#include <nftversion.h>
 
 #include <errno.h>
 #include <libmnl/libmnl.h>
@@ -799,6 +800,10 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 			if (value[len - 1] != '\0')
 				return -1;
 			break;
+		case NFTNL_UDATA_TABLE_NFTVER:
+			if (len != sizeof(nftversion))
+				return -1;
+			break;
 		default:
 			return 0;
 	}
@@ -806,10 +811,23 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
 	return 0;
 }
 
+static int version_cmp(const struct nftnl_udata *ud)
+{
+	const char *udbuf = nftnl_udata_get(ud);
+	size_t i;
+
+	/* udbuf length checked by table_parse_udata_cb() */
+	for (i = 0; i < sizeof(nftversion); i++) {
+		if (nftversion[i] != udbuf[i])
+			return nftversion[i] - udbuf[i];
+	}
+	return 0;
+}
+
 struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 					const struct nftnl_table *nlt)
 {
-	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 1] = {};
+	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 2] = {};
 	struct table *table;
 	const char *udata;
 	uint32_t ulen;
@@ -830,6 +848,9 @@ struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 		}
 		if (ud[NFTNL_UDATA_TABLE_COMMENT])
 			table->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_TABLE_COMMENT]));
+		if (ud[NFTNL_UDATA_TABLE_NFTVER] &&
+		    version_cmp(ud[NFTNL_UDATA_TABLE_NFTVER]) < 0)
+			table->is_from_future = true;
 	}
 
 	return table;
diff --git a/src/rule.c b/src/rule.c
index 0ad948ea87f2f..fd69c622cfe75 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1298,6 +1298,10 @@ static void table_print(const struct table *table, struct output_ctx *octx)
 		fprintf(octx->error_fp,
 			"# Warning: table %s %s is managed by iptables-nft, do not touch!\n",
 			family, table->handle.table.name);
+	if (table->is_from_future)
+		fprintf(octx->error_fp,
+			"# Warning: table %s %s was created by a newer version of nftables, content may be incomplete!\n",
+			family, table->handle.table.name);
 
 	nft_print(octx, "table %s %s {", family, table->handle.table.name);
 	if (nft_output_handle(octx) || table->flags & TABLE_F_OWNER)
-- 
2.49.0

Re: [nft PATCH] table: Embed creating nft version into userdata

[Pablo Neira Ayuso]

Hi Phil,

Thanks for your patch, comments below.

On Fri, Aug 08, 2025 at 02:46:18PM +0200, Phil Sutter wrote:
> Upon listing a table which was created by a newer version of nftables,
> warn about the potentially incomplete content.
> 
> Suggested-by: Florian Westphal <fw@strlen.de>
> Cc: Dan Winship <danwinship@redhat.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since RFC:
> - Change NFTNL_UDATA_TABLE_NFTVER content from string (PACKAGE_VERSION)
>   to 12Byte binary data consisting of:
>   - the version 3-tuple
>   - a stable release number specified at configure-time
>   - the build time in seconds since epoch (a 64bit value - could scrap
>     some bytes, but maybe worth leaving some space)
> ---
>  .gitignore     |  1 +
>  Makefile.am    |  3 +++
>  configure.ac   | 22 ++++++++++++++++++++++
>  include/nft.h  |  2 ++
>  include/rule.h |  1 +
>  src/mnl.c      | 19 +++++++++++++------
>  src/netlink.c  | 23 ++++++++++++++++++++++-
>  src/rule.c     |  4 ++++
>  8 files changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index a62e31f31c6b5..1e3bc5146b2f1 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -14,6 +14,7 @@ autom4te.cache
>  build-aux/
>  libnftables.pc
>  libtool
> +nftversion.h
>  
>  # cscope files
>  /cscope.*
> diff --git a/Makefile.am b/Makefile.am
> index b5580b5451fca..ca6af2e393bed 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -33,6 +33,7 @@ sbin_PROGRAMS =
>  check_PROGRAMS =
>  dist_man_MANS =
>  CLEANFILES =
> +DISTCLEANFILES =
>  
>  ###############################################################################
>  
> @@ -106,6 +107,8 @@ noinst_HEADERS = \
>  	\
>  	$(NULL)
>  
> +DISTCLEANFILES += nftversion.h
> +
>  ###############################################################################
>  
>  AM_CPPFLAGS = \
> diff --git a/configure.ac b/configure.ac
> index 550913ef04964..2c68c2b8e0560 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -114,6 +114,28 @@ AC_CHECK_DECLS([getprotobyname_r, getprotobynumber_r, getservbyport_r], [], [],
>  #include <netdb.h>
>  ]])
>  
> +AC_ARG_WITH([stable-release], [AS_HELP_STRING([--with-stable-release],
> +            [Stable release number])],
> +            [], [with_stable_release=0])
> +AC_CONFIG_COMMANDS([stable_release],
> +                   [STABLE_RELEASE=$stable_release],
> +                   [stable_release=$with_stable_release])
> +AC_CONFIG_COMMANDS([nftversion.h], [
> +(
> +	echo "static char nftversion[[]] = {"
> +	echo "	${VERSION}," | tr '.' ','
> +	echo "	${STABLE_RELEASE},"
> +	for ((i = 56; i >= 0; i-= 8)); do
> +		echo "	((uint64_t)MAKE_STAMP >> $i) & 0xff,"
> +	done
> +	echo "};"
> +) >nftversion.h
> +])
> +# Current date should be fetched exactly once per build,
> +# so have 'make' call date and pass the value to every 'gcc' call
> +AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
> +CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
> +
>  AC_CONFIG_FILES([					\
>  		Makefile				\
>  		libnftables.pc				\
> diff --git a/include/nft.h b/include/nft.h
> index a2d62dbf4808a..b406a68ffeb18 100644
> --- a/include/nft.h
> +++ b/include/nft.h
> @@ -15,4 +15,6 @@
>   * something we frequently need to do and it's intentional. */
>  #define free_const(ptr) free((void *)(ptr))
>  
> +#define NFTNL_UDATA_TABLE_NFTVER 1

Better define this in libnftnl?

libnftnl$ git grep NFTNL_UDATA_TABLE_COMMENT
include/libnftnl/udata.h:       NFTNL_UDATA_TABLE_COMMENT,

>  #endif /* NFTABLES_NFT_H */
> diff --git a/include/rule.h b/include/rule.h
> index 470ae10754ba5..319f9c39f1107 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -170,6 +170,7 @@ struct table {
>  	uint32_t		owner;
>  	const char		*comment;
>  	bool			has_xt_stmts;
> +	bool			is_from_future;
>  };
>  
>  extern struct table *table_alloc(void);
> diff --git a/src/mnl.c b/src/mnl.c
> index 43229f2498e55..67ec60a6fee00 100644
> --- a/src/mnl.c
> +++ b/src/mnl.c
> @@ -10,6 +10,7 @@
>  
>  #include <nft.h>
>  #include <iface.h>
> +#include <nftversion.h>
>  
>  #include <libmnl/libmnl.h>
>  #include <libnftnl/common.h>
> @@ -1074,24 +1075,30 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
>  	if (nlt == NULL)
>  		memory_allocation_error();
>  
> +	udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> +	if (!udbuf)
> +		memory_allocation_error();
> +
>  	nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, cmd->handle.family);
>  	if (cmd->table) {
>  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, cmd->table->flags);
>  
>  		if (cmd->table->comment) {
> -			udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> -			if (!udbuf)
> -				memory_allocation_error();
>  			if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_TABLE_COMMENT, cmd->table->comment))
>  				memory_allocation_error();
> -			nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA, nftnl_udata_buf_data(udbuf),
> -					     nftnl_udata_buf_len(udbuf));

I suggest two separated TLVs, one for version and another for build time.

> -			nftnl_udata_buf_free(udbuf);
>  		}
>  	} else {
>  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, 0);
>  	}
>  
> +	if (!nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTVER,
> +			     sizeof(nftversion), nftversion))
> +		memory_allocation_error();
> +	nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA,
> +			     nftnl_udata_buf_data(udbuf),
> +			     nftnl_udata_buf_len(udbuf));
> +	nftnl_udata_buf_free(udbuf);
> +
>  	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(ctx->batch),
>  				    NFT_MSG_NEWTABLE,
>  				    cmd->handle.family,
> diff --git a/src/netlink.c b/src/netlink.c
> index 94cbcbfc6c094..97a49c08b1e82 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <nft.h>
> +#include <nftversion.h>
>  
>  #include <errno.h>
>  #include <libmnl/libmnl.h>
> @@ -799,6 +800,10 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
>  			if (value[len - 1] != '\0')
>  				return -1;
>  			break;
> +		case NFTNL_UDATA_TABLE_NFTVER:
> +			if (len != sizeof(nftversion))
> +				return -1;
> +			break;
>  		default:
>  			return 0;
>  	}
> @@ -806,10 +811,23 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
>  	return 0;
>  }
>  
> +static int version_cmp(const struct nftnl_udata *ud)
> +{
> +	const char *udbuf = nftnl_udata_get(ud);
> +	size_t i;
> +
> +	/* udbuf length checked by table_parse_udata_cb() */
> +	for (i = 0; i < sizeof(nftversion); i++) {
> +		if (nftversion[i] != udbuf[i])
> +			return nftversion[i] - udbuf[i];
> +	}

Interesting trick.

> +	return 0;
> +}
> +
>  struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
>  					const struct nftnl_table *nlt)
>  {
> -	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 1] = {};
> +	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 2] = {};
>  	struct table *table;
>  	const char *udata;
>  	uint32_t ulen;
> @@ -830,6 +848,9 @@ struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
>  		}
>  		if (ud[NFTNL_UDATA_TABLE_COMMENT])
>  			table->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_TABLE_COMMENT]));
> +		if (ud[NFTNL_UDATA_TABLE_NFTVER] &&
> +		    version_cmp(ud[NFTNL_UDATA_TABLE_NFTVER]) < 0)
> +			table->is_from_future = true;
>  	}
>  
>  	return table;
> diff --git a/src/rule.c b/src/rule.c
> index 0ad948ea87f2f..fd69c622cfe75 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1298,6 +1298,10 @@ static void table_print(const struct table *table, struct output_ctx *octx)
>  		fprintf(octx->error_fp,
>  			"# Warning: table %s %s is managed by iptables-nft, do not touch!\n",
>  			family, table->handle.table.name);
> +	if (table->is_from_future)
> +		fprintf(octx->error_fp,
> +			"# Warning: table %s %s was created by a newer version of nftables, content may be incomplete!\n",

+			"# Warning: this table %s %s was created by a newer version of nftables? Content may be incomplete!\n",

Maybe rise it as a question? This is just an indication, I was
considering you could write a program to push anything in the userdata
area. But not a deal breaker if you prefer this sentence.

> +			family, table->handle.table.name);
>  
>  	nft_print(octx, "table %s %s {", family, table->handle.table.name);
>  	if (nft_output_handle(octx) || table->flags & TABLE_F_OWNER)
> -- 
> 2.49.0
> 

Re: [nft PATCH] table: Embed creating nft version into userdata

[Phil Sutter]

Hi Pablo,

On Mon, Aug 11, 2025 at 08:58:48PM +0200, Pablo Neira Ayuso wrote:
> Thanks for your patch, comments below.

Thanks for your review! (Replies below.)

> On Fri, Aug 08, 2025 at 02:46:18PM +0200, Phil Sutter wrote:
> > Upon listing a table which was created by a newer version of nftables,
> > warn about the potentially incomplete content.
> > 
> > Suggested-by: Florian Westphal <fw@strlen.de>
> > Cc: Dan Winship <danwinship@redhat.com>
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > Changes since RFC:
> > - Change NFTNL_UDATA_TABLE_NFTVER content from string (PACKAGE_VERSION)
> >   to 12Byte binary data consisting of:
> >   - the version 3-tuple
> >   - a stable release number specified at configure-time
> >   - the build time in seconds since epoch (a 64bit value - could scrap
> >     some bytes, but maybe worth leaving some space)
> > ---
> >  .gitignore     |  1 +
> >  Makefile.am    |  3 +++
> >  configure.ac   | 22 ++++++++++++++++++++++
> >  include/nft.h  |  2 ++
> >  include/rule.h |  1 +
> >  src/mnl.c      | 19 +++++++++++++------
> >  src/netlink.c  | 23 ++++++++++++++++++++++-
> >  src/rule.c     |  4 ++++
> >  8 files changed, 68 insertions(+), 7 deletions(-)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index a62e31f31c6b5..1e3bc5146b2f1 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -14,6 +14,7 @@ autom4te.cache
> >  build-aux/
> >  libnftables.pc
> >  libtool
> > +nftversion.h
> >  
> >  # cscope files
> >  /cscope.*
> > diff --git a/Makefile.am b/Makefile.am
> > index b5580b5451fca..ca6af2e393bed 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -33,6 +33,7 @@ sbin_PROGRAMS =
> >  check_PROGRAMS =
> >  dist_man_MANS =
> >  CLEANFILES =
> > +DISTCLEANFILES =
> >  
> >  ###############################################################################
> >  
> > @@ -106,6 +107,8 @@ noinst_HEADERS = \
> >  	\
> >  	$(NULL)
> >  
> > +DISTCLEANFILES += nftversion.h
> > +
> >  ###############################################################################
> >  
> >  AM_CPPFLAGS = \
> > diff --git a/configure.ac b/configure.ac
> > index 550913ef04964..2c68c2b8e0560 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -114,6 +114,28 @@ AC_CHECK_DECLS([getprotobyname_r, getprotobynumber_r, getservbyport_r], [], [],
> >  #include <netdb.h>
> >  ]])
> >  
> > +AC_ARG_WITH([stable-release], [AS_HELP_STRING([--with-stable-release],
> > +            [Stable release number])],
> > +            [], [with_stable_release=0])
> > +AC_CONFIG_COMMANDS([stable_release],
> > +                   [STABLE_RELEASE=$stable_release],
> > +                   [stable_release=$with_stable_release])
> > +AC_CONFIG_COMMANDS([nftversion.h], [
> > +(
> > +	echo "static char nftversion[[]] = {"
> > +	echo "	${VERSION}," | tr '.' ','
> > +	echo "	${STABLE_RELEASE},"
> > +	for ((i = 56; i >= 0; i-= 8)); do
> > +		echo "	((uint64_t)MAKE_STAMP >> $i) & 0xff,"
> > +	done
> > +	echo "};"
> > +) >nftversion.h
> > +])
> > +# Current date should be fetched exactly once per build,
> > +# so have 'make' call date and pass the value to every 'gcc' call
> > +AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
> > +CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
> > +
> >  AC_CONFIG_FILES([					\
> >  		Makefile				\
> >  		libnftables.pc				\
> > diff --git a/include/nft.h b/include/nft.h
> > index a2d62dbf4808a..b406a68ffeb18 100644
> > --- a/include/nft.h
> > +++ b/include/nft.h
> > @@ -15,4 +15,6 @@
> >   * something we frequently need to do and it's intentional. */
> >  #define free_const(ptr) free((void *)(ptr))
> >  
> > +#define NFTNL_UDATA_TABLE_NFTVER 1
> 
> Better define this in libnftnl?

ACK, you found my "quick hack", sorry for that.

> libnftnl$ git grep NFTNL_UDATA_TABLE_COMMENT
> include/libnftnl/udata.h:       NFTNL_UDATA_TABLE_COMMENT,
> 
> >  #endif /* NFTABLES_NFT_H */
> > diff --git a/include/rule.h b/include/rule.h
> > index 470ae10754ba5..319f9c39f1107 100644
> > --- a/include/rule.h
> > +++ b/include/rule.h
> > @@ -170,6 +170,7 @@ struct table {
> >  	uint32_t		owner;
> >  	const char		*comment;
> >  	bool			has_xt_stmts;
> > +	bool			is_from_future;
> >  };
> >  
> >  extern struct table *table_alloc(void);
> > diff --git a/src/mnl.c b/src/mnl.c
> > index 43229f2498e55..67ec60a6fee00 100644
> > --- a/src/mnl.c
> > +++ b/src/mnl.c
> > @@ -10,6 +10,7 @@
> >  
> >  #include <nft.h>
> >  #include <iface.h>
> > +#include <nftversion.h>
> >  
> >  #include <libmnl/libmnl.h>
> >  #include <libnftnl/common.h>
> > @@ -1074,24 +1075,30 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
> >  	if (nlt == NULL)
> >  		memory_allocation_error();
> >  
> > +	udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> > +	if (!udbuf)
> > +		memory_allocation_error();
> > +
> >  	nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, cmd->handle.family);
> >  	if (cmd->table) {
> >  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, cmd->table->flags);
> >  
> >  		if (cmd->table->comment) {
> > -			udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> > -			if (!udbuf)
> > -				memory_allocation_error();
> >  			if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_TABLE_COMMENT, cmd->table->comment))
> >  				memory_allocation_error();
> > -			nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA, nftnl_udata_buf_data(udbuf),
> > -					     nftnl_udata_buf_len(udbuf));
> 
> I suggest two separated TLVs, one for version and another for build time.

This seems wrong to me. Build time is just an extra to the version info.
What benefit do you see with splitting them up?

> > -			nftnl_udata_buf_free(udbuf);
> >  		}
> >  	} else {
> >  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, 0);
> >  	}
> >  
> > +	if (!nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTVER,
> > +			     sizeof(nftversion), nftversion))
> > +		memory_allocation_error();
> > +	nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA,
> > +			     nftnl_udata_buf_data(udbuf),
> > +			     nftnl_udata_buf_len(udbuf));
> > +	nftnl_udata_buf_free(udbuf);
> > +
> >  	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(ctx->batch),
> >  				    NFT_MSG_NEWTABLE,
> >  				    cmd->handle.family,
> > diff --git a/src/netlink.c b/src/netlink.c
> > index 94cbcbfc6c094..97a49c08b1e82 100644
> > --- a/src/netlink.c
> > +++ b/src/netlink.c
> > @@ -10,6 +10,7 @@
> >   */
> >  
> >  #include <nft.h>
> > +#include <nftversion.h>
> >  
> >  #include <errno.h>
> >  #include <libmnl/libmnl.h>
> > @@ -799,6 +800,10 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> >  			if (value[len - 1] != '\0')
> >  				return -1;
> >  			break;
> > +		case NFTNL_UDATA_TABLE_NFTVER:
> > +			if (len != sizeof(nftversion))
> > +				return -1;
> > +			break;
> >  		default:
> >  			return 0;
> >  	}
> > @@ -806,10 +811,23 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> >  	return 0;
> >  }
> >  
> > +static int version_cmp(const struct nftnl_udata *ud)
> > +{
> > +	const char *udbuf = nftnl_udata_get(ud);
> > +	size_t i;
> > +
> > +	/* udbuf length checked by table_parse_udata_cb() */
> > +	for (i = 0; i < sizeof(nftversion); i++) {
> > +		if (nftversion[i] != udbuf[i])
> > +			return nftversion[i] - udbuf[i];
> > +	}
> 
> Interesting trick.

There's a German proverb about a blind hen and grains. But I like that
function, too! :)

> > +	return 0;
> > +}
> > +
> >  struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
> >  					const struct nftnl_table *nlt)
> >  {
> > -	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 1] = {};
> > +	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 2] = {};
> >  	struct table *table;
> >  	const char *udata;
> >  	uint32_t ulen;
> > @@ -830,6 +848,9 @@ struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
> >  		}
> >  		if (ud[NFTNL_UDATA_TABLE_COMMENT])
> >  			table->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_TABLE_COMMENT]));
> > +		if (ud[NFTNL_UDATA_TABLE_NFTVER] &&
> > +		    version_cmp(ud[NFTNL_UDATA_TABLE_NFTVER]) < 0)
> > +			table->is_from_future = true;
> >  	}
> >  
> >  	return table;
> > diff --git a/src/rule.c b/src/rule.c
> > index 0ad948ea87f2f..fd69c622cfe75 100644
> > --- a/src/rule.c
> > +++ b/src/rule.c
> > @@ -1298,6 +1298,10 @@ static void table_print(const struct table *table, struct output_ctx *octx)
> >  		fprintf(octx->error_fp,
> >  			"# Warning: table %s %s is managed by iptables-nft, do not touch!\n",
> >  			family, table->handle.table.name);
> > +	if (table->is_from_future)
> > +		fprintf(octx->error_fp,
> > +			"# Warning: table %s %s was created by a newer version of nftables, content may be incomplete!\n",
> 
> +			"# Warning: this table %s %s was created by a newer version of nftables? Content may be incomplete!\n",
> 
> Maybe rise it as a question? This is just an indication, I was
> considering you could write a program to push anything in the userdata
> area. But not a deal breaker if you prefer this sentence.

Fine with me!

Thanks, Phil

Re: [nft PATCH] table: Embed creating nft version into userdata

[Pablo Neira Ayuso]

On Tue, Aug 12, 2025 at 03:06:30PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Mon, Aug 11, 2025 at 08:58:48PM +0200, Pablo Neira Ayuso wrote:
> > Thanks for your patch, comments below.
> 
> Thanks for your review! (Replies below.)
> 
> > On Fri, Aug 08, 2025 at 02:46:18PM +0200, Phil Sutter wrote:
> > > Upon listing a table which was created by a newer version of nftables,
> > > warn about the potentially incomplete content.
> > > 
> > > Suggested-by: Florian Westphal <fw@strlen.de>
> > > Cc: Dan Winship <danwinship@redhat.com>
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > > Changes since RFC:
> > > - Change NFTNL_UDATA_TABLE_NFTVER content from string (PACKAGE_VERSION)
> > >   to 12Byte binary data consisting of:
> > >   - the version 3-tuple
> > >   - a stable release number specified at configure-time
> > >   - the build time in seconds since epoch (a 64bit value - could scrap
> > >     some bytes, but maybe worth leaving some space)
> > > ---
> > >  .gitignore     |  1 +
> > >  Makefile.am    |  3 +++
> > >  configure.ac   | 22 ++++++++++++++++++++++
> > >  include/nft.h  |  2 ++
> > >  include/rule.h |  1 +
> > >  src/mnl.c      | 19 +++++++++++++------
> > >  src/netlink.c  | 23 ++++++++++++++++++++++-
> > >  src/rule.c     |  4 ++++
> > >  8 files changed, 68 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/.gitignore b/.gitignore
> > > index a62e31f31c6b5..1e3bc5146b2f1 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -14,6 +14,7 @@ autom4te.cache
> > >  build-aux/
> > >  libnftables.pc
> > >  libtool
> > > +nftversion.h
> > >  
> > >  # cscope files
> > >  /cscope.*
> > > diff --git a/Makefile.am b/Makefile.am
> > > index b5580b5451fca..ca6af2e393bed 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -33,6 +33,7 @@ sbin_PROGRAMS =
> > >  check_PROGRAMS =
> > >  dist_man_MANS =
> > >  CLEANFILES =
> > > +DISTCLEANFILES =
> > >  
> > >  ###############################################################################
> > >  
> > > @@ -106,6 +107,8 @@ noinst_HEADERS = \
> > >  	\
> > >  	$(NULL)
> > >  
> > > +DISTCLEANFILES += nftversion.h
> > > +
> > >  ###############################################################################
> > >  
> > >  AM_CPPFLAGS = \
> > > diff --git a/configure.ac b/configure.ac
> > > index 550913ef04964..2c68c2b8e0560 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -114,6 +114,28 @@ AC_CHECK_DECLS([getprotobyname_r, getprotobynumber_r, getservbyport_r], [], [],
> > >  #include <netdb.h>
> > >  ]])
> > >  
> > > +AC_ARG_WITH([stable-release], [AS_HELP_STRING([--with-stable-release],
> > > +            [Stable release number])],
> > > +            [], [with_stable_release=0])
> > > +AC_CONFIG_COMMANDS([stable_release],
> > > +                   [STABLE_RELEASE=$stable_release],
> > > +                   [stable_release=$with_stable_release])
> > > +AC_CONFIG_COMMANDS([nftversion.h], [
> > > +(
> > > +	echo "static char nftversion[[]] = {"
> > > +	echo "	${VERSION}," | tr '.' ','
> > > +	echo "	${STABLE_RELEASE},"
> > > +	for ((i = 56; i >= 0; i-= 8)); do
> > > +		echo "	((uint64_t)MAKE_STAMP >> $i) & 0xff,"
> > > +	done
> > > +	echo "};"
> > > +) >nftversion.h
> > > +])
> > > +# Current date should be fetched exactly once per build,
> > > +# so have 'make' call date and pass the value to every 'gcc' call
> > > +AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
> > > +CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
> > > +
> > >  AC_CONFIG_FILES([					\
> > >  		Makefile				\
> > >  		libnftables.pc				\
> > > diff --git a/include/nft.h b/include/nft.h
> > > index a2d62dbf4808a..b406a68ffeb18 100644
> > > --- a/include/nft.h
> > > +++ b/include/nft.h
> > > @@ -15,4 +15,6 @@
> > >   * something we frequently need to do and it's intentional. */
> > >  #define free_const(ptr) free((void *)(ptr))
> > >  
> > > +#define NFTNL_UDATA_TABLE_NFTVER 1
> > 
> > Better define this in libnftnl?
> 
> ACK, you found my "quick hack", sorry for that.

If you prefer easier backporting, then add an #ifdef here to skip the
libnftnl dependency.

> > libnftnl$ git grep NFTNL_UDATA_TABLE_COMMENT
> > include/libnftnl/udata.h:       NFTNL_UDATA_TABLE_COMMENT,
> > 
> > >  #endif /* NFTABLES_NFT_H */
> > > diff --git a/include/rule.h b/include/rule.h
> > > index 470ae10754ba5..319f9c39f1107 100644
> > > --- a/include/rule.h
> > > +++ b/include/rule.h
> > > @@ -170,6 +170,7 @@ struct table {
> > >  	uint32_t		owner;
> > >  	const char		*comment;
> > >  	bool			has_xt_stmts;
> > > +	bool			is_from_future;
> > >  };
> > >  
> > >  extern struct table *table_alloc(void);
> > > diff --git a/src/mnl.c b/src/mnl.c
> > > index 43229f2498e55..67ec60a6fee00 100644
> > > --- a/src/mnl.c
> > > +++ b/src/mnl.c
> > > @@ -10,6 +10,7 @@
> > >  
> > >  #include <nft.h>
> > >  #include <iface.h>
> > > +#include <nftversion.h>
> > >  
> > >  #include <libmnl/libmnl.h>
> > >  #include <libnftnl/common.h>
> > > @@ -1074,24 +1075,30 @@ int mnl_nft_table_add(struct netlink_ctx *ctx, struct cmd *cmd,
> > >  	if (nlt == NULL)
> > >  		memory_allocation_error();
> > >  
> > > +	udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> > > +	if (!udbuf)
> > > +		memory_allocation_error();
> > > +
> > >  	nftnl_table_set_u32(nlt, NFTNL_TABLE_FAMILY, cmd->handle.family);
> > >  	if (cmd->table) {
> > >  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, cmd->table->flags);
> > >  
> > >  		if (cmd->table->comment) {
> > > -			udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> > > -			if (!udbuf)
> > > -				memory_allocation_error();
> > >  			if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_TABLE_COMMENT, cmd->table->comment))
> > >  				memory_allocation_error();
> > > -			nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA, nftnl_udata_buf_data(udbuf),
> > > -					     nftnl_udata_buf_len(udbuf));
> > 
> > I suggest two separated TLVs, one for version and another for build time.
> 
> This seems wrong to me. Build time is just an extra to the version info.
> What benefit do you see with splitting them up?

It is the same as in Netlink. If someone shows up saying built time
and version is not enough, a new attribute should be sufficient.

I cannot see how that can happen, but I have regretted everytime I
added TLVs that embed multiple fields to save a header.

> > > -			nftnl_udata_buf_free(udbuf);
> > >  		}
> > >  	} else {
> > >  		nftnl_table_set_u32(nlt, NFTNL_TABLE_FLAGS, 0);
> > >  	}
> > >  
> > > +	if (!nftnl_udata_put(udbuf, NFTNL_UDATA_TABLE_NFTVER,
> > > +			     sizeof(nftversion), nftversion))
> > > +		memory_allocation_error();
> > > +	nftnl_table_set_data(nlt, NFTNL_TABLE_USERDATA,
> > > +			     nftnl_udata_buf_data(udbuf),
> > > +			     nftnl_udata_buf_len(udbuf));
> > > +	nftnl_udata_buf_free(udbuf);
> > > +
> > >  	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(ctx->batch),
> > >  				    NFT_MSG_NEWTABLE,
> > >  				    cmd->handle.family,
> > > diff --git a/src/netlink.c b/src/netlink.c
> > > index 94cbcbfc6c094..97a49c08b1e82 100644
> > > --- a/src/netlink.c
> > > +++ b/src/netlink.c
> > > @@ -10,6 +10,7 @@
> > >   */
> > >  
> > >  #include <nft.h>
> > > +#include <nftversion.h>
> > >  
> > >  #include <errno.h>
> > >  #include <libmnl/libmnl.h>
> > > @@ -799,6 +800,10 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> > >  			if (value[len - 1] != '\0')
> > >  				return -1;
> > >  			break;
> > > +		case NFTNL_UDATA_TABLE_NFTVER:
> > > +			if (len != sizeof(nftversion))
> > > +				return -1;
> > > +			break;
> > >  		default:
> > >  			return 0;
> > >  	}
> > > @@ -806,10 +811,23 @@ static int table_parse_udata_cb(const struct nftnl_udata *attr, void *data)
> > >  	return 0;
> > >  }
> > >  
> > > +static int version_cmp(const struct nftnl_udata *ud)
> > > +{
> > > +	const char *udbuf = nftnl_udata_get(ud);
> > > +	size_t i;
> > > +
> > > +	/* udbuf length checked by table_parse_udata_cb() */
> > > +	for (i = 0; i < sizeof(nftversion); i++) {
> > > +		if (nftversion[i] != udbuf[i])
> > > +			return nftversion[i] - udbuf[i];
> > > +	}
> > 
> > Interesting trick.
> 
> There's a German proverb about a blind hen and grains. But I like that
> function, too! :)

We don't have such proverb in Spanish :)

> > > +	return 0;
> > > +}
> > > +
> > >  struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
> > >  					const struct nftnl_table *nlt)
> > >  {
> > > -	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 1] = {};
> > > +	const struct nftnl_udata *ud[NFTNL_UDATA_TABLE_MAX + 2] = {};
> > >  	struct table *table;
> > >  	const char *udata;
> > >  	uint32_t ulen;
> > > @@ -830,6 +848,9 @@ struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
> > >  		}
> > >  		if (ud[NFTNL_UDATA_TABLE_COMMENT])
> > >  			table->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_TABLE_COMMENT]));
> > > +		if (ud[NFTNL_UDATA_TABLE_NFTVER] &&
> > > +		    version_cmp(ud[NFTNL_UDATA_TABLE_NFTVER]) < 0)
> > > +			table->is_from_future = true;
> > >  	}
> > >  
> > >  	return table;
> > > diff --git a/src/rule.c b/src/rule.c
> > > index 0ad948ea87f2f..fd69c622cfe75 100644
> > > --- a/src/rule.c
> > > +++ b/src/rule.c
> > > @@ -1298,6 +1298,10 @@ static void table_print(const struct table *table, struct output_ctx *octx)
> > >  		fprintf(octx->error_fp,
> > >  			"# Warning: table %s %s is managed by iptables-nft, do not touch!\n",
> > >  			family, table->handle.table.name);
> > > +	if (table->is_from_future)
> > > +		fprintf(octx->error_fp,
> > > +			"# Warning: table %s %s was created by a newer version of nftables, content may be incomplete!\n",
> > 
> > +			"# Warning: this table %s %s was created by a newer version of nftables? Content may be incomplete!\n",
> > 
> > Maybe rise it as a question? This is just an indication, I was
> > considering you could write a program to push anything in the userdata
> > area. But not a deal breaker if you prefer this sentence.
> 
> Fine with me!

Thanks.