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@xxxxxxxxx>
> Cc: Dan Winship <danwinship@xxxxxxxxxx>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> 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
>
