[PATCH RFC 0/4] Netlink bus descriptions

[PATCH RFC 0/4] Netlink bus descriptions

[Pablo Neira Ayuso]

Hi,

Modern messaging systems usually provide facilities that allows you to
inquire about supported commands and message layouts. Netlink has no
such facility so far, hence people end up probing for features, which is
a bit sloppy. Sometimes there are also magic version numbers in place
that gives a hint to userspace on what this kernel supports, so
userspace makes assumptions based on this version number.

This patchset aims to improve this situation by adding a new
NETLINK_DESC bus with two commands, one to fetch the list of existing
commands and another one to describe supported objects. Command
descriptions also indicate what netlink objects the interface expects to
be used in the netlink message payload, thus, we can relate commands and
objects. Objects are represented as compounds of attributes, some of
these attributes are nesting other attributes. The netlink description
exports the attribute tree and, for simplicity, a linear list of netlink
objects identified via unique ID.

Patch 1 adds NLA_PAD, this datatype is needed by netlink description.
Patch 2 adds the new generic netlink description bus NETLINK_DESC.
Patch 3 adds support for netlink descriptions to nfnetlink.
Patch 4 adds a netlink description for nf_tables, so you have an initial
	client for this infrastructure to look at as reference.

Not covered by this patchset, but I think it should be possible to
(fully?) generate the C file containing the description and the netlink
headers based on some generic notation that describes the netlink
interface, something like this:

    netlink description  ---> nldesc_compiler ---> .c and .h files
         notation

This would probably make the work of describing the netlink interface
less error prone, given people may add a new attribute to the header
file and forget about updating the description. I also think this
description could be also useful to fuzz tests netlink interfaces, by
interpreting the description that the kernel provides.

I've been looking into this since NetDev 2.1 in Canada - already one
year ago - I think we need this in nftables so we can evolve more
freely.

Comments welcome.

Pablo Neira Ayuso (4):
  netlink: add NLA_PAD definition
  netlink: add generic object description infrastructure
  netfilter: nfnetlink: add support for netlink descriptions
  netfilter: nf_tables: add netlink description

 include/linux/netfilter/nfnetlink.h           |   9 +
 include/net/net_namespace.h                   |   1 +
 include/net/netfilter/nf_tables.h             |   2 +
 include/net/netlink.h                         |   2 +
 include/net/nldesc.h                          | 163 +++++++++
 include/uapi/linux/netfilter/nf_tables_desc.h |  57 +++
 include/uapi/linux/netfilter/nfnetlink.h      |   7 +
 include/uapi/linux/netlink.h                  |  67 ++++
 net/netfilter/Makefile                        |   7 +-
 net/netfilter/nf_tables_api.c                 |   2 +
 net/netfilter/nf_tables_desc.c                | 471 ++++++++++++++++++++++++
 net/netfilter/nfnetlink.c                     | 108 ++++++
 net/netlink/Makefile                          |   2 +-
 net/netlink/desc.c                            | 499 ++++++++++++++++++++++++++
 14 files changed, 1393 insertions(+), 4 deletions(-)
 create mode 100644 include/net/nldesc.h
 create mode 100644 include/uapi/linux/netfilter/nf_tables_desc.h
 create mode 100644 net/netfilter/nf_tables_desc.c
 create mode 100644 net/netlink/desc.c

-- 
2.11.0

[PATCH RFC 1/4] netlink: add NLA_PAD definition

[Pablo Neira Ayuso]

The new generic netlink description infrastructure needs this new type
to describe padding attributes.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netlink.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 0c154f98e987..76e850ead593 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -180,6 +180,7 @@ enum {
 	NLA_S32,
 	NLA_S64,
 	NLA_BITFIELD32,
+	NLA_PAD,
 	__NLA_TYPE_MAX,
 };
 
@@ -209,6 +210,7 @@ enum {
  *                         given type fits, using it verifies minimum length
  *                         just like "All other"
  *    NLA_BITFIELD32      A 32-bit bitmap/bitselector attribute
+ *    NLA_PAD		   Empty attribute to align next attribute to 64-bits
  *    All other            Minimum length of attribute payload
  *
  * Example:
-- 
2.11.0

Re: [PATCH RFC 1/4] netlink: add NLA_PAD definition

[Johannes Berg]

On Wed, 2018-02-07 at 02:37 +0100, Pablo Neira Ayuso wrote:
> The new generic netlink description infrastructure needs this new type
> to describe padding attributes.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/net/netlink.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 0c154f98e987..76e850ead593 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -180,6 +180,7 @@ enum {
>  	NLA_S32,
>  	NLA_S64,
>  	NLA_BITFIELD32,
> +	NLA_PAD,
>  	__NLA_TYPE_MAX,
>  };
>  
> @@ -209,6 +210,7 @@ enum {
>   *                         given type fits, using it verifies minimum length
>   *                         just like "All other"
>   *    NLA_BITFIELD32      A 32-bit bitmap/bitselector attribute
> + *    NLA_PAD		   Empty attribute to align next attribute to 64-bits

I guess this should also move to uapi/linux/netlink.h.

johannes

[PATCH RFC 2/4] netlink: add generic object description infrastructure

[Pablo Neira Ayuso]

This patch allows netlink busses to provide object descriptions to
userspace, in terms of supported attributes and its corresponding
datatypes.

Userspace sends a requests that looks like:

	netlink header
	NLA_DESC_REQ_BUS
	NLA_DESC_REQ_DATA

Where NLA_DESC_REQ_BUS is the netlink bus/protocol number, eg.
NETLINK_NETFILTER, and NLA_DESC_REQ_DATA is an attribute layout is
specific to the bus that you are inspecting, this is useful for both
nfnetlink and genetlink since they need to what subsystem in the bus
specifically you're targeting to.

Then, the netlink description subsystem response via netlink dump looks
like this:

	netlink header
	NLA_DESC_NUM_OBJS
	NLA_DESC_OBJS (nest)
		NLA_DESC_LIST_ITEM (nest)
			NLA_DESC_OBJ_ID
			NLA_DESC_OBJ_ATTRS_MAX
			NLA_DESC_OBJ_ATTRS (nest)
				NLA_DESC_LIST_ITEM (nest)
					NLA_DESC_ATTR_NUM
					NLA_DESC_ATTR_TYPE
					NLA_DESC_ATTR_LEN
					NLA_DESC_ATTR_MAXVAL
					NLA_DESC_ATTR_NEST_ID
		NLA_DESC_LIST_ITEM (nest)
			...

Each object definition is composed of an unique ID, the number of
attributes and the list of attribute definitions.

The NETLINK_DESC bus provides a generic interface to retrieve the list
of existing objects and its attributes via netlink dump. This new
description family autoloads module dependencies based on what userspace
requests.

Each bus needs to register a struct nl_desc_subsys definition, that
provides the lookup and parse callbacks. These route the description
requests to the corresponding backend subsystem for genetlink and
nfnetlink. The lookup callback returns struct nl_desc_objs that provides
the array of object descriptions.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/net_namespace.h  |   1 +
 include/net/nldesc.h         | 160 ++++++++++++++
 include/uapi/linux/netlink.h |  67 ++++++
 net/netlink/Makefile         |   2 +-
 net/netlink/desc.c           | 499 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 728 insertions(+), 1 deletion(-)
 create mode 100644 include/net/nldesc.h
 create mode 100644 net/netlink/desc.c

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index f8a84a2c2341..0921b1d7acfe 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -78,6 +78,7 @@ struct net {
 
 	struct sock 		*rtnl;			/* rtnetlink socket */
 	struct sock		*genl_sock;
+	struct sock		*nl_desc_sock;
 
 	struct list_head 	dev_base_head;
 	struct hlist_head 	*dev_name_head;
diff --git a/include/net/nldesc.h b/include/net/nldesc.h
new file mode 100644
index 000000000000..19306a648f10
--- /dev/null
+++ b/include/net/nldesc.h
@@ -0,0 +1,160 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NET_NLDESC_H
+#define __NET_NLDESC_H
+
+#include <linux/types.h>
+
+struct nl_desc_cmd;
+struct nl_desc_obj;
+
+struct nl_desc_cmds {
+	int				max;
+	const struct nl_desc_cmd	*table;
+};
+
+struct nl_desc_objs {
+	int				max;
+	const struct nl_desc_obj	**table;
+};
+
+struct nl_desc_req {
+	u32				bus;
+};
+
+struct net;
+struct sk_buff;
+struct nlmsghdr;
+struct nlattr;
+
+struct nl_desc_subsys {
+	struct list_head		list;
+	u32				bus;
+	const struct nl_desc_cmds *	(*getcmds)(struct sk_buff *skb,
+						   struct nlmsghdr *nlh,
+						   struct nl_desc_req *req);
+	const struct nl_desc_objs *	(*getobjs)(struct sk_buff *skb,
+						   struct nlmsghdr *nlh,
+						   struct nl_desc_req *req);
+	int				(*parse)(struct net *net,
+						 struct sk_buff *skb,
+						 struct nlmsghdr *nlh,
+						 const struct nlattr *data,
+						 struct nl_desc_req *req);
+};
+
+/**
+ * struct nl_desc_attr - netlink attribute description
+ * @nest: netlink description for nested attribute
+ * @attr: attribute number
+ * @type: attribute datatype (see NLA_* enumeration)
+ * @len: attribute payload length
+ * @max: attribute maximum value (upper limit if any, zero means unset)
+ */
+struct nl_desc_attr {
+	const struct nl_desc_obj	*nest;
+	u16				attr;
+	u16				type;
+	u16				len;
+	u16				max;
+};
+
+/**
+ * struct nl_desc_obj - netlink object description
+ * @id: unique ID to identify this netlink object
+ * @max: number of attributes to describe this object
+ * @attrs: array of attribute descriptions
+ */
+struct nl_desc_obj {
+	u16				id;
+	u16				attr_max;
+	const struct nl_desc_attr	*attrs;
+};
+
+#define NLDESC_OBJ(__id, __attrs, __max)		\
+	{						\
+		.id		= __id,			\
+		.attr_max	= __max,		\
+		.attrs		= __attrs,		\
+	}
+
+#define NLDESC_OBJ_END	{ 0, 0, NULL }
+
+#define NLDESC_ATTR_MAX(__attr, __type, __len, __max)	\
+	{						\
+		.attr		= __attr,		\
+		.type		= __type,		\
+		.len		= __len,		\
+		.max		= __max,		\
+	}
+
+#define NLDESC_ATTR(__attr, __type, __len)		\
+	NLDESC_ATTR_MAX(__attr, __type, __len, 0)
+
+#define NLDESC_ATTR_NESTED(__attr, __nest)		\
+	{						\
+		.attr		= __attr,		\
+		.type		= NLA_NESTED,		\
+		.nest		= __nest,		\
+	}
+
+#define NLDESC_ATTR_STRING(__attr, __len)		\
+	NLDESC_ATTR(__attr, NLA_STRING, __len)
+
+#define NLDESC_ATTR_NUL_STRING(__attr)			\
+	NLDESC_ATTR(__attr, NLA_NUL_STRING, 0)
+
+#define NLDESC_ATTR_U8_MAX(__attr, __max)		\
+	NLDESC_ATTR_MAX(__attr, NLA_U8, sizeof(u8), __max)
+
+#define NLDESC_ATTR_U16_MAX(__attr, __max)		\
+	NLDESC_ATTR_MAX(__attr, NLA_U16, sizeof(u16), __max)
+
+#define NLDESC_ATTR_U32_MAX(__attr, __max)		\
+	NLDESC_ATTR_MAX(__attr, NLA_U32, sizeof(u32), __max)
+
+#define NLDESC_ATTR_U64_MAX(__attr, __max)		\
+	NLDESC_ATTR_MAX(__attr, NLA_U64, sizeof(u64), __max)
+
+#define NLDESC_ATTR_U8(__attr)				\
+	NLDESC_ATTR_U8_MAX(__attr, 0)
+
+#define NLDESC_ATTR_U16(__attr)				\
+	NLDESC_ATTR_U16_MAX(__attr, 0)
+
+#define NLDESC_ATTR_U32(__attr)				\
+	NLDESC_ATTR_U32_MAX(__attr, 0)
+
+#define NLDESC_ATTR_U64(__attr)				\
+	NLDESC_ATTR_U64_MAX(__attr, 0)
+
+#define NLDESC_ATTR_BINARY(__attr, __len)		\
+	NLDESC_ATTR(__attr, NLA_BINARY, __len)
+
+#define NLDESC_ATTR_PAD(__attr)				\
+	NLDESC_ATTR(__attr, NLA_PAD, 0)
+
+/**
+ * struct nl_desc_cmd - netlink command description
+ * @cmd: command identifier
+ * @obj_id: netlink object that this command takes
+ */
+struct nl_desc_cmd {
+	u16				cmd;
+	u16				obj_id;
+};
+
+#define NLDESC_CMD(__cmd, __obj_id)			\
+	{						\
+		.cmd		= __cmd,		\
+		.obj_id		= __obj_id,		\
+	}
+
+#define NLDESC_CMD_END	{ 0, 0 }
+
+int nl_desc_register_subsys(struct nl_desc_subsys *subsys);
+void nl_desc_unregister_subsys(struct nl_desc_subsys *subsys);
+
+#define MODULE_ALIAS_NLDESC(bus) \
+        MODULE_ALIAS("nl-desc-" __stringify(bus))
+
+#endif
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 776bc92e9118..208946e84893 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -29,6 +29,7 @@
 #define NETLINK_RDMA		20
 #define NETLINK_CRYPTO		21	/* Crypto layer */
 #define NETLINK_SMC		22	/* SMC monitoring */
+#define NETLINK_DESC		23
 
 #define NETLINK_INET_DIAG	NETLINK_SOCK_DIAG
 
@@ -248,4 +249,70 @@ struct nla_bitfield32 {
 	__u32 selector;
 };
 
+enum {
+	NLDESC_GET_CMDS		= 16,
+	NLDESC_NEW_CMDS,
+	NLDESC_GET_OBJS,
+	NLDESC_NEW_OBJS,
+};
+
+enum nft_nldesc_req_attributes {
+	NLA_DESC_REQ_UNSPEC,
+	NLA_DESC_REQ_BUS,
+	NLA_DESC_REQ_DATA,
+	__NLA_DESC_REQ_MAX,
+};
+#define NLA_DESC_REQ_MAX	(__NLA_DESC_REQ_MAX - 1)
+
+enum nft_nldesc_attributes {
+	NLA_DESC_UNSPEC,
+	NLA_DESC_NUM_OBJS,
+	NLA_DESC_OBJS,
+	__NLA_DESC_MAX,
+};
+#define NLA_DESC_MAX		(__NLA_DESC_MAX - 1)
+
+enum nft_nldesc_cmds_attributes {
+	NLA_DESC_CMDS_UNSPEC,
+	NLA_DESC_CMDS_NUM,
+	NLA_DESC_CMDS,
+	__NLA_DESC_CMDS_MAX,
+};
+#define NLA_DESC_CMDS_MAX	(__NLA_DESC_CMDS_MAX - 1)
+
+enum nft_nldesc_list_attributes {
+	NLA_DESC_LIST_UNSPEC,
+	NLA_DESC_LIST_ITEM,
+	__NLA_DESC_LIST_MAX,
+};
+#define NLA_DESC_LIST_MAX	(__NLA_DESC_LIST_MAX - 1)
+
+enum nft_nldesc_obj_attributes {
+	NLA_DESC_OBJ_UNSPEC,
+	NLA_DESC_OBJ_ID,
+	NLA_DESC_OBJ_ATTRS_MAX,
+	NLA_DESC_OBJ_ATTRS,
+	__NLA_DESC_OBJ_MAX,
+};
+#define NLA_DESC_OBJ_MAX	(__NLA_DESC_OBJ_MAX - 1)
+
+enum nft_nldesc_attr_attributes {
+	NLA_DESC_ATTR_UNSPEC,
+	NLA_DESC_ATTR_NUM,
+	NLA_DESC_ATTR_TYPE,
+	NLA_DESC_ATTR_LEN,
+	NLA_DESC_ATTR_MAXVAL,
+	NLA_DESC_ATTR_NEST_ID,
+	__NLA_DESC_ATTR_MAX,
+};
+#define NLA_DESC_ATTR_MAX	(__NLA_DESC_ATTR_MAX - 1)
+
+enum nft_nldesc_cmd_attributes {
+	NLA_DESC_CMD_UNSPEC,
+	NLA_DESC_CMD_ID,
+	NLA_DESC_CMD_OBJ,
+	__NLA_DESC_CMD_MAX,
+};
+#define NLA_DESC_CMD_MAX	(__NLA_DESC_CMD_MAX - 1)
+
 #endif /* _UAPI__LINUX_NETLINK_H */
diff --git a/net/netlink/Makefile b/net/netlink/Makefile
index e837917f6c03..0e44f10a084b 100644
--- a/net/netlink/Makefile
+++ b/net/netlink/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the netlink driver.
 #
 
-obj-y  				:= af_netlink.o genetlink.o
+obj-y  				:= af_netlink.o genetlink.o desc.o
 
 obj-$(CONFIG_NETLINK_DIAG)	+= netlink_diag.o
 netlink_diag-y			:= diag.o
diff --git a/net/netlink/desc.c b/net/netlink/desc.c
new file mode 100644
index 000000000000..55aaf6da0a24
--- /dev/null
+++ b/net/netlink/desc.c
@@ -0,0 +1,499 @@
+#include <net/net_namespace.h>
+#include <linux/skbuff.h>
+#include <linux/types.h>
+#include <net/netlink.h>
+#include <net/nldesc.h>
+#include <net/sock.h>
+
+static DEFINE_MUTEX(nl_desc_mutex);
+static LIST_HEAD(nl_desc_subsys_list);
+
+static int nl_fill_desc_attr_info(struct sk_buff *skb,
+				  const struct nl_desc_attr *entry)
+{
+	if (nla_put_u32(skb, NLA_DESC_ATTR_NUM, entry->attr) ||
+	    nla_put_u32(skb, NLA_DESC_ATTR_TYPE, entry->type))
+		goto nla_put_failure;
+
+	if (entry->nest && entry->nest->id) {
+		if (nla_put_u32(skb, NLA_DESC_ATTR_NEST_ID, entry->nest->id))
+			goto nla_put_failure;
+	} else if (nla_put_u32(skb, NLA_DESC_ATTR_LEN, entry->len)) {
+		goto nla_put_failure;
+	}
+
+	if (entry->max && nla_put_u32(skb, NLA_DESC_ATTR_MAXVAL, entry->max))
+		goto nla_put_failure;
+
+	return 0;
+
+nla_put_failure:
+	return -1;
+}
+
+static int nl_fill_desc_attr_item_info(struct sk_buff *skb,
+				       const struct nl_desc_attr *entry)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, NLA_DESC_LIST_ITEM | NLA_F_NESTED);
+	if (!nest)
+		return -1;
+
+	if (nl_fill_desc_attr_info(skb, entry) < 0)
+		return -1;
+
+	nla_nest_end(skb, nest);
+
+	return 0;
+}
+
+static int nl_fill_desc_attrs_info(struct sk_buff *skb,
+				   const struct nl_desc_attr *attrs,
+				   int attrs_max)
+{
+	struct nlattr *nest;
+	int k;
+
+	nest = nla_nest_start(skb, NLA_DESC_OBJ_ATTRS | NLA_F_NESTED);
+	if (!nest)
+		return -1;
+
+	for (k = 0; k < attrs_max; k++) {
+		if (nl_fill_desc_attr_item_info(skb, &attrs[k]))
+			return -1;
+	}
+	nla_nest_end(skb, nest);
+
+	return 0;
+}
+
+static int nl_fill_desc_objs(struct sk_buff *skb,
+			     const struct nl_desc_obj *desc)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, NLA_DESC_LIST_ITEM | NLA_F_NESTED);
+	if (!nest)
+		goto nla_put_failure;
+
+	if (nla_put_u32(skb, NLA_DESC_OBJ_ID, desc->id) ||
+	    nla_put_u32(skb, NLA_DESC_OBJ_ATTRS_MAX, desc->attr_max))
+		goto nla_put_failure;
+
+	if (nl_fill_desc_attrs_info(skb, desc->attrs, desc->attr_max) < 0)
+		goto nla_put_failure;
+
+	nla_nest_end(skb, nest);
+
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -1;
+}
+
+static int nl_fill_desc_cmds(struct sk_buff *skb,
+			     const struct nl_desc_cmd *desc)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, NLA_DESC_LIST_ITEM | NLA_F_NESTED);
+	if (!nest)
+		goto nla_put_failure;
+
+	if (nla_put_u32(skb, NLA_DESC_CMD_ID, desc->cmd) ||
+	    nla_put_u32(skb, NLA_DESC_CMD_OBJ, desc->obj_id))
+		goto nla_put_failure;
+
+	nla_nest_end(skb, nest);
+
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -1;
+}
+
+#define NLDESC_DUMP_DONE	~0ULL
+
+static void nl_dump_desc_objs_info(struct sk_buff *skb, unsigned long *index,
+				   const struct nl_desc_obj *nl_desc_obj_array[],
+				   u32 num_objects)
+{
+	const struct nl_desc_obj *desc, *last = (struct nl_desc_obj *)*index;
+	struct nlattr *nest;
+	int i, j;
+
+	if (!last && nla_put_u32(skb, NLA_DESC_NUM_OBJS, num_objects))
+		goto out;
+
+	nest = nla_nest_start(skb, NLA_DESC_OBJS | NLA_F_NESTED);
+	if (!nest)
+		goto out;
+
+	for (i = 0; nl_desc_obj_array[i]; i++) {
+		for (j = 0; nl_desc_obj_array[i][j].id; j++) {
+			desc = &nl_desc_obj_array[i][j];
+			if (last) {
+				if (last != desc)
+					continue;
+				else
+					last = NULL;
+			}
+
+			if (nl_fill_desc_objs(skb, desc))
+				goto nla_put_failure;
+		}
+	}
+	nla_nest_end(skb, nest);
+out:
+	*index = NLDESC_DUMP_DONE;
+	return;
+
+nla_put_failure:
+	nla_nest_end(skb, nest);
+	*index = (unsigned long)desc;
+	return;
+}
+
+static void nl_dump_desc_cmds_info(struct sk_buff *skb, unsigned long *index,
+				   const struct nl_desc_cmd nl_desc_cmd_array[],
+				   u32 num_cmds)
+{
+	const struct nl_desc_cmd *desc, *last = (struct nl_desc_cmd *)*index;
+	struct nlattr *nest;
+	int i;
+
+	if (!last && nla_put_u32(skb, NLA_DESC_CMDS_NUM, num_cmds))
+		goto out;
+
+	nest = nla_nest_start(skb, NLA_DESC_CMDS | NLA_F_NESTED);
+	if (!nest)
+		goto out;
+
+	for (i = 0; nl_desc_cmd_array[i].obj_id; i++) {
+		desc = &nl_desc_cmd_array[i];
+		if (last) {
+			if (last != desc)
+				continue;
+			else
+				last = NULL;
+		}
+		if (nl_fill_desc_cmds(skb, desc))
+			goto nla_put_failure;
+	}
+	nla_nest_end(skb, nest);
+out:
+	*index = NLDESC_DUMP_DONE;
+	return;
+
+nla_put_failure:
+	nla_nest_end(skb, nest);
+	*index = (unsigned long)desc;
+	return;
+}
+
+static struct nl_desc_subsys *nl_desc_find(struct net *net, int bus)
+{
+	struct nl_desc_subsys *subsys;
+
+	list_for_each_entry_rcu(subsys, &nl_desc_subsys_list, list) {
+		if (subsys->bus != bus)
+			continue;
+
+		return subsys;
+	}
+
+	return NULL;
+}
+
+static int nl_dump_nldesc_cmds_callback(struct sk_buff *skb,
+					struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nl_desc_req *req = cb->data;
+	const struct nl_desc_cmds *cmds;
+	struct nl_desc_subsys *subsys;
+	struct nlmsghdr *nlh;
+	int err;
+
+	if (cb->args[0] == NLDESC_DUMP_DONE)
+		return 0;
+
+	rcu_read_lock();
+	subsys = nl_desc_find(net, req->bus);
+	if (!subsys) {
+		err = -ENOENT;
+		goto err;
+	}
+
+	cmds = subsys->getcmds(skb, nlh, req);
+	if (IS_ERR(cmds)) {
+		err = PTR_ERR(cmds);
+		goto err;
+	}
+	rcu_read_unlock();
+
+	nlh = nlmsg_put(skb, NETLINK_CB(skb).portid, cb->nlh->nlmsg_seq,
+			NLDESC_NEW_CMDS, 0, 0);
+	if (nlh == NULL)
+		return -ENOMEM;
+
+	nl_dump_desc_cmds_info(skb, &cb->args[0], cmds->table, cmds->max);
+	nlmsg_end(skb, nlh);
+
+	return skb->len;
+err:
+	rcu_read_unlock();
+	return err;
+}
+
+static int nl_dump_nldesc_objs_callback(struct sk_buff *skb,
+					struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nl_desc_req *req = cb->data;
+	const struct nl_desc_objs *objs;
+	struct nl_desc_subsys *subsys;
+	struct nlmsghdr *nlh;
+	int err;
+
+	if (cb->args[0] == NLDESC_DUMP_DONE)
+		return 0;
+
+	rcu_read_lock();
+	subsys = nl_desc_find(net, req->bus);
+	if (!subsys) {
+		err = -ENOENT;
+		goto err;
+	}
+
+	objs = subsys->getobjs(skb, nlh, req);
+	if (IS_ERR(objs)) {
+		err = PTR_ERR(objs);
+		goto err;
+	}
+	rcu_read_unlock();
+
+	nlh = nlmsg_put(skb, NETLINK_CB(skb).portid, cb->nlh->nlmsg_seq,
+			NLDESC_NEW_OBJS, 0, 0);
+	if (nlh == NULL)
+		return -ENOMEM;
+
+	nl_dump_desc_objs_info(skb, &cb->args[0], objs->table, objs->max);
+	nlmsg_end(skb, nlh);
+
+	return skb->len;
+err:
+	rcu_read_unlock();
+	return err;
+}
+
+static int nl_dump_nldesc_done(struct netlink_callback *cb)
+{
+	kfree(cb->data);
+
+	return 0;
+}
+
+static int nl_desc_handle(struct net *net, struct sk_buff *skb,
+			  struct nlmsghdr *nlh, struct nl_desc_req *req,
+			  int (*nl_dump_nldesc)(struct sk_buff *skb,
+						struct netlink_callback *cb))
+{
+	struct netlink_dump_control c = {
+		.dump 	= nl_dump_nldesc,
+		.done	= nl_dump_nldesc_done,
+		.data 	= req,
+	};
+
+	return netlink_dump_start(net->nl_desc_sock, skb, nlh, &c);
+}
+
+static const struct nla_policy nl_desc_policy[NLA_DESC_REQ_MAX + 1] = {
+	[NLA_DESC_REQ_BUS]	= { .type = NLA_U32 },
+	[NLA_DESC_REQ_DATA]	= { .type = NLA_NESTED },
+};
+
+static int nl_desc_handle_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack)
+{
+	int min_len = nlmsg_total_size(0);
+	struct nlattr *attr = (void *)nlh + min_len;
+	struct nlattr *tb[NLA_DESC_REQ_MAX + 1];
+	int attrlen = nlh->nlmsg_len - min_len;
+	struct net *net = sock_net(skb->sk);
+	const struct nl_desc_subsys *subsys;
+	const struct nl_desc_objs *objs;
+	const struct nl_desc_cmds *cmds;
+	struct nl_desc_req *req;
+	u32 bus;
+	int err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(0))
+		return -EINVAL;
+
+	if (!netlink_capable(skb, CAP_NET_ADMIN))
+		return -EPERM;
+
+	if (!(nlh->nlmsg_flags & NLM_F_DUMP))
+		return -EOPNOTSUPP;
+
+	if (nlh->nlmsg_type != NLDESC_GET_CMDS &&
+	    nlh->nlmsg_type != NLDESC_GET_OBJS)
+		return -EOPNOTSUPP;
+
+	err = nla_parse(tb, NLA_DESC_REQ_MAX, attr, attrlen, nl_desc_policy,
+			extack);
+	if (err < 0)
+		return err;
+
+	if (!tb[NLA_DESC_REQ_BUS])
+		return -EINVAL;
+
+	bus = nla_get_u32(tb[NLA_DESC_REQ_BUS]);
+
+	rcu_read_lock();
+	subsys = nl_desc_find(net, bus);
+	if (!subsys) {
+#ifdef CONFIG_MODULES
+		rcu_read_unlock();
+		request_module("nl-desc-%u", bus);
+		rcu_read_lock();
+#endif
+		subsys = nl_desc_find(net, bus);
+		if (!subsys) {
+			err = -ENOENT;
+			goto err;
+		}
+	}
+
+	req = kzalloc(sizeof(struct nl_desc_req), GFP_ATOMIC);
+	if (!req) {
+		err = -ENOMEM;
+		goto err;
+	}
+	req->bus = subsys->bus;
+
+	if (tb[NLA_DESC_REQ_DATA]) {
+		/* This may return -EAGAIN to autoload module dependencies. */
+		err = subsys->parse(net, skb, nlh, tb[NLA_DESC_REQ_DATA], req);
+		if (err < 0) {
+			kfree(req);
+			goto err;
+		}
+		/* Make sure subsystem is loaded before starting netlink dump. */
+		switch (nlh->nlmsg_type) {
+		case NLDESC_GET_CMDS:
+			cmds = subsys->getcmds(skb, nlh, req);
+			if (IS_ERR(cmds)) {
+				err = PTR_ERR(cmds);
+				goto err;
+			}
+			break;
+		case NLDESC_GET_OBJS:
+			objs = subsys->getobjs(skb, nlh, req);
+			if (IS_ERR(objs)) {
+				err = PTR_ERR(objs);
+				goto err;
+			}
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	switch (nlh->nlmsg_type) {
+	case NLDESC_GET_CMDS:
+		err = nl_desc_handle(net, skb, nlh, req,
+				     nl_dump_nldesc_cmds_callback);
+		break;
+	case NLDESC_GET_OBJS:
+		err = nl_desc_handle(net, skb, nlh, req,
+				     nl_dump_nldesc_objs_callback);
+		break;
+	}
+	return err;
+err:
+	rcu_read_unlock();
+	return err;
+}
+
+static int nl_desc_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
+			   struct netlink_ext_ack *extack)
+{
+	bool retry = false;
+	int err;
+
+	do {
+		err = nl_desc_handle_msg(skb, nlh, extack);
+		if (err == -EAGAIN && !retry) {
+			retry = true;
+			continue;
+		}
+		break;
+	} while (1);
+
+	return err;
+}
+
+static void nl_desc_rcv(struct sk_buff *skb)
+{
+	netlink_rcv_skb(skb, &nl_desc_rcv_msg);
+}
+
+int nl_desc_register_subsys(struct nl_desc_subsys *subsys)
+{
+	if (subsys->bus >= MAX_LINKS)
+		return -ENOENT;
+
+	mutex_lock(&nl_desc_mutex);
+	list_add_rcu(&subsys->list, &nl_desc_subsys_list);
+	mutex_unlock(&nl_desc_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nl_desc_register_subsys);
+
+void nl_desc_unregister_subsys(struct nl_desc_subsys *subsys)
+{
+	mutex_lock(&nl_desc_mutex);
+	list_del_rcu(&subsys->list);
+	mutex_unlock(&nl_desc_mutex);
+
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(nl_desc_unregister_subsys);
+
+static int __net_init nl_desc_pernet_init(struct net *net)
+{
+	struct netlink_kernel_cfg cfg = {
+		.input		= nl_desc_rcv,
+	};
+
+	net->nl_desc_sock = netlink_kernel_create(net, NETLINK_DESC, &cfg);
+	if (!net->nl_desc_sock && net_eq(net, &init_net))
+		panic("Cannot initialize netlink description bus\n");
+
+	if (!net->nl_desc_sock)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void __net_exit nl_desc_pernet_exit(struct net *net)
+{
+	netlink_kernel_release(net->nl_desc_sock);
+	net->nl_desc_sock = NULL;
+}
+
+static struct pernet_operations nl_desc_pernet_ops = {
+	.init = nl_desc_pernet_init,
+	.exit = nl_desc_pernet_exit,
+};
+
+static int __init nl_desc_init(void)
+{
+	return register_pernet_subsys(&nl_desc_pernet_ops);
+}
+subsys_initcall(nl_desc_init);
-- 
2.11.0

Re: [PATCH RFC 2/4] netlink: add generic object description infrastructure

[Randy Dunlap]

On 02/06/2018 05:37 PM, Pablo Neira Ayuso wrote:
> This patch allows netlink busses to provide object descriptions to
> userspace, in terms of supported attributes and its corresponding
> datatypes.
> 
> Userspace sends a requests that looks like:
> 
> 	netlink header
> 	NLA_DESC_REQ_BUS
> 	NLA_DESC_REQ_DATA
> 
> Where NLA_DESC_REQ_BUS is the netlink bus/protocol number, eg.
> NETLINK_NETFILTER, and NLA_DESC_REQ_DATA is an attribute layout is
> specific to the bus that you are inspecting, this is useful for both
> nfnetlink and genetlink since they need to what subsystem in the bus
> specifically you're targeting to.
> 
> Then, the netlink description subsystem response via netlink dump looks
> like this:
> 
> 	netlink header
> 	NLA_DESC_NUM_OBJS
> 	NLA_DESC_OBJS (nest)
> 		NLA_DESC_LIST_ITEM (nest)
> 			NLA_DESC_OBJ_ID
> 			NLA_DESC_OBJ_ATTRS_MAX
> 			NLA_DESC_OBJ_ATTRS (nest)
> 				NLA_DESC_LIST_ITEM (nest)
> 					NLA_DESC_ATTR_NUM
> 					NLA_DESC_ATTR_TYPE
> 					NLA_DESC_ATTR_LEN
> 					NLA_DESC_ATTR_MAXVAL
> 					NLA_DESC_ATTR_NEST_ID
> 		NLA_DESC_LIST_ITEM (nest)
> 			...
> 
> Each object definition is composed of an unique ID, the number of
> attributes and the list of attribute definitions.
> 
> The NETLINK_DESC bus provides a generic interface to retrieve the list
> of existing objects and its attributes via netlink dump. This new
> description family autoloads module dependencies based on what userspace
> requests.
> 
> Each bus needs to register a struct nl_desc_subsys definition, that
> provides the lookup and parse callbacks. These route the description
> requests to the corresponding backend subsystem for genetlink and
> nfnetlink. The lookup callback returns struct nl_desc_objs that provides
> the array of object descriptions.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/net/net_namespace.h  |   1 +
>  include/net/nldesc.h         | 160 ++++++++++++++
>  include/uapi/linux/netlink.h |  67 ++++++
>  net/netlink/Makefile         |   2 +-
>  net/netlink/desc.c           | 499 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 728 insertions(+), 1 deletion(-)
>  create mode 100644 include/net/nldesc.h
>  create mode 100644 net/netlink/desc.c
> 

> diff --git a/include/net/nldesc.h b/include/net/nldesc.h
> new file mode 100644
> index 000000000000..19306a648f10
> --- /dev/null
> +++ b/include/net/nldesc.h
> @@ -0,0 +1,160 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __NET_NLDESC_H
> +#define __NET_NLDESC_H
> +
> +#include <linux/types.h>
> +
> +struct nl_desc_cmd;
> +struct nl_desc_obj;
> +
> +struct nl_desc_cmds {
> +	int				max;
> +	const struct nl_desc_cmd	*table;
> +};
> +
> +struct nl_desc_objs {
> +	int				max;
> +	const struct nl_desc_obj	**table;
> +};
> +
> +struct nl_desc_req {
> +	u32				bus;
> +};
> +
> +struct net;
> +struct sk_buff;
> +struct nlmsghdr;
> +struct nlattr;
> +

> +
> +/**
> + * struct nl_desc_obj - netlink object description
> + * @id: unique ID to identify this netlink object
> + * @max: number of attributes to describe this object

      @attr_max:

> + * @attrs: array of attribute descriptions
> + */
> +struct nl_desc_obj {
> +	u16				id;
> +	u16				attr_max;
> +	const struct nl_desc_attr	*attrs;
> +};


Is there a test program for this?
Maybe add it to tools/testing/ ?

thanks,
-- 
~Randy

Re: [PATCH RFC 2/4] netlink: add generic object description infrastructure

[Pablo Neira Ayuso]

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

Hi Randy,

On Wed, Feb 07, 2018 at 05:28:20PM -0800, Randy Dunlap wrote:
[...]
> > diff --git a/include/net/nldesc.h b/include/net/nldesc.h
> > new file mode 100644
> > index 000000000000..19306a648f10
> > --- /dev/null
> > +++ b/include/net/nldesc.h
> > @@ -0,0 +1,160 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __NET_NLDESC_H
> > +#define __NET_NLDESC_H
> > +
> > +#include <linux/types.h>
> > +
> > +struct nl_desc_cmd;
> > +struct nl_desc_obj;
> > +
> > +struct nl_desc_cmds {
> > +	int				max;
> > +	const struct nl_desc_cmd	*table;
> > +};
> > +
> > +struct nl_desc_objs {
> > +	int				max;
> > +	const struct nl_desc_obj	**table;
> > +};
> > +
> > +struct nl_desc_req {
> > +	u32				bus;
> > +};
> > +
> > +struct net;
> > +struct sk_buff;
> > +struct nlmsghdr;
> > +struct nlattr;
> > +
> 
> > +
> > +/**
> > + * struct nl_desc_obj - netlink object description
> > + * @id: unique ID to identify this netlink object
> > + * @max: number of attributes to describe this object
> 
>       @attr_max:

Thanks for spotting this.

> > + * @attrs: array of attribute descriptions
> > + */
> > +struct nl_desc_obj {
> > +	u16				id;
> > +	u16				attr_max;
> > +	const struct nl_desc_attr	*attrs;
> > +};
> 
> 
> Is there a test program for this?

I'm attaching what I have used to test this. These files print the
netlink bus description.

> Maybe add it to tools/testing/ ?

Yes, I can place it there, no problem. This userspace code depends on
libmnl though.

I was planning to add infrastructure to libmnl to add a couple of helper
functions that allows us to populate the nl_desc cache and to look up
for presence of commands/attributes.

People that don't like libmnl for whatever reason can add similar code
to their libraries too, of course.

Thanks!

[-- Attachment #2: 0001-examples-add-netlink-bus-description.patch --]
[-- Type: text/x-diff, Size: 12199 bytes --]

>From 7826d6aa47d20bc09f7c8e33a457a5a338a8db55 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 16 Jan 2018 00:05:37 +0100
Subject: [PATCH libmnl] examples: add netlink bus description

Add nft-dump-desc-cmds.c and nft-dump-desc-obj.c to dump command and
object descriptions.
---
 examples/Makefile.am          |  11 ++
 examples/nft-dump-desc-cmds.c | 177 ++++++++++++++++++++++++++++
 examples/nft-dump-desc-objs.c | 263 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 451 insertions(+)
 create mode 100644 examples/nft-dump-desc-cmds.c
 create mode 100644 examples/nft-dump-desc-objs.c

diff --git a/examples/Makefile.am b/examples/Makefile.am
index e5cb052b315c..a8d4ba50f5ad 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -1 +1,12 @@
+include $(top_srcdir)/Make_global.am
+
 SUBDIRS = genl kobject netfilter rtnl
+
+check_PROGRAMS = nft-dump-desc-cmds \
+                 nft-dump-desc-objs
+
+nft_dump_desc_cmds_SOURCES = nft-dump-desc-cmds.c
+nft_dump_desc_cmds_LDADD = ../src/libmnl.la
+
+nft_dump_desc_objs_SOURCES = nft-dump-desc-objs.c
+nft_dump_desc_objs_LDADD = ../src/libmnl.la
diff --git a/examples/nft-dump-desc-cmds.c b/examples/nft-dump-desc-cmds.c
new file mode 100644
index 000000000000..cfb5276e911f
--- /dev/null
+++ b/examples/nft-dump-desc-cmds.c
@@ -0,0 +1,177 @@
+#include <endian.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <time.h>
+#include <arpa/inet.h>
+#include <inttypes.h>
+
+#include <linux/netlink.h>
+#include <linux/netfilter/nfnetlink.h>
+
+#include <libmnl/libmnl.h>
+
+struct nl_desc_cmd;
+struct nl_desc_attr;
+
+struct nl_desc {
+	uint32_t			num_cmds;
+	struct nl_desc_cmd		*cmds;
+};
+
+struct nl_desc_cmd {
+	uint32_t			id;
+	uint32_t			obj_id;
+};
+
+static struct nl_desc nl_desc;
+
+static int nla_desc_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_CMD_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NLA_DESC_CMD_ID:
+	case NLA_DESC_CMD_OBJ:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_cmd(const struct nlattr *nest, struct nl_desc_cmd *cmd)
+{
+	struct nlattr *tb[NLA_DESC_CMD_MAX + 1] = {};
+
+	mnl_attr_parse_nested(nest, nla_desc_attr_cb, tb);
+	if (tb[NLA_DESC_CMD_ID])
+		cmd->id = mnl_attr_get_u32(tb[NLA_DESC_CMD_ID]);
+	if (tb[NLA_DESC_CMD_OBJ])
+		cmd->obj_id = mnl_attr_get_u32(tb[NLA_DESC_CMD_OBJ]);
+}
+
+static void print_desc_cmds(const struct nlattr *nest, struct nl_desc_cmd *cmds)
+{
+	struct nlattr *pos;
+	int j = 1;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_cmd(pos, &cmds[j++]);
+}
+
+static int nla_desc_cmds_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_NUM_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int data_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[NLA_DESC_CMDS_MAX + 1] = {};
+
+	mnl_attr_parse(nlh, 0, nla_desc_cmds_cb, tb);
+	if (tb[NLA_DESC_CMDS_NUM]) {
+		nl_desc.num_cmds = mnl_attr_get_u32(tb[NLA_DESC_CMDS_NUM]);
+
+		nl_desc.cmds = calloc(nl_desc.num_cmds + 1, sizeof(struct nl_desc_cmd));
+		if (!nl_desc.cmds)
+			return MNL_CB_ERROR;
+	}
+
+	if (tb[NLA_DESC_CMDS])
+		print_desc_cmds(tb[NLA_DESC_CMDS], nl_desc.cmds);
+
+	return MNL_CB_OK;
+}
+
+#define NETLINK_DESC	23
+#define NLDESC_GET_CMDS	16
+
+int main(void)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	struct mnl_socket *nl;
+	struct nlmsghdr *nlh;
+	uint32_t seq, portid;
+	struct nlattr *nest;
+	int ret, i;
+
+	nl = mnl_socket_open(NETLINK_DESC);
+	if (nl == NULL) {
+		perror("mnl_socket_open");
+		exit(EXIT_FAILURE);
+	}
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		perror("mnl_socket_bind");
+		exit(EXIT_FAILURE);
+	}
+
+	nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = NLDESC_GET_CMDS;
+	nlh->nlmsg_flags = NLM_F_REQUEST|NLM_F_DUMP;
+	nlh->nlmsg_seq = seq = time(NULL);
+
+	mnl_attr_put_u32(nlh, NLA_DESC_REQ_BUS, NETLINK_NETFILTER);
+	nest = mnl_attr_nest_start(nlh, NLA_DESC_REQ_DATA);
+	mnl_attr_put_u32(nlh, NFNL_DESC_REQ_SUBSYS, NFNL_SUBSYS_NFTABLES);
+	mnl_attr_nest_end(nlh, nest);
+
+	ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+	if (ret == -1) {
+		perror("mnl_socket_sendto");
+		exit(EXIT_FAILURE);
+	}
+	portid = mnl_socket_get_portid(nl);
+
+	while (1) {
+		ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("mnl_socket_recvfrom");
+			exit(EXIT_FAILURE);
+		}
+		ret = mnl_cb_run(buf, ret, seq, portid, data_cb, &nl_desc);
+		if (ret == -1) {
+			perror("mnl_cb_run");
+			exit(EXIT_FAILURE);
+		} else if (ret <= MNL_CB_STOP)
+                        break;
+	}
+
+	mnl_socket_close(nl);
+
+	for (i = 1; nl_desc.cmds[i].obj_id; i++) {
+		printf("cmd = %d\n", nl_desc.cmds[i].id);
+		printf("obj_id = %d\n", nl_desc.cmds[i].obj_id);
+	}
+
+	return 0;
+}
diff --git a/examples/nft-dump-desc-objs.c b/examples/nft-dump-desc-objs.c
new file mode 100644
index 000000000000..8f5b365e3c64
--- /dev/null
+++ b/examples/nft-dump-desc-objs.c
@@ -0,0 +1,263 @@
+#include <endian.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <time.h>
+#include <arpa/inet.h>
+#include <inttypes.h>
+
+#include <linux/netlink.h>
+#include <linux/netfilter/nfnetlink.h>
+
+#include <libmnl/libmnl.h>
+
+struct n_desc_obj;
+struct n_desc_attr;
+
+struct nl_desc {
+	uint32_t			num_objs;
+	struct nl_desc_obj		*objs;
+};
+
+struct nl_desc_obj {
+	uint16_t			id;
+	uint16_t			max;
+	struct nl_desc_attr		*attrs;
+};
+
+struct nl_desc_attr {
+	uint16_t			nest_id;
+	uint16_t			num;
+	uint16_t			type;
+	uint16_t			len;
+	uint32_t			max;
+};
+
+static struct nl_desc nl_desc;
+
+static int nla_desc_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_ATTR_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NLA_DESC_ATTR_NUM:
+	case NLA_DESC_ATTR_TYPE:
+	case NLA_DESC_ATTR_LEN:
+	case NLA_DESC_ATTR_MAXVAL:
+	case NLA_DESC_ATTR_NEST_ID:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_attr(const struct nlattr *nest, struct nl_desc_attr *attr)
+{
+	struct nlattr *tb[NLA_DESC_ATTR_MAX + 1] = {};
+
+	mnl_attr_parse_nested(nest, nla_desc_attr_cb, tb);
+	if (tb[NLA_DESC_ATTR_NUM])
+		attr->num = mnl_attr_get_u32(tb[NLA_DESC_ATTR_NUM]);
+	if (tb[NLA_DESC_ATTR_TYPE])
+		attr->type = mnl_attr_get_u32(tb[NLA_DESC_ATTR_TYPE]);
+	if (tb[NLA_DESC_ATTR_LEN])
+		attr->len = mnl_attr_get_u32(tb[NLA_DESC_ATTR_LEN]);
+	if (tb[NLA_DESC_ATTR_MAXVAL])
+		attr->max = mnl_attr_get_u32(tb[NLA_DESC_ATTR_MAXVAL]);
+	if (tb[NLA_DESC_ATTR_NEST_ID])
+		attr->nest_id = mnl_attr_get_u32(tb[NLA_DESC_ATTR_NEST_ID]);
+}
+
+static void print_desc_attrs(const struct nlattr *nest, struct nl_desc_attr *attrs)
+{
+	struct nlattr *pos;
+	int j = 1;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_attr(pos, &attrs[j++]);
+}
+
+static int nla_desc_obj_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_OBJ_ID:
+	case NLA_DESC_OBJ_ATTRS_MAX:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJ_ATTRS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_obj(const struct nlattr *nest)
+{
+	struct nlattr *tb[NLA_DESC_OBJ_MAX + 1] = {};
+	uint32_t id = 0, attrs_max;
+
+	mnl_attr_parse_nested(nest, nla_desc_obj_attr_cb, tb);
+	if (tb[NLA_DESC_OBJ_ID])
+		id = mnl_attr_get_u32(tb[NLA_DESC_OBJ_ID]);
+	if (tb[NLA_DESC_OBJ_ATTRS_MAX]) {
+		attrs_max = mnl_attr_get_u32(tb[NLA_DESC_OBJ_ATTRS_MAX]);
+
+		nl_desc.objs[id].attrs = calloc(attrs_max + 1, sizeof(struct nl_desc_attr));
+		if (!nl_desc.objs[id].attrs)
+			return;
+
+		nl_desc.objs[id].max = attrs_max;
+	}
+	if (tb[NLA_DESC_OBJ_ATTRS])
+		print_desc_attrs(tb[NLA_DESC_OBJ_ATTRS], nl_desc.objs[id].attrs);
+}
+
+static void print_desc_objs(const struct nlattr *nest)
+{
+	struct nlattr *pos;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_obj(pos);
+}
+
+static int nla_desc_objs_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_NUM_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int data_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[NLA_DESC_MAX + 1] = {};
+
+	mnl_attr_parse(nlh, 0, nla_desc_objs_cb, tb);
+	if (tb[NLA_DESC_NUM_OBJS]) {
+		nl_desc.num_objs = mnl_attr_get_u32(tb[NLA_DESC_NUM_OBJS]);
+
+		nl_desc.objs = calloc(nl_desc.num_objs + 1, sizeof(struct nl_desc_obj));
+		if (!nl_desc.objs)
+			return MNL_CB_ERROR;
+	}
+
+	if (tb[NLA_DESC_OBJS])
+		print_desc_objs(tb[NLA_DESC_OBJS]);
+
+	return MNL_CB_OK;
+}
+
+#define NETLINK_DESC	23
+#define NLDESC_GET_OBJS	18
+
+int main(void)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	struct mnl_socket *nl;
+	struct nlmsghdr *nlh;
+	uint32_t seq, portid;
+	struct nlattr *nest;
+	int ret, i, j;
+
+	nl = mnl_socket_open(NETLINK_DESC);
+	if (nl == NULL) {
+		perror("mnl_socket_open");
+		exit(EXIT_FAILURE);
+	}
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		perror("mnl_socket_bind");
+		exit(EXIT_FAILURE);
+	}
+
+	nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = NLDESC_GET_OBJS;
+	nlh->nlmsg_flags = NLM_F_REQUEST|NLM_F_DUMP;
+	nlh->nlmsg_seq = seq = time(NULL);
+
+	mnl_attr_put_u32(nlh, NLA_DESC_REQ_BUS, NETLINK_NETFILTER);
+	nest = mnl_attr_nest_start(nlh, NLA_DESC_REQ_DATA);
+	mnl_attr_put_u32(nlh, NFNL_DESC_REQ_SUBSYS, NFNL_SUBSYS_NFTABLES);
+	mnl_attr_nest_end(nlh, nest);
+
+	ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+	if (ret == -1) {
+		perror("mnl_socket_sendto");
+		exit(EXIT_FAILURE);
+	}
+	portid = mnl_socket_get_portid(nl);
+
+	while (1) {
+		ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("mnl_socket_recvfrom");
+			exit(EXIT_FAILURE);
+		}
+		ret = mnl_cb_run(buf, ret, seq, portid, data_cb, &nl_desc);
+		if (ret == -1) {
+			perror("mnl_cb_run");
+			exit(EXIT_FAILURE);
+		} else if (ret <= MNL_CB_STOP)
+                        break;
+	}
+
+	mnl_socket_close(nl);
+
+	for (i = 1; i <= nl_desc.num_objs; i++) {
+		printf("id = %d\n", i);
+		printf("attrs_max = %d\n", nl_desc.objs[i].max);
+		for (j = 1; j < nl_desc.objs[i].max + 1; j++) {
+			printf("\t---------\n");
+			printf("\tnum = %d\n", nl_desc.objs[i].attrs[j].num);
+			printf("\ttype = %d\n", nl_desc.objs[i].attrs[j].type);
+			if (nl_desc.objs[i].attrs[j].nest_id)
+				printf("\tnest_id = %d\n", nl_desc.objs[i].attrs[j].nest_id);
+			else {
+				printf("\tlen = %d\n", nl_desc.objs[i].attrs[j].len);
+				if (nl_desc.objs[i].attrs[j].max)
+					printf("\tmax = %d\n", nl_desc.objs[i].attrs[j].max);
+			}
+		}
+	}
+
+	return 0;
+}
-- 
2.11.0

Re: [PATCH RFC 2/4] netlink: add generic object description infrastructure

[Johannes Berg]

On Wed, 2018-02-07 at 02:37 +0100, Pablo Neira Ayuso wrote:
> +++ b/include/net/net_namespace.h
> @@ -78,6 +78,7 @@ struct net {
>  
>  	struct sock 		*rtnl;			/* rtnetlink socket */
>  	struct sock		*genl_sock;
> +	struct sock		*nl_desc_sock;

Using genl would save that =)

> +enum {
> +	NLDESC_GET_CMDS		= 16,
> +	NLDESC_NEW_CMDS,

I would say all of this new API should be in a new header file.

> +enum nft_nldesc_req_attributes {

nft_ prefix also doesn't seem appropriate.

> +static int nl_desc_handle_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
> +			      struct netlink_ext_ack *extack)

From here on it's also mostly boilerplate code that using genl handles
:-)

johannes

[PATCH RFC 3/4] netfilter: nfnetlink: add support for netlink descriptions

[Pablo Neira Ayuso]

NETLINK_NETFILTER is shared by several netfilter subsystems, add new
infrastructure to allow subsystems to register their own descriptions.
Hence, nfnetlink routes description requests to the corresponding
subsystem backend.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/nfnetlink.h      |   9 +++
 include/net/nldesc.h                     |   3 +
 include/uapi/linux/netfilter/nfnetlink.h |   7 ++
 net/netfilter/nfnetlink.c                | 108 +++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 495ba4dd9da5..87b3d9860444 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -37,6 +37,15 @@ struct nfnetlink_subsystem {
 int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n);
 int nfnetlink_subsys_unregister(const struct nfnetlink_subsystem *n);
 
+struct nfnl_desc_subsys {
+	u16				id;
+	const struct nl_desc_cmds	*cmds;
+	const struct nl_desc_objs	*objs;
+};
+
+int nfnl_desc_register_subsys(const struct nfnl_desc_subsys *subsys);
+void nfnl_desc_unregister_subsys(const struct nfnl_desc_subsys *subsys);
+
 int nfnetlink_has_listeners(struct net *net, unsigned int group);
 int nfnetlink_send(struct sk_buff *skb, struct net *net, u32 portid,
 		   unsigned int group, int echo, gfp_t flags);
diff --git a/include/net/nldesc.h b/include/net/nldesc.h
index 19306a648f10..0d232846005a 100644
--- a/include/net/nldesc.h
+++ b/include/net/nldesc.h
@@ -19,6 +19,9 @@ struct nl_desc_objs {
 
 struct nl_desc_req {
 	u32				bus;
+	union {
+		u32			nf_subsys_id;
+	};
 };
 
 struct net;
diff --git a/include/uapi/linux/netfilter/nfnetlink.h b/include/uapi/linux/netfilter/nfnetlink.h
index 5bc960f220b3..7dacf264e0b5 100644
--- a/include/uapi/linux/netfilter/nfnetlink.h
+++ b/include/uapi/linux/netfilter/nfnetlink.h
@@ -62,6 +62,13 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_NFT_COMPAT		11
 #define NFNL_SUBSYS_COUNT		12
 
+enum nfnl_desc_attr {
+	NFNL_DESC_REQ_UNSPEC,
+	NFNL_DESC_REQ_SUBSYS,
+	__NFNL_DESC_REQ_MAX
+};
+#define NFNL_DESC_REQ_MAX	(__NFNL_DESC_REQ_MAX - 1)
+
 /* Reserved control nfnetlink messages */
 #define NFNL_MSG_BATCH_BEGIN		NLMSG_MIN_TYPE
 #define NFNL_MSG_BATCH_END		NLMSG_MIN_TYPE+1
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 03ead8a9e90c..df5792534935 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -27,6 +27,7 @@
 #include <linux/init.h>
 
 #include <net/netlink.h>
+#include <net/nldesc.h>
 #include <linux/netfilter/nfnetlink.h>
 
 MODULE_LICENSE("GPL");
@@ -40,6 +41,7 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_NETFILTER);
 static struct {
 	struct mutex				mutex;
 	const struct nfnetlink_subsystem __rcu	*subsys;
+	const struct nfnl_desc_subsys __rcu	*desc;
 } table[NFNL_SUBSYS_COUNT];
 
 static const int nfnl_group2type[NFNLGRP_MAX+1] = {
@@ -513,6 +515,107 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 		netlink_rcv_skb(skb, nfnetlink_rcv_msg);
 }
 
+int nfnl_desc_register_subsys(const struct nfnl_desc_subsys *subsys)
+{
+	if (subsys->id >= NFNL_SUBSYS_COUNT)
+		return -ENOENT;
+
+	nfnl_lock(subsys->id);
+	rcu_assign_pointer(table[subsys->id].desc, subsys);
+	nfnl_unlock(subsys->id);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nfnl_desc_register_subsys);
+
+void nfnl_desc_unregister_subsys(const struct nfnl_desc_subsys *subsys)
+{
+	nfnl_lock(subsys->id);
+	rcu_assign_pointer(table[subsys->id].desc, NULL);
+	nfnl_unlock(subsys->id);
+
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(nfnl_desc_unregister_subsys);
+
+static const struct nfnl_desc_subsys *nfnl_desc_get(struct sk_buff *skb,
+						    struct nlmsghdr *nlh,
+						    struct nl_desc_req *req)
+{
+	const struct nfnl_desc_subsys *desc;
+
+	if (req->nf_subsys_id >= NFNL_SUBSYS_COUNT)
+		return ERR_PTR(-ENOENT);
+
+	desc = rcu_dereference(table[req->nf_subsys_id].desc);
+	if (!desc) {
+		rcu_read_unlock();
+		request_module("nfnetlink-subsys-%d", req->nf_subsys_id);
+		rcu_read_lock();
+		desc = rcu_dereference(table[req->nf_subsys_id].desc);
+		if (desc)
+			return ERR_PTR(-EAGAIN);
+	}
+	return desc;
+}
+
+static const struct nl_desc_cmds *nfnl_desc_getcmds(struct sk_buff *skb,
+						    struct nlmsghdr *nlh,
+						    struct nl_desc_req *req)
+{
+	const struct nfnl_desc_subsys *desc;
+
+	desc = nfnl_desc_get(skb, nlh, req);
+	if (IS_ERR(desc))
+		return (struct nl_desc_cmds *)desc;
+
+	return desc->cmds;
+}
+
+static const struct nl_desc_objs *nfnl_desc_getobjs(struct sk_buff *skb,
+						    struct nlmsghdr *nlh,
+						    struct nl_desc_req *req)
+{
+	const struct nfnl_desc_subsys *desc;
+
+	desc = nfnl_desc_get(skb, nlh, req);
+	if (IS_ERR(desc))
+		return (struct nl_desc_objs *)desc;
+
+	return desc->objs;
+}
+
+static const struct nla_policy nfnl_desc_req_policy[NFNL_DESC_REQ_MAX + 1] = {
+	[NFNL_DESC_REQ_SUBSYS]	= { .type = NLA_U32 },
+};
+
+static int nfnl_desc_parse(struct net *net, struct sk_buff *skb,
+			   struct nlmsghdr *nlh, const struct nlattr *attr,
+			   struct nl_desc_req *req)
+{
+	struct nlattr *tb[NFNL_DESC_REQ_MAX + 1];
+	int err;
+
+	err = nla_parse_nested(tb, NFNL_DESC_REQ_MAX, attr,
+			       nfnl_desc_req_policy, NULL);
+	if (err < 0)
+		return err;
+
+	if (!tb[NFNL_DESC_REQ_SUBSYS])
+		return -EINVAL;
+
+	req->nf_subsys_id = nla_get_u32(tb[NFNL_DESC_REQ_SUBSYS]);
+
+	return 0;
+}
+
+static struct nl_desc_subsys nfnl_subsys = {
+	.bus		= NETLINK_NETFILTER,
+	.getcmds	= nfnl_desc_getcmds,
+	.getobjs	= nfnl_desc_getobjs,
+	.parse		= nfnl_desc_parse,
+};
+
 #ifdef CONFIG_MODULES
 static int nfnetlink_bind(struct net *net, int group)
 {
@@ -549,6 +652,8 @@ static int __net_init nfnetlink_net_init(struct net *net)
 		return -ENOMEM;
 	net->nfnl_stash = nfnl;
 	rcu_assign_pointer(net->nfnl, nfnl);
+	nl_desc_register_subsys(&nfnl_subsys);
+
 	return 0;
 }
 
@@ -556,6 +661,7 @@ static void __net_exit nfnetlink_net_exit_batch(struct list_head *net_exit_list)
 {
 	struct net *net;
 
+	nl_desc_unregister_subsys(&nfnl_subsys);
 	list_for_each_entry(net, net_exit_list, exit_list)
 		RCU_INIT_POINTER(net->nfnl, NULL);
 	synchronize_net();
@@ -587,3 +693,5 @@ static void __exit nfnetlink_exit(void)
 }
 module_init(nfnetlink_init);
 module_exit(nfnetlink_exit);
+
+MODULE_ALIAS_NLDESC(NETLINK_NETFILTER);
-- 
2.11.0

[PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

This patch adds the netlink description for nf_tables.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h             |   2 +
 include/uapi/linux/netfilter/nf_tables_desc.h |  57 ++++
 net/netfilter/Makefile                        |   7 +-
 net/netfilter/nf_tables_api.c                 |   2 +
 net/netfilter/nf_tables_desc.c                | 471 ++++++++++++++++++++++++++
 5 files changed, 536 insertions(+), 3 deletions(-)
 create mode 100644 include/uapi/linux/netfilter/nf_tables_desc.h
 create mode 100644 net/netfilter/nf_tables_desc.c

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 663b015dace5..91b52b365f7e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1345,4 +1345,6 @@ struct nft_trans_flowtable {
 #define nft_trans_flowtable(trans)	\
 	(((struct nft_trans_flowtable *)trans->data)->flowtable)
 
+extern const struct nfnl_desc_subsys nft_nldesc;
+
 #endif /* _NET_NF_TABLES_H */
diff --git a/include/uapi/linux/netfilter/nf_tables_desc.h b/include/uapi/linux/netfilter/nf_tables_desc.h
new file mode 100644
index 000000000000..e596ad9f78c3
--- /dev/null
+++ b/include/uapi/linux/netfilter/nf_tables_desc.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _LINUX_NF_TABLES_DESC_H
+#define _LINUX_NF_TABLES_DESC_H
+
+enum nft_nldesc_obj {
+	NFT_UNSPEC,
+	NFT_TABLE,
+	NFT_CHAIN,
+	NFT_CHAIN_COUNTER,
+	NFT_CHAIN_HOOK,
+	NFT_CHAIN_DEV,
+	NFT_RULE,
+	NFT_RULE_COMPAT,
+	NFT_SET,
+	NFT_SET_DESC,
+	NFT_SET_ELEM,
+	NFT_OBJ,
+	NFT_OBJ_COUNTER,
+	NFT_OBJ_QUOTA,
+	NFT_OBJ_LIMIT,
+	NFT_FLOWTABLE,
+	NFT_DATA,
+	NFT_EXPR,
+	NFT_EXPR_COUNTER,
+	NFT_EXPR_IMMEDIATE,
+	NFT_EXPR_BITWISE,
+	NFT_EXPR_BYTEORDER,
+	NFT_EXPR_CMP,
+	NFT_EXPR_RANGE,
+	NFT_EXPR_LOOKUP,
+	NFT_EXPR_DYNSET,
+	NFT_EXPR_PAYLOAD,
+	NFT_EXPR_EXTHDR,
+	NFT_EXPR_META,
+	NFT_EXPR_HASH,
+	NFT_EXPR_RT,
+	NFT_EXPR_CT,
+	NFT_EXPR_FLOW,
+	NFT_EXPR_LIMIT,
+	NFT_EXPR_LOG,
+	NFT_EXPR_QUEUE,
+	NFT_EXPR_QUOTA,
+	NFT_EXPR_REJECT,
+	NFT_EXPR_NAT,
+	NFT_EXPR_MASQ,
+	NFT_EXPR_REDIR,
+	NFT_EXPR_DUP,
+	NFT_EXPR_FWD,
+	NFT_EXPR_OBJREF,
+	NFT_EXPR_FIB,
+	NFT_EXPR_CT_HELPER,
+	NFT_EXPR_NUMGEN,
+	__NFT_MAX,
+};
+#define NFT_MAX	(__NFT_MAX - 1)
+
+#endif
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 5d9b8b959e58..38e048ea7e42 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -73,9 +73,10 @@ obj-$(CONFIG_NETFILTER_CONNCOUNT) += nf_conncount.o
 obj-$(CONFIG_NF_DUP_NETDEV)	+= nf_dup_netdev.o
 
 # nf_tables
-nf_tables-objs := nf_tables_core.o nf_tables_api.o nf_tables_trace.o \
-		  nft_immediate.o nft_cmp.o nft_range.o nft_bitwise.o \
-		  nft_byteorder.o nft_payload.o nft_lookup.o nft_dynset.o
+nf_tables-objs := nf_tables_core.o nf_tables_api.o nf_tables_desc.o \
+		  nf_tables_trace.o nft_immediate.o nft_cmp.o nft_range.o \
+		  nft_bitwise.o nft_byteorder.o nft_payload.o nft_lookup.o \
+		  nft_dynset.o
 
 obj-$(CONFIG_NF_TABLES)		+= nf_tables.o
 obj-$(CONFIG_NF_TABLES_INET)	+= nf_tables_inet.o
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0791813a1e7d..cb500aeaa729 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6601,6 +6601,7 @@ static int __init nf_tables_module_init(void)
 	if (err < 0)
 		goto err3;
 
+	nfnl_desc_register_subsys(&nft_nldesc);
 	register_netdevice_notifier(&nf_tables_flowtable_notifier);
 
 	return register_pernet_subsys(&nf_tables_net_ops);
@@ -6617,6 +6618,7 @@ static void __exit nf_tables_module_exit(void)
 	unregister_pernet_subsys(&nf_tables_net_ops);
 	nfnetlink_subsys_unregister(&nf_tables_subsys);
 	unregister_netdevice_notifier(&nf_tables_flowtable_notifier);
+	nfnl_desc_unregister_subsys(&nft_nldesc);
 	rcu_barrier();
 	nf_tables_core_module_exit();
 	kfree(info);
diff --git a/net/netfilter/nf_tables_desc.c b/net/netfilter/nf_tables_desc.c
new file mode 100644
index 000000000000..2acaff69edb0
--- /dev/null
+++ b/net/netfilter/nf_tables_desc.c
@@ -0,0 +1,471 @@
+#include <net/nldesc.h>
+#include <net/netlink.h>
+#include <linux/if.h>
+#include <uapi/linux/netfilter.h>
+#include <linux/netfilter/nfnetlink.h>
+#include <linux/netfilter/nf_tables.h>
+#include <linux/netfilter/nf_tables_desc.h>
+#include <linux/netfilter/nf_log.h>
+#include <uapi/linux/netfilter/nf_nat.h>
+#include <uapi/linux/netfilter/nf_conntrack_tuple_common.h>
+
+static const struct nl_desc_attr nft_nldesc_table_attrs[NFTA_TABLE_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_TABLE_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_TABLE_FLAGS, NFT_TABLE_F_DORMANT),
+	NLDESC_ATTR_U32(NFTA_TABLE_USE),
+	NLDESC_ATTR_U64(NFTA_TABLE_HANDLE),
+	NLDESC_ATTR_PAD(NFTA_TABLE_PAD),
+};
+
+static const struct nl_desc_attr nft_nldesc_chain_dev_attrs[NFTA_DEVICE_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_DEVICE_NAME, IFNAMSIZ),
+};
+
+static const struct nl_desc_obj nft_nldesc_chain_dev[] = {
+	NLDESC_OBJ(NFT_CHAIN_DEV, nft_nldesc_chain_dev_attrs, NFTA_DEVICE_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_chain_hook_attrs[NFTA_HOOK_MAX + 1] = {
+	NLDESC_ATTR_U32(NFTA_HOOK_HOOKNUM),
+	NLDESC_ATTR_U32(NFTA_HOOK_PRIORITY),
+	NLDESC_ATTR_NESTED(NFTA_HOOK_DEV, nft_nldesc_chain_dev),
+};
+
+static const struct nl_desc_obj nft_nldesc_chain_hook[] = {
+	NLDESC_OBJ(NFT_CHAIN_HOOK, nft_nldesc_chain_hook_attrs, NFTA_HOOK_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_counter_attrs[NFTA_COUNTER_MAX + 1] = {
+	NLDESC_ATTR_U64(NFTA_COUNTER_BYTES),
+	NLDESC_ATTR_U64(NFTA_COUNTER_PACKETS),
+	NLDESC_ATTR_PAD(NFTA_COUNTER_PAD),
+};
+
+static const struct nl_desc_obj nft_nldesc_counters[] = {
+	NLDESC_OBJ(NFT_CHAIN_COUNTER, nft_nldesc_counter_attrs, NFTA_COUNTER_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_chain_attrs[NFTA_CHAIN_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_CHAIN_TABLE, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U64(NFTA_CHAIN_HANDLE),
+	NLDESC_ATTR_STRING(NFTA_CHAIN_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_NESTED(NFTA_CHAIN_HOOK, nft_nldesc_chain_hook),
+	NLDESC_ATTR_U32_MAX(NFTA_CHAIN_POLICY, NF_ACCEPT),
+	NLDESC_ATTR_U32(NFTA_CHAIN_USE),
+	NLDESC_ATTR_NUL_STRING(NFTA_CHAIN_TYPE),
+	NLDESC_ATTR_NESTED(NFTA_CHAIN_COUNTERS, nft_nldesc_counters),
+	NLDESC_ATTR_PAD(NFTA_CHAIN_PAD),
+};
+
+static const struct nl_desc_attr nft_nldesc_data_attrs[NFTA_DATA_MAX + 1] = {
+	NLDESC_ATTR_U32(NFTA_SET_DESC_SIZE),
+};
+
+static const struct nl_desc_obj nft_nldesc_data[] = {
+	NLDESC_OBJ(NFT_DATA, nft_nldesc_data_attrs, NFTA_DATA_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_immediate_attrs[NFTA_IMMEDIATE_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_IMMEDIATE_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_NESTED(NFTA_IMMEDIATE_DATA, nft_nldesc_data),
+};
+
+static const struct nl_desc_attr nft_nldesc_bitwise_attrs[NFTA_BITWISE_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_BITWISE_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_BITWISE_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_BITWISE_LEN, U8_MAX),
+	NLDESC_ATTR_NESTED(NFTA_BITWISE_MASK, nft_nldesc_data),
+	NLDESC_ATTR_NESTED(NFTA_BITWISE_XOR, nft_nldesc_data),
+};
+
+static const struct nl_desc_attr nft_nldesc_byteorder_attrs[NFTA_BYTEORDER_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_BYTEORDER_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_BYTEORDER_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_BYTEORDER_OP, NFT_BYTEORDER_HTON),
+	NLDESC_ATTR_U32_MAX(NFTA_BYTEORDER_LEN, U8_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_BYTEORDER_SIZE, U8_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_cmp_attrs[NFTA_CMP_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_CMP_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_CMP_OP, NFT_CMP_GTE),
+	NLDESC_ATTR_NESTED(NFTA_CMP_DATA, nft_nldesc_data),
+};
+
+static const struct nl_desc_attr nft_nldesc_range_attrs[NFTA_RANGE_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_RANGE_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_RANGE_OP, NFT_RANGE_NEQ),
+	NLDESC_ATTR_NESTED(NFTA_RANGE_FROM_DATA, nft_nldesc_data),
+	NLDESC_ATTR_NESTED(NFTA_RANGE_TO_DATA, nft_nldesc_data),
+};
+
+static const struct nl_desc_attr nft_nldesc_lookup_attrs[NFTA_LOOKUP_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_LOOKUP_SET, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_LOOKUP_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_LOOKUP_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32(NFTA_LOOKUP_SET_ID),
+	NLDESC_ATTR_U32_MAX(NFTA_LOOKUP_FLAGS, NFT_LOOKUP_F_INV),
+};
+
+static const struct nl_desc_obj nft_nldesc_expressions[];
+
+static const struct nl_desc_attr nft_nldesc_dynset_attrs[NFTA_DYNSET_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_DYNSET_SET_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32(NFTA_DYNSET_SET_ID),
+	NLDESC_ATTR_U32_MAX(NFTA_DYNSET_OP, NFT_DYNSET_OP_UPDATE),
+	NLDESC_ATTR_U32(NFTA_DYNSET_SREG_KEY),
+	NLDESC_ATTR_U32(NFTA_DYNSET_SREG_DATA),
+	NLDESC_ATTR_U64(NFTA_DYNSET_TIMEOUT),
+	NLDESC_ATTR_NESTED(NFTA_DYNSET_EXPR, nft_nldesc_expressions),
+	NLDESC_ATTR_PAD(NFTA_DYNSET_PAD),
+	NLDESC_ATTR_U32_MAX(NFTA_DYNSET_FLAGS, NFT_DYNSET_F_INV),
+};
+
+static const struct nl_desc_attr nft_nldesc_payload_attrs[NFTA_PAYLOAD_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_BASE, NFT_PAYLOAD_TRANSPORT_HEADER),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_OFFSET, U16_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_LEN, U8_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_CSUM_TYPE, NFT_PAYLOAD_CSUM_INET),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_CSUM_OFFSET, U16_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_PAYLOAD_CSUM_FLAGS, NFT_PAYLOAD_L4CSUM_PSEUDOHDR),
+};
+
+static const struct nl_desc_attr nft_nldesc_exthdr_attrs[NFTA_EXTHDR_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_EXTHDR_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U8(NFTA_EXTHDR_TYPE),
+	NLDESC_ATTR_U32_MAX(NFTA_EXTHDR_OFFSET, U8_MAX),
+	NLDESC_ATTR_U32(NFTA_EXTHDR_LEN),
+	NLDESC_ATTR_U32_MAX(NFTA_EXTHDR_FLAGS, NFT_EXTHDR_F_PRESENT),
+	NLDESC_ATTR_U32_MAX(NFTA_EXTHDR_OP, NFT_EXTHDR_OP_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_EXTHDR_SREG, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_meta_attrs[NFTA_META_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_META_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_META_KEY, NFT_META_SECPATH),
+	NLDESC_ATTR_U32_MAX(NFTA_META_SREG, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_hash_attrs[NFTA_HASH_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_HASH_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_HASH_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_HASH_LEN, U8_MAX),
+	NLDESC_ATTR_U32(NFTA_HASH_MODULUS),
+	NLDESC_ATTR_U32(NFTA_HASH_SEED),
+	NLDESC_ATTR_U32(NFTA_HASH_OFFSET),
+	NLDESC_ATTR_U32_MAX(NFTA_HASH_TYPE, NFT_HASH_SYM),
+};
+
+static const struct nl_desc_attr nft_nldesc_rt_attrs[NFTA_RT_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_RT_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_RT_KEY, NFT_RT_TCPMSS),
+};
+
+static const struct nl_desc_attr nft_nldesc_ct_attrs[NFTA_CT_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_CT_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_CT_KEY, NFT_CT_EVENTMASK),
+	NLDESC_ATTR_U32_MAX(NFTA_CT_DIRECTION, IP_CT_DIR_REPLY),
+	NLDESC_ATTR_U32_MAX(NFTA_CT_SREG, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_flow_attrs[NFTA_FLOW_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_FLOW_TABLE_NAME, NFT_NAME_MAXLEN - 1),
+};
+
+static const struct nl_desc_attr nft_nldesc_limit_attrs[NFTA_LIMIT_MAX + 1] = {
+	NLDESC_ATTR_U64(NFTA_LIMIT_RATE),
+	NLDESC_ATTR_U64(NFTA_LIMIT_UNIT),
+	NLDESC_ATTR_U32(NFTA_LIMIT_BURST),
+	NLDESC_ATTR_U32_MAX(NFTA_LIMIT_TYPE, NFT_LIMIT_PKT_BYTES),
+	NLDESC_ATTR_U32_MAX(NFTA_LIMIT_FLAGS, NFT_LIMIT_F_INV),
+	NLDESC_ATTR_PAD(NFTA_LIMIT_PAD),
+};
+
+static const struct nl_desc_attr nft_nldesc_log_attrs[NFTA_LOG_MAX + 1] = {
+	NLDESC_ATTR_U16(NFTA_LOG_GROUP),
+	NLDESC_ATTR_STRING(NFTA_LOG_PREFIX, NF_LOG_PREFIXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_LOG_SNAPLEN, U16_MAX),
+	NLDESC_ATTR_U16(NFTA_LOG_QTHRESHOLD),
+	NLDESC_ATTR_U32_MAX(NFTA_LOG_LEVEL, LOGLEVEL_DEBUG),
+	NLDESC_ATTR_U32_MAX(NFTA_LOG_FLAGS, NF_LOG_MASK),
+};
+
+static const struct nl_desc_attr nft_nldesc_queue_attrs[NFTA_QUEUE_MAX + 1] = {
+	NLDESC_ATTR_U16(NFTA_QUEUE_NUM),
+	NLDESC_ATTR_U16(NFTA_QUEUE_TOTAL),
+	NLDESC_ATTR_U16(NFTA_QUEUE_FLAGS),
+	NLDESC_ATTR_U32_MAX(NFTA_QUEUE_SREG_QNUM, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_quota_attrs[NFTA_QUOTA_MAX + 1] = {
+	NLDESC_ATTR_U64(NFTA_QUOTA_BYTES),
+	NLDESC_ATTR_U32_MAX(NFTA_QUOTA_FLAGS, NFT_QUOTA_F_DEPLETED),
+	NLDESC_ATTR_U64(NFTA_QUOTA_CONSUMED),
+	NLDESC_ATTR_PAD(NFTA_QUOTA_PAD),
+};
+
+static const struct nl_desc_attr nft_nldesc_reject_attrs[NFTA_REJECT_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_REJECT_TYPE, NFT_REJECT_ICMPX_UNREACH),
+	NLDESC_ATTR_U8(NFTA_REJECT_ICMP_CODE),
+};
+
+static const struct nl_desc_attr nft_nldesc_nat_attrs[NFTA_NAT_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_TYPE, NFT_NAT_DNAT),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_FAMILY, NFPROTO_NUMPROTO),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_REG_ADDR_MIN, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_REG_ADDR_MAX, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_REG_PROTO_MIN, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_REG_PROTO_MAX, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_NAT_FLAGS, NF_NAT_RANGE_MASK),
+};
+
+static const struct nl_desc_attr nft_nldesc_masq_attrs[NFTA_MASQ_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_MASQ_FLAGS, NF_NAT_RANGE_MASK),
+	NLDESC_ATTR_U32_MAX(NFTA_MASQ_REG_PROTO_MIN, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_MASQ_REG_PROTO_MAX, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_redir_attrs[NFTA_REDIR_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_REDIR_REG_PROTO_MIN, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_REDIR_REG_PROTO_MAX, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_REDIR_FLAGS, NF_NAT_RANGE_MASK),
+};
+
+static const struct nl_desc_attr nft_nldesc_dup_attrs[NFTA_DUP_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_DUP_SREG_ADDR, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_DUP_SREG_DEV, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_fwd_attrs[NFTA_FWD_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_FWD_SREG_DEV, NFT_REG_MAX),
+};
+
+static const struct nl_desc_attr nft_nldesc_objref_attrs[NFTA_OBJREF_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_OBJREF_IMM_TYPE, NFT_OBJECT_MAX),
+	NLDESC_ATTR_STRING(NFTA_OBJREF_IMM_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_OBJREF_SET_SREG, NFT_REG_MAX),
+	NLDESC_ATTR_STRING(NFTA_OBJREF_SET_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32(NFTA_OBJREF_SET_ID),
+};
+
+static const struct nl_desc_attr nft_nldesc_fib_attrs[NFTA_FIB_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_FIB_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_FIB_RESULT, NFT_FIB_RESULT_MAX),
+	NLDESC_ATTR_U32_MAX(NFTA_FIB_FLAGS, (NFTA_FIB_F_PRESENT << 1) - 1),
+};
+
+static const struct nl_desc_attr nft_nldesc_ct_helper_attrs[NFTA_CT_HELPER_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_CT_HELPER_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U16(NFTA_CT_HELPER_L3PROTO),
+	NLDESC_ATTR_U8(NFTA_CT_HELPER_L4PROTO),
+};
+
+static const struct nl_desc_attr nft_nldesc_numgen_attrs[NFTA_NG_MAX + 1] = {
+	NLDESC_ATTR_U32_MAX(NFTA_NG_DREG, NFT_REG_MAX),
+	NLDESC_ATTR_U32(NFTA_NG_MODULUS),
+	NLDESC_ATTR_U32_MAX(NFTA_NG_TYPE, NFT_NG_MAX),
+	NLDESC_ATTR_U32(NFTA_NG_OFFSET),
+};
+
+static const struct nl_desc_obj nft_nldesc_expr_data[] = {
+	NLDESC_OBJ(NFT_EXPR_IMMEDIATE, nft_nldesc_immediate_attrs, NFTA_IMMEDIATE_MAX),
+	NLDESC_OBJ(NFT_EXPR_BITWISE, nft_nldesc_bitwise_attrs, NFTA_BITWISE_MAX),
+	NLDESC_OBJ(NFT_EXPR_BYTEORDER, nft_nldesc_byteorder_attrs, NFTA_BYTEORDER_MAX),
+	NLDESC_OBJ(NFT_EXPR_CMP, nft_nldesc_cmp_attrs, NFTA_CMP_MAX),
+	NLDESC_OBJ(NFT_EXPR_RANGE, nft_nldesc_range_attrs, NFTA_RANGE_MAX),
+	NLDESC_OBJ(NFT_EXPR_LOOKUP, nft_nldesc_lookup_attrs, NFTA_LOOKUP_MAX),
+	NLDESC_OBJ(NFT_EXPR_DYNSET, nft_nldesc_dynset_attrs, NFTA_DYNSET_MAX),
+	NLDESC_OBJ(NFT_EXPR_PAYLOAD, nft_nldesc_payload_attrs, NFTA_PAYLOAD_MAX),
+	NLDESC_OBJ(NFT_EXPR_EXTHDR, nft_nldesc_exthdr_attrs, NFTA_EXTHDR_MAX),
+	NLDESC_OBJ(NFT_EXPR_META, nft_nldesc_meta_attrs, NFTA_META_MAX),
+	NLDESC_OBJ(NFT_EXPR_HASH, nft_nldesc_hash_attrs, NFTA_HASH_MAX),
+	NLDESC_OBJ(NFT_EXPR_RT, nft_nldesc_rt_attrs, NFTA_RT_MAX),
+	NLDESC_OBJ(NFT_EXPR_CT, nft_nldesc_ct_attrs, NFTA_CT_MAX),
+	NLDESC_OBJ(NFT_EXPR_FLOW, nft_nldesc_flow_attrs, NFTA_FLOW_MAX),
+	NLDESC_OBJ(NFT_EXPR_LIMIT, nft_nldesc_limit_attrs, NFTA_LIMIT_MAX),
+	NLDESC_OBJ(NFT_EXPR_COUNTER, nft_nldesc_counter_attrs, NFTA_COUNTER_MAX),
+	NLDESC_OBJ(NFT_EXPR_LOG, nft_nldesc_log_attrs, NFTA_LOG_MAX),
+	NLDESC_OBJ(NFT_EXPR_QUEUE, nft_nldesc_queue_attrs, NFTA_QUEUE_MAX),
+	NLDESC_OBJ(NFT_EXPR_QUOTA, nft_nldesc_quota_attrs, NFTA_QUOTA_MAX),
+	NLDESC_OBJ(NFT_EXPR_REJECT, nft_nldesc_reject_attrs, NFTA_REJECT_MAX),
+	NLDESC_OBJ(NFT_EXPR_NAT, nft_nldesc_nat_attrs, NFTA_NAT_MAX),
+	NLDESC_OBJ(NFT_EXPR_MASQ, nft_nldesc_masq_attrs, NFTA_MASQ_MAX),
+	NLDESC_OBJ(NFT_EXPR_REDIR, nft_nldesc_redir_attrs, NFTA_REDIR_MAX),
+	NLDESC_OBJ(NFT_EXPR_DUP, nft_nldesc_dup_attrs, NFTA_DUP_MAX),
+	NLDESC_OBJ(NFT_EXPR_FWD, nft_nldesc_fwd_attrs, NFTA_FWD_MAX),
+	NLDESC_OBJ(NFT_EXPR_OBJREF, nft_nldesc_objref_attrs, NFTA_OBJREF_MAX),
+	NLDESC_OBJ(NFT_EXPR_FIB, nft_nldesc_fib_attrs, NFTA_FIB_MAX),
+	NLDESC_OBJ(NFT_EXPR_CT_HELPER, nft_nldesc_ct_helper_attrs, NFTA_CT_HELPER_MAX),
+	NLDESC_OBJ(NFT_EXPR_NUMGEN, nft_nldesc_numgen_attrs, NFTA_NG_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_expressions_attrs[NFTA_EXPR_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_EXPR_NAME, 0),
+	NLDESC_ATTR_NESTED(NFTA_EXPR_DATA, nft_nldesc_expr_data),
+};
+
+static const struct nl_desc_obj nft_nldesc_expressions[] = {
+	NLDESC_OBJ(NFT_EXPR, nft_nldesc_expressions_attrs, NFTA_EXPR_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_rule_compat_attrs[NFTA_RULE_COMPAT_MAX + 1] = {
+	NLDESC_ATTR_U32(NFTA_RULE_COMPAT_PROTO),
+	NLDESC_ATTR_U32(NFTA_RULE_COMPAT_FLAGS),
+};
+
+static const struct nl_desc_obj nft_nldesc_rule_compat[] = {
+	NLDESC_OBJ(NFT_RULE_COMPAT, nft_nldesc_rule_compat_attrs, NFTA_RULE_COMPAT_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_rule_attrs[NFTA_RULE_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_RULE_TABLE, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_STRING(NFTA_RULE_CHAIN, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U64(NFTA_RULE_HANDLE),
+	NLDESC_ATTR_NESTED(NFTA_RULE_EXPRESSIONS, nft_nldesc_expressions),
+	NLDESC_ATTR_NESTED(NFTA_RULE_COMPAT, nft_nldesc_rule_compat),
+	NLDESC_ATTR_U64(NFTA_RULE_POSITION),
+	NLDESC_ATTR_BINARY(NFTA_RULE_USERDATA, NFT_USERDATA_MAXLEN),
+	NLDESC_ATTR_U32(NFTA_RULE_ID),
+};
+
+static const struct nl_desc_attr nft_nldesc_set_desc_attrs[NFTA_SET_DESC_MAX + 1] = {
+	NLDESC_ATTR_U32(NFTA_SET_DESC_SIZE),
+};
+
+static const struct nl_desc_obj nft_nldesc_set_desc[] = {
+	NLDESC_OBJ(NFT_SET_DESC, nft_nldesc_set_desc_attrs, NFTA_SET_DESC_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_set_attrs[NFTA_SET_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_SET_TABLE, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_STRING(NFTA_SET_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_SET_FLAGS, NFT_SET_OBJECT),
+	NLDESC_ATTR_U32(NFTA_SET_KEY_TYPE),
+	NLDESC_ATTR_U32(NFTA_SET_KEY_LEN),
+	NLDESC_ATTR_U32(NFTA_SET_DATA_TYPE),
+	NLDESC_ATTR_U32(NFTA_SET_DATA_LEN),
+	NLDESC_ATTR_U32_MAX(NFTA_SET_POLICY, NFT_SET_POL_MEMORY),
+	NLDESC_ATTR_NESTED(NFTA_SET_DESC, nft_nldesc_set_desc),
+	NLDESC_ATTR_U32(NFTA_SET_ID),
+	NLDESC_ATTR_U64(NFTA_SET_TIMEOUT),
+	NLDESC_ATTR_U32(NFTA_SET_GC_INTERVAL),
+	NLDESC_ATTR_BINARY(NFTA_SET_USERDATA, NFT_USERDATA_MAXLEN),
+	NLDESC_ATTR_PAD(NFTA_SET_PAD),
+	NLDESC_ATTR_U32_MAX(NFTA_SET_OBJ_TYPE, NFT_OBJECT_MAX),
+	NLDESC_ATTR_U64(NFTA_SET_HANDLE),
+};
+
+static const struct nl_desc_attr nft_nldesc_set_elem_attrs[NFTA_SET_ELEM_MAX + 1] = {
+	NLDESC_ATTR_NESTED(NFTA_SET_ELEM_KEY, nft_nldesc_data),
+	NLDESC_ATTR_NESTED(NFTA_SET_ELEM_DATA, nft_nldesc_data),
+	NLDESC_ATTR_U32_MAX(NFTA_SET_ELEM_FLAGS, NFT_SET_ELEM_INTERVAL_END),
+	NLDESC_ATTR_U64(NFTA_SET_ELEM_TIMEOUT),
+	NLDESC_ATTR_U64(NFTA_SET_ELEM_EXPIRATION),
+	NLDESC_ATTR_BINARY(NFTA_SET_ELEM_USERDATA, NFT_USERDATA_MAXLEN),
+	NLDESC_ATTR_NESTED(NFTA_SET_ELEM_EXPR, nft_nldesc_expressions),
+	NLDESC_ATTR_STRING(NFTA_SET_ELEM_OBJREF, NFT_NAME_MAXLEN - 1),
+};
+
+static const struct nl_desc_obj nft_nldesc_obj_data[] = {
+	NLDESC_OBJ(NFT_OBJ_COUNTER, nft_nldesc_counter_attrs, NFTA_COUNTER_MAX),
+	NLDESC_OBJ(NFT_OBJ_QUOTA, nft_nldesc_quota_attrs, NFTA_QUOTA_MAX),
+	NLDESC_OBJ(NFT_OBJ_LIMIT, nft_nldesc_limit_attrs, NFTA_LIMIT_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_attr nft_nldesc_obj_attrs[NFTA_OBJ_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_OBJ_TABLE, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_STRING(NFTA_OBJ_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_OBJ_TYPE, NFT_OBJECT_MAX),
+	NLDESC_ATTR_NESTED(NFTA_OBJ_DATA, nft_nldesc_obj_data),
+	NLDESC_ATTR_U32(NFTA_OBJ_USE),
+	NLDESC_ATTR_U64(NFTA_OBJ_HANDLE),
+	NLDESC_ATTR_PAD(NFTA_OBJ_PAD),
+};
+
+static const struct nl_desc_attr nft_nldesc_flowtable_attrs[NFTA_FLOWTABLE_MAX + 1] = {
+	NLDESC_ATTR_STRING(NFTA_FLOWTABLE_TABLE, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_STRING(NFTA_FLOWTABLE_NAME, NFT_NAME_MAXLEN - 1),
+	NLDESC_ATTR_U32_MAX(NFTA_FLOWTABLE_HOOK, NF_NETDEV_INGRESS),
+	NLDESC_ATTR_U32(NFTA_FLOWTABLE_USE),
+	NLDESC_ATTR_U64(NFTA_FLOWTABLE_HANDLE),
+	NLDESC_ATTR_PAD(NFTA_FLOWTABLE_PAD),
+};
+
+static const struct nl_desc_obj nft_nldesc_base[] = {
+	NLDESC_OBJ(NFT_TABLE, nft_nldesc_table_attrs, NFTA_TABLE_MAX),
+	NLDESC_OBJ(NFT_CHAIN, nft_nldesc_chain_attrs, NFTA_CHAIN_MAX),
+	NLDESC_OBJ(NFT_RULE, nft_nldesc_rule_attrs, NFTA_RULE_MAX),
+	NLDESC_OBJ(NFT_SET, nft_nldesc_set_attrs, NFTA_SET_MAX),
+	NLDESC_OBJ(NFT_SET_ELEM, nft_nldesc_set_elem_attrs, NFTA_SET_ELEM_MAX),
+	NLDESC_OBJ(NFT_OBJ, nft_nldesc_obj_attrs, NFTA_OBJ_MAX),
+	NLDESC_OBJ(NFT_FLOWTABLE, nft_nldesc_flowtable_attrs, NFTA_FLOWTABLE_MAX),
+	NLDESC_OBJ_END,
+};
+
+static const struct nl_desc_obj *nft_nldesc_obj_table[] = {
+	nft_nldesc_base,
+	nft_nldesc_chain_dev,
+	nft_nldesc_chain_hook,
+	nft_nldesc_counters,
+	nft_nldesc_data,
+	nft_nldesc_expressions,
+	nft_nldesc_expr_data,
+	nft_nldesc_expressions,
+	nft_nldesc_rule_compat,
+	nft_nldesc_set_desc,
+	nft_nldesc_obj_data,
+	NULL,
+};
+
+static const struct nl_desc_objs nft_desc_objs = {
+	.max	= NFT_MAX,
+	.table	= nft_nldesc_obj_table,
+};
+
+static const struct nl_desc_cmd nft_nldesc_cmd_table[] = {
+	NLDESC_CMD(NFT_MSG_NEWTABLE, NFT_TABLE),
+	NLDESC_CMD(NFT_MSG_GETTABLE, NFT_TABLE),
+	NLDESC_CMD(NFT_MSG_DELTABLE, NFT_TABLE),
+	NLDESC_CMD(NFT_MSG_NEWCHAIN, NFT_CHAIN),
+	NLDESC_CMD(NFT_MSG_GETCHAIN, NFT_CHAIN),
+	NLDESC_CMD(NFT_MSG_DELCHAIN, NFT_CHAIN),
+	NLDESC_CMD(NFT_MSG_NEWRULE, NFT_RULE),
+	NLDESC_CMD(NFT_MSG_GETRULE, NFT_RULE),
+	NLDESC_CMD(NFT_MSG_DELRULE, NFT_RULE),
+	NLDESC_CMD(NFT_MSG_NEWSET, NFT_SET),
+	NLDESC_CMD(NFT_MSG_GETSET, NFT_SET),
+	NLDESC_CMD(NFT_MSG_DELSET, NFT_SET),
+	NLDESC_CMD(NFT_MSG_NEWSETELEM, NFT_SET_ELEM),
+	NLDESC_CMD(NFT_MSG_GETSETELEM, NFT_SET_ELEM),
+	NLDESC_CMD(NFT_MSG_DELSETELEM, NFT_SET_ELEM),
+	NLDESC_CMD(NFT_MSG_NEWOBJ, NFT_OBJ),
+	NLDESC_CMD(NFT_MSG_GETOBJ, NFT_OBJ),
+	NLDESC_CMD(NFT_MSG_DELOBJ, NFT_OBJ),
+	NLDESC_CMD(NFT_MSG_GETOBJ_RESET, NFT_OBJ),
+	NLDESC_CMD(NFT_MSG_NEWFLOWTABLE, NFT_FLOWTABLE),
+	NLDESC_CMD(NFT_MSG_GETFLOWTABLE, NFT_FLOWTABLE),
+	NLDESC_CMD(NFT_MSG_DELFLOWTABLE, NFT_FLOWTABLE),
+	NLDESC_CMD_END,
+};
+
+static const struct nl_desc_cmds nft_desc_cmds = {
+	.max	= NFT_MSG_MAX,
+	.table	= nft_nldesc_cmd_table,
+};
+
+const struct nfnl_desc_subsys nft_nldesc = {
+	.id	= NFNL_SUBSYS_NFTABLES,
+	.cmds	= &nft_desc_cmds,
+	.objs	= &nft_desc_objs,
+};
-- 
2.11.0

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Wed, 2018-02-07 at 02:37 +0100, Pablo Neira Ayuso wrote:
> 
> +static const struct nl_desc_attr nft_nldesc_table_attrs[NFTA_TABLE_MAX + 1] = {
> +	NLDESC_ATTR_STRING(NFTA_TABLE_NAME, NFT_NAME_MAXLEN - 1),
> +	NLDESC_ATTR_U32_MAX(NFTA_TABLE_FLAGS, NFT_TABLE_F_DORMANT),
> +	NLDESC_ATTR_U32(NFTA_TABLE_USE),
> +	NLDESC_ATTR_U64(NFTA_TABLE_HANDLE),
> +	NLDESC_ATTR_PAD(NFTA_TABLE_PAD),
> +};

This part I don't really like so much. Yes, it's >1yo, so we didn't have
all the stuff at the time.

But this is _lots_ of code, and most of it is already encoded in the
nla_policy for all those attributes. I think we really need to find a
way to unify the two sets of data.

I'd actually be happy to *just* expose the policy with all its nested
elements/objects in there. That'd probably be also almost enough for
you? And that way you don't need to maintain the same descriptions twice
(and we *will* get it wrong if we do that).

For genl sub-families, I'd also say that the whole thing could be
exposed within the genl family, so I think we'd want to express this as
some kind of helper logic to export the policy data.

I'm not entirely sure what the whole story with your "objects" is
though. Aren't those just nested attributes of sort?

I see e.g.

> 
> +static const struct nl_desc_attr nft_nldesc_set_desc_attrs[NFTA_SET_DESC_MAX + 1] = {
> +       NLDESC_ATTR_U32(NFTA_SET_DESC_SIZE),
> +};

This is policy data - like I said above we should unify the two.

> +static const struct nl_desc_obj nft_nldesc_set_desc[] = {
> +       NLDESC_OBJ(NFT_SET_DESC, nft_nldesc_set_desc_attrs, NFTA_SET_DESC_MAX),

This is some kind of "object", which I'm not sure I understand, but
NFTA_SET_DESC_MAX is just the policy data of what the max attribute is
for this thing.

> +static const struct nl_desc_attr nft_nldesc_set_attrs[NFTA_SET_MAX + 1] = {
...
> +       NLDESC_ATTR_NESTED(NFTA_SET_DESC, nft_nldesc_set_desc),

and here this is again just a policy thing, more or less.

So it seems to me that without even having an object enum, we'd get the
same kind of data by

struct nla_policy nft_policy_set_desc[NFTA_SET_DESC_MAX + 1] = {
	[NFTA_SET_DESC_SIZE] = { .type = NLA_U32 },
};

struct nla_policy nft_policy_set[NFTA_SET_MAX + 1] = {
	[NFTA_SET_DESC] = NLA_POLICY_NESTED(nft_policy_set_desc),
};

Except we don't have an "object ID" (and obviously also that we can use
the same data to actually parse/validate messages, and so don't end up
with bad situations of nldesc saying one thing and the policy another.)



So ... Assuming I write some code to expose the policy (which I plan on
doing), what would this still be able to expose in addition?

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

On Fri, Mar 29, 2019 at 11:59:46AM +0100, Johannes Berg wrote:
> On Wed, 2018-02-07 at 02:37 +0100, Pablo Neira Ayuso wrote:
> > 
> > +static const struct nl_desc_attr nft_nldesc_table_attrs[NFTA_TABLE_MAX + 1] = {
> > +	NLDESC_ATTR_STRING(NFTA_TABLE_NAME, NFT_NAME_MAXLEN - 1),
> > +	NLDESC_ATTR_U32_MAX(NFTA_TABLE_FLAGS, NFT_TABLE_F_DORMANT),
> > +	NLDESC_ATTR_U32(NFTA_TABLE_USE),
> > +	NLDESC_ATTR_U64(NFTA_TABLE_HANDLE),
> > +	NLDESC_ATTR_PAD(NFTA_TABLE_PAD),
> > +};
> 
> This part I don't really like so much. Yes, it's >1yo, so we didn't have
> all the stuff at the time.

I'm glad to receive feedback, even 1 year later :-)

> But this is _lots_ of code, and most of it is already encoded in the
> nla_policy for all those attributes. I think we really need to find a
> way to unify the two sets of data.

We could probably extract this from the nla_policy if we extend it. I
can have a look...

> I'd actually be happy to *just* expose the policy with all its nested
> elements/objects in there. That'd probably be also almost enough for
> you?

Exposing commands is also important, I know of people poking for
commands to see if they are available or not.

I think we should full description to userspace, not only policies.

> And that way you don't need to maintain the same descriptions twice
> (and we *will* get it wrong if we do that).
> 
> For genl sub-families, I'd also say that the whole thing could be
> exposed within the genl family, so I think we'd want to express this as
> some kind of helper logic to export the policy data.
> 
> I'm not entirely sure what the whole story with your "objects" is
> though. Aren't those just nested attributes of sort?
> 
> I see e.g.
> 
> > 
> > +static const struct nl_desc_attr nft_nldesc_set_desc_attrs[NFTA_SET_DESC_MAX + 1] = {
> > +       NLDESC_ATTR_U32(NFTA_SET_DESC_SIZE),
> > +};
> 
> This is policy data - like I said above we should unify the two.
> 
> > +static const struct nl_desc_obj nft_nldesc_set_desc[] = {
> > +       NLDESC_OBJ(NFT_SET_DESC, nft_nldesc_set_desc_attrs, NFTA_SET_DESC_MAX),
> 
> This is some kind of "object", which I'm not sure I understand, but
> NFTA_SET_DESC_MAX is just the policy data of what the max attribute is
> for this thing.
> 
> > +static const struct nl_desc_attr nft_nldesc_set_attrs[NFTA_SET_MAX + 1] = {
> ...
> > +       NLDESC_ATTR_NESTED(NFTA_SET_DESC, nft_nldesc_set_desc),
> 
> and here this is again just a policy thing, more or less.
> 
> So it seems to me that without even having an object enum, we'd get the
> same kind of data by
> 
> struct nla_policy nft_policy_set_desc[NFTA_SET_DESC_MAX + 1] = {
> 	[NFTA_SET_DESC_SIZE] = { .type = NLA_U32 },
> };
> 
> struct nla_policy nft_policy_set[NFTA_SET_MAX + 1] = {
> 	[NFTA_SET_DESC] = NLA_POLICY_NESTED(nft_policy_set_desc),
> };
> 
> Except we don't have an "object ID" (and obviously also that we can use
> the same data to actually parse/validate messages, and so don't end up
> with bad situations of nldesc saying one thing and the policy another.)

The object ID is needed by userspace, to build a flat table, and look
up for the nested IDs, to express the graph. We can probably place
this in the policy.

> So ... Assuming I write some code to expose the policy (which I plan on
> doing), what would this still be able to expose in addition?

This would be incomplete, since commands are an essential part too,
and relation of attributes with commands are also too.

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

Hi Pablo,

> I'm glad to receive feedback, even 1 year later :-)

:-)

> > But this is _lots_ of code, and most of it is already encoded in the
> > nla_policy for all those attributes. I think we really need to find a
> > way to unify the two sets of data.
> 
> We could probably extract this from the nla_policy if we extend it. I
> can have a look...

Please do, and please have a look at the patch(es) I sent out regarding
this. I think we've extended the policy enough that most of this is
really there, and we can certainly add more to the policy.

For example, I am now considering allowing NLA_POLICY_RANGE(NLA_BINARY,
...) to have an upper *and* lower length bound for an NLA_BINARY value,
right now you can only have either of the two and we have uses cases
where we need both.

> > I'd actually be happy to *just* expose the policy with all its nested
> > elements/objects in there. That'd probably be also almost enough for
> > you?
> 
> Exposing commands is also important, I know of people poking for
> commands to see if they are available or not.

Alright, well, generic netlink already exposes the commands :-)

> I think we should full description to userspace, not only policies.

I guess the way I see it, they're all pieces of the puzzle. The policy
is good for both code in the kernel and for exposing it. The command
list is already exposed.

What now we're missing, in a sense, is which commands take which
attributes. This would actually *also* be a good thing for validation in
the kernel, but I haven't really found a good way to encode it yet.
Doing a bitmap isn't really feasible, and doing a list is difficult to
type out.

Perhaps if we do end up going with your code generation idea then we can
do this more easily.

> > Except we don't have an "object ID" (and obviously also that we can use
> > the same data to actually parse/validate messages, and so don't end up
> > with bad situations of nldesc saying one thing and the policy another.)
> 
> The object ID is needed by userspace, to build a flat table, and look
> up for the nested IDs, to express the graph. We can probably place
> this in the policy.

I don't really understand the object ID.

Say you have a top-level "filter" attribute (I'm completely making up
things here).

Now this filter "object" can have a number of attributes, and that's
captured by the nested policy there?

IOW - the code I wrote until now would expose

policy 0: attribute filter: nested with attrs from policy 1
policy 1: attribute abc: NLA_U8
policy 1: attribute xyz: NLA_U32

etc.

> This would be incomplete, since commands are an essential part too,
> and relation of attributes with commands are also too.

Agree. Still, I think it solves much of the problem, like I said above -
all are parts of the puzzle.

With generic netlink we have a list of commands already. That should be
easy to add elsewhere, though generic netlink has the advantage of
having the list of commands (and their handlers) as *data*, which I
think is expressed *code* in rtnetlink now? But that can either be fixed
or you just carry a separate list of commands there.

With the policy export I wrote we have the policy, and if you set it up
properly with all the nested sub-policies for each attribute etc.

So I agree - we're missing "this command uses these attributes", but
again I don't think the object description is actually the right
solution here, because you *also* want to enforce that in some way.

I feel the generic netlink model where the command handlers etc. are not
code but data is superior here because it lets us add things like that
relatively easily, though of course we haven't actually done it now.


So basically - my main concern with this bus description stuff you've
done here is that it duplicates all the data and needs to be maintained
separately from what should be the same data used for enforcement.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

On Fri, Apr 12, 2019 at 01:56:22PM +0200, Johannes Berg wrote:
> > > I'd actually be happy to *just* expose the policy with all its nested
> > > elements/objects in there. That'd probably be also almost enough for
> > > you?
> > 
> > Exposing commands is also important, I know of people poking for
> > commands to see if they are available or not.
> 
> Alright, well, generic netlink already exposes the commands :-)

How are you going to relate this with the policy description?

Could you post a schema of the netlink attribute hierarchy of your
policy descriptions?

> > I think we should full description to userspace, not only policies.
> 
> I guess the way I see it, they're all pieces of the puzzle. The policy
> is good for both code in the kernel and for exposing it. The command
> list is already exposed.
> 
> What now we're missing, in a sense, is which commands take which
> attributes. This would actually *also* be a good thing for validation in
> the kernel, but I haven't really found a good way to encode it yet.
> Doing a bitmap isn't really feasible, and doing a list is difficult to
> type out.
> 
> Perhaps if we do end up going with your code generation idea then we can
> do this more easily.
> 
> > > Except we don't have an "object ID" (and obviously also that we can use
> > > the same data to actually parse/validate messages, and so don't end up
> > > with bad situations of nldesc saying one thing and the policy another.)
> > 
> > The object ID is needed by userspace, to build a flat table, and look
> > up for the nested IDs, to express the graph. We can probably place
> > this in the policy.
> 
> I don't really understand the object ID.

This object ID is a policy nest ID.

The idea is to expose an array of nest IDs (object IDs), then this array
contains the attributes that this nest contains and if there are nested
attributes, then in case of nested attributed, the nest ID that describe
this attribute.

The idea is to make it easy for userspace to build this table, and then
userspace performs lookups based on this object ID (policy nest ID).

> Say you have a top-level "filter" attribute (I'm completely making up
> things here).
> 
> Now this filter "object" can have a number of attributes, and that's
> captured by the nested policy there?
> 
> IOW - the code I wrote until now would expose
> 
> policy 0: attribute filter: nested with attrs from policy 1
> policy 1: attribute abc: NLA_U8
> policy 1: attribute xyz: NLA_U32
> 
> etc.
> 
> > This would be incomplete, since commands are an essential part too,
> > and relation of attributes with commands are also too.
> 
> Agree. Still, I think it solves much of the problem, like I said above -
> all are parts of the puzzle.
> 
> With generic netlink we have a list of commands already. That should be
> easy to add elsewhere, though generic netlink has the advantage of
> having the list of commands (and their handlers) as *data*, which I
> think is expressed *code* in rtnetlink now? But that can either be fixed
> or you just carry a separate list of commands there.
> 
> With the policy export I wrote we have the policy, and if you set it up
> properly with all the nested sub-policies for each attribute etc.
> 
> So I agree - we're missing "this command uses these attributes", but
> again I don't think the object description is actually the right
> solution here, because you *also* want to enforce that in some way.
> 
> I feel the generic netlink model where the command handlers etc. are not
> code but data is superior here because it lets us add things like that
> relatively easily, though of course we haven't actually done it now.
> 
> So basically - my main concern with this bus description stuff you've
> done here is that it duplicates all the data and needs to be maintained
> separately from what should be the same data used for enforcement.

My idea was to provide a description of netlink, then a compiler tool
that would autogenerate the policy structures and the UAPI headers from
this bus description. Yes, it sounds too futuristic to autogenerate code
in the kernel, but other projects do already.

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 18:42 +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 12, 2019 at 01:56:22PM +0200, Johannes Berg wrote:
> > > > I'd actually be happy to *just* expose the policy with all its nested
> > > > elements/objects in there. That'd probably be also almost enough for
> > > > you?
> > > 
> > > Exposing commands is also important, I know of people poking for
> > > commands to see if they are available or not.
> > 
> > Alright, well, generic netlink already exposes the commands :-)
> 
> How are you going to relate this with the policy description?

Well, I'm not actually relating this at all right now.

Since generic netlink already has a list of commands like

{
  .cmd = XYZ,
  .doit = do_xyz,
  .dumpit = dump_xyz,
}

and I recently moved the policy to be for the whole *family*, not for
each command, basically all we're missing is a list of attributes used
by this command.

Ideally, we'd add this as

{
  .cmd = XYZ,
  .doit = do_xyz,
  .dumpit = dump_xyz,
  .attrs = { ATTR_A, ATTR_B, ATTR_C, ATTR_D },
}

but of course there's no good way to express this in C, you'd have to
build an out-of-line array and point to it.

If we do this though, it's easy to expose, right? Just a list of
attribute types that you can go look up in the policy description
obtained separately.

> Could you post a schema of the netlink attribute hierarchy of your
> policy descriptions?

The whole patch is in your inbox ;-)

But basically it looks like this:
FAMILY_SPECIFIC_POLICY_ATTRIBUTE = { // nested
  [policy index] = { // nested, 0 = root policy
    [attribute type] = { // nested
      [NL_POLICY_TYPE_ATTR_TYPE] = U32(NL_ATTR_TYPE_*),

      // for signed limited values:
      [NL_POLICY_TYPE_ATTR_MIN_VALUE_S] = S64(...),
      [NL_POLICY_TYPE_ATTR_MAX_VALUE_S] = S64(...),

      // for unsigned limited values:
      [NL_POLICY_TYPE_ATTR_MIN_VALUE_U] = U64(...),
      [NL_POLICY_TYPE_ATTR_MAX_VALUE_U] = U64(...),

      // for binary min/max length:
      [NL_POLICY_TYPE_ATTR_MIN_LENGTH] = U32(...),
      [NL_POLICY_TYPE_ATTR_MAX_LENGTH] = U32(...),

      // for nested policy (nested or nested array):
      [NL_POLICY_TYPE_ATTR_POLICY_IDX] = U32(...),
      [NL_POLICY_TYPE_ATTR_POLICY_MAXTYPE] = U32(...),

      // for bitfield32:
      [NL_POLICY_TYPE_ATTR_BITFIELD32_MASK] = U32(...),
    }
  }
}

> > > The object ID is needed by userspace, to build a flat table, and look
> > > up for the nested IDs, to express the graph. We can probably place
> > > this in the policy.
> > 
> > I don't really understand the object ID.
> 
> This object ID is a policy nest ID.

Ok. So basically I even already have the object ID, except that I don't
really specifically give it one, I just build it based on the flattening
algorithm I described in my other email..

> The idea is to expose an array of nest IDs (object IDs), then this array
> contains the attributes that this nest contains and if there are nested
> attributes, then in case of nested attributed, the nest ID that describe
> this attribute.
> 
> The idea is to make it easy for userspace to build this table, and then
> userspace performs lookups based on this object ID (policy nest ID).

Right. I think I have that?

> > So basically - my main concern with this bus description stuff you've
> > done here is that it duplicates all the data and needs to be maintained
> > separately from what should be the same data used for enforcement.
> 
> My idea was to provide a description of netlink, then a compiler tool
> that would autogenerate the policy structures and the UAPI headers from
> this bus description. Yes, it sounds too futuristic to autogenerate code
> in the kernel, but other projects do already.

Yeah, ok, but I think you'll have a hard time convincing everyone to
switch to that whole-sale, and ban the other approach. What I've done
with the policy is far more agreeable to making progress without a
complete rewrite. The policy exposure for generic netlink is mostly
automatic, for example, as it could be for other netlink families I
guess.

The issue with C I noted above of course does lend itself really well to
expressing it in a DSL and then generating the C code, but even *then* I
would still argue that having all of this duplicated is a waste of
memory since we need to have the same data already.

Anyway. Please check out the patches I posted today. I do know that they
cannot currently express the "which attribute belongs to which command"
part, but I believe that we can add that in some way without rewriting
the whole thing, and that making step-by-step progress is better.

So please check out the patches and tell me whether you see any
fundamental problem with that, or if you just think that they're
incomplete in this manner - I agree and really would like to address
that, but need to also see how that feeds back into the validation (it
must, after all) etc.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 19:17 +0200, Johannes Berg wrote:
> 
> Ideally, we'd add this as
> 
> {
>   .cmd = XYZ,
>   .doit = do_xyz,
>   .dumpit = dump_xyz,
>   .attrs = { ATTR_A, ATTR_B, ATTR_C, ATTR_D },
> }
> 
> but of course there's no good way to express this in C, you'd have to
> build an out-of-line array and point to it.

Actually, it's possibly even more complicated. After all, it is possible
that you have an ATTR_N, that is nested, and that contains certain sub-
attributes (ATTR_N_A, ATTR_N_B, ...) of which only some are valid for
the operation X, but a different subset is valid for operation Y.

I'm sort of hoping we don't have enough of these cases to make it really
something we want to express, because that'll be really hard to express
in a way that we can validate and expose.

> The issue with C I noted above of course does lend itself really well to
> expressing it in a DSL and then generating the C code, but even *then* I
> would still argue that having all of this duplicated is a waste of
> memory since we need to have the same data already.

And also, in addition to this, I just realized that we really don't want
any sort of separate descriptions because we want to have the ability to
validate everything that we express to userspace.

IOW - if we tell userspace "this is valid" then we should at the same
time be able to use those description data structures to actually
validate that userspace isn't sending us something out of this spec.

So I guess that's just one more reason I think that we fundamentally
must provide to userspace exactly what we validate, and make the
validation data structures (or language) expressive enough to be able to
capture the real constraints.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

On Fri, Apr 26, 2019 at 07:28:15PM +0200, Johannes Berg wrote:
> On Fri, 2019-04-26 at 19:17 +0200, Johannes Berg wrote:
> > 
> > Ideally, we'd add this as
> > 
> > {
> >   .cmd = XYZ,
> >   .doit = do_xyz,
> >   .dumpit = dump_xyz,
> >   .attrs = { ATTR_A, ATTR_B, ATTR_C, ATTR_D },
> > }
> > 
> > but of course there's no good way to express this in C, you'd have to
> > build an out-of-line array and point to it.
> 
> Actually, it's possibly even more complicated. After all, it is possible
> that you have an ATTR_N, that is nested, and that contains certain sub-
> attributes (ATTR_N_A, ATTR_N_B, ...) of which only some are valid for
> the operation X, but a different subset is valid for operation Y.

I solved this in my patchset through the object ID. So each command
points to an object ID, then such object ID comes with a list of
attributes.

If we use the list policies that you propose, then it's just an extra
enumeration to maintain for each command. And many commands will
likely reuse the same object ID.

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 20:04 +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 26, 2019 at 07:28:15PM +0200, Johannes Berg wrote:
> > On Fri, 2019-04-26 at 19:17 +0200, Johannes Berg wrote:
> > > 
> > > Ideally, we'd add this as
> > > 
> > > {
> > >   .cmd = XYZ,
> > >   .doit = do_xyz,
> > >   .dumpit = dump_xyz,
> > >   .attrs = { ATTR_A, ATTR_B, ATTR_C, ATTR_D },
> > > }
> > > 
> > > but of course there's no good way to express this in C, you'd have to
> > > build an out-of-line array and point to it.
> > 
> > Actually, it's possibly even more complicated. After all, it is possible
> > that you have an ATTR_N, that is nested, and that contains certain sub-
> > attributes (ATTR_N_A, ATTR_N_B, ...) of which only some are valid for
> > the operation X, but a different subset is valid for operation Y.
> 
> I solved this in my patchset through the object ID. So each command
> points to an object ID, then such object ID comes with a list of
> attributes.

Yeah, ok. Each object you had is basically its own policy. I just
*removed* having a separate policy for each command in generic netlink,
as  ;-)

What really I think we should have is a common policy, but only some
attributes are valid in some commands.

I guess you can slice this in different ways. From a "how much space
does this consume" and "can I reuse some code across different commands"
I think having the same policy is a good idea though.

> If we use the list policies that you propose, then it's just an extra
> enumeration to maintain for each command. And many commands will
> likely reuse the same object ID.

A policy pointer, really.

The list of policies is just built internally when you dump out a policy
with its sub-policies for nested attributes/arrays.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

On Fri, Apr 26, 2019 at 09:14:43PM +0200, Johannes Berg wrote:
> On Fri, 2019-04-26 at 20:04 +0200, Pablo Neira Ayuso wrote:
> > On Fri, Apr 26, 2019 at 07:28:15PM +0200, Johannes Berg wrote:
> > > On Fri, 2019-04-26 at 19:17 +0200, Johannes Berg wrote:
> > > > 
> > > > Ideally, we'd add this as
> > > > 
> > > > {
> > > >   .cmd = XYZ,
> > > >   .doit = do_xyz,
> > > >   .dumpit = dump_xyz,
> > > >   .attrs = { ATTR_A, ATTR_B, ATTR_C, ATTR_D },
> > > > }
> > > > 
> > > > but of course there's no good way to express this in C, you'd have to
> > > > build an out-of-line array and point to it.
> > > 
> > > Actually, it's possibly even more complicated. After all, it is possible
> > > that you have an ATTR_N, that is nested, and that contains certain sub-
> > > attributes (ATTR_N_A, ATTR_N_B, ...) of which only some are valid for
> > > the operation X, but a different subset is valid for operation Y.
> > 
> > I solved this in my patchset through the object ID. So each command
> > points to an object ID, then such object ID comes with a list of
> > attributes.
> 
> Yeah, ok. Each object you had is basically its own policy.

Hm, not necessarily.

The object id is matching to the "root policy". Several commands may
have the same "root policy" (or as I call it "object id"). For
example, in netfilter we have commands to add and to delete rules,
both would have the same "root policy".

> I just *removed* having a separate policy for each command in
> generic netlink, as  ;-)
> 
> What really I think we should have is a common policy, but only some
> attributes are valid in some commands.
> 
> I guess you can slice this in different ways. From a "how much space
> does this consume" and "can I reuse some code across different commands"
> I think having the same policy is a good idea though.
>
> > If we use the list policies that you propose, then it's just an extra
> > enumeration to maintain for each command. And many commands will
> > likely reuse the same object ID.
> 
> A policy pointer, really.

Sort of, but it has to be stable along time for userspace, right?
Actually, it's an ID.

> The list of policies is just built internally when you dump out a policy
> with its sub-policies for nested attributes/arrays.

If we expose these to userspace, we need that these object IDs are
stable, hence the enum. So userspace results in a simple program that
just makes look ups for the object ID that contains the description of
the attributes that are available.

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 21:20 +0200, Pablo Neira Ayuso wrote:

> > > I solved this in my patchset through the object ID. So each command
> > > points to an object ID, then such object ID comes with a list of
> > > attributes.
> > 
> > Yeah, ok. Each object you had is basically its own policy.
> 
> Hm, not necessarily.
> 
> The object id is matching to the "root policy". Several commands may
> have the same "root policy" (or as I call it "object id"). For
> example, in netfilter we have commands to add and to delete rules,
> both would have the same "root policy".

Right, I didn't mean they necessarily had a different policy, just that
they could.

So ... I guess I need to think about this a bit more.

My approach to this was that as long as you have a single enum
(namespace) for attributes (e.g. enum nft_chain_attributes to pick a
random example), you'd also want to have a single policy for these,
describing the possibilities.

I do admit though that this doesn't cover the "only some of these
attributes are valid for a given command" part, and I also admit that I
had sort of punted that part.

We can solve that by having separate policies, so if you have two
operations using NTFA_CHAIN_*, then perhaps an operation deletes a chain
can only take a NFTA_CHAIN_HANDLE or NFTA_CHAIN_NAME, but an operations
that sets things inside might take all the other attributes.

HOWEVER, having separate policies then opens up an easy avenue for
(mistakenly or not) having, well, separate policies! Even for the same
attribute type. And that's something we really *don't* want.

So I actually think that separating "type description", which is the
policy today, from the "type validity description" is valuable, because
ideally we do want each type (e.g. NFTA_CHAIN_HANDLE) to always be the
same regardless of command, so that a hypothetical GET_STATS command
won't take a U64 CHAIN_HANDLE while a (similarly hypothetical)
DELETE_CHAIN command takes a U32 CHAIN_HANDLE!

Hence my thought of separating "this is the policy for the attributes
(types)" and "this is the list of allowed types for this command". I do
realize now that the latter becomes difficult with nested attributes
though, but we're probably better off finding ways to express that than
having entirely different policies (also, the data would be smaller).

[[[ total aside: we could do something like the "list of allowed types"
being an array of
union valid_type {
   u16 attr;
   struct nested *nested;
};

with

struct nested {
  u16 attr;
  union valid_type *valid_nested_types;
};

I think we can probably safely assume that pointers are always >>MAX_U16
but if not we can either make that larger or find other ways of
disambiguating, e.g. by alignment (and making ATTR_TO_PTR(attr)
something like "(attr << 1) | 0x1" since "struct nested" requires 2-byte 
alignment.

> > > If we use the list policies that you propose, then it's just an extra
> > > enumeration to maintain for each command. And many commands will
> > > likely reuse the same object ID.
> > 
> > A policy pointer, really.
> 
> Sort of, but it has to be stable along time for userspace, right?
> Actually, it's an ID.

The ID in my policy export is not stable in any way. The only thing that
it's used for is to identify which sub-policy is used while you're
dumping it. It will be stable by virtue of the algorithm, but will
change when you add different sub-policy links etc. You cannot rely on
it being stable, but you also do not need to since it's only relevant
that you are able to identify the nested policy while dumping.

> > The list of policies is just built internally when you dump out a policy
> > with its sub-policies for nested attributes/arrays.
> 
> If we expose these to userspace, we need that these object IDs are
> stable, hence the enum. So userspace results in a simple program that
> just makes look ups for the object ID that contains the description of
> the attributes that are available.

Well, no.

You're now thinking of the "policy ID" I assigned for the wire format as
the object ID, but really that's not what it is. The object ID that
you're looking for is the attribute type of the nested attribute.

So if you have

struct nla_policy nested_policy[...] = { ... };

struct nla_policy policy[...] = {
    [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
};

and you dump out starting from "policy" then yes, "policy" will have ID
0, and "nested_policy" will have ID 1, but those are only temporary
identifiers for the dump. What's really relevant is the attribute type
"MY_ATTR".

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 21:37 +0200, Johannes Berg wrote:

> You're now thinking of the "policy ID" I assigned for the wire format as
> the object ID, but really that's not what it is. The object ID that
> you're looking for is the attribute type of the nested attribute.
> 
> So if you have
> 
> struct nla_policy nested_policy[...] = { ... };
> 
> struct nla_policy policy[...] = {
>     [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
> };
> 
So if we extend this, say like this:

struct nla_policy policy[...] = {
    [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
    [MY_OTHER_ATTR] = NLA_POLICY_NESTED(nested_policy),
};

then you could perhaps argue that having an object ID makes sense, and
assigning the same object ID to MY_ATTR and MY_OTHER_ATTR would make
sense?

Of course, my could would assign this the same (temporary) policy ID,
but there can be no reliance on the policy ID beyond what's needed at
runtime to map the attribute to the nested policy.

You still see at runtime that these have the same policy (since they
have the same policy ID), but at the same time presumably there was a
reason to have MY_ATTR and MY_OTHER_ATTR, so perhaps the semantics are
different even if the attributes are the same, as could perhaps be
expected if you have a SET and a CLEAR attribute (MY_ATTR and
MY_OTHER_ATTR respectively) and the contents you give has the same
policy, but different logic?

Basically, I just didn't consider this case to be significant enough to
manually and assign stable IDs of some sort, when we already have them
in the form of the attribute type.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

On Fri, Apr 26, 2019 at 09:46:33PM +0200, Johannes Berg wrote:
> On Fri, 2019-04-26 at 21:37 +0200, Johannes Berg wrote:
> 
> > You're now thinking of the "policy ID" I assigned for the wire format as
> > the object ID, but really that's not what it is. The object ID that
> > you're looking for is the attribute type of the nested attribute.
> > 
> > So if you have
> > 
> > struct nla_policy nested_policy[...] = { ... };
> > 
> > struct nla_policy policy[...] = {
> >     [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
> > };
> > 
> So if we extend this, say like this:
> 
> struct nla_policy policy[...] = {
>     [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
>     [MY_OTHER_ATTR] = NLA_POLICY_NESTED(nested_policy),
> };
> 
> then you could perhaps argue that having an object ID makes sense, and
> assigning the same object ID to MY_ATTR and MY_OTHER_ATTR would make
> sense?
> 
> Of course, my could would assign this the same (temporary) policy ID,
> but there can be no reliance on the policy ID beyond what's needed at
> runtime to map the attribute to the nested policy.
> 
> You still see at runtime that these have the same policy (since they
> have the same policy ID), but at the same time presumably there was a
> reason to have MY_ATTR and MY_OTHER_ATTR, so perhaps the semantics are
> different even if the attributes are the same, as could perhaps be
> expected if you have a SET and a CLEAR attribute (MY_ATTR and
> MY_OTHER_ATTR respectively) and the contents you give has the same
> policy, but different logic?
> 
> Basically, I just didn't consider this case to be significant enough to
> manually and assign stable IDs of some sort, when we already have them
> in the form of the attribute type.

But they all point to the same nested_policy, ie. these nested
atributes represent instances of the same object class.

I think this is meaningful to userspace in terms of providing a
description of the interface, rather than making it look.

Without the ID, it is not possible from userspace to see that MY_ATTR
and MY_OTHER_ATTR refer to the same object, right?

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Sat, 2019-04-27 at 12:57 +0200, Pablo Neira Ayuso wrote:

> But they all point to the same nested_policy, ie. these nested
> atributes represent instances of the same object class.

To some extent, yes.

> I think this is meaningful to userspace in terms of providing a
> description of the interface, rather than making it look.

Sure.

> Without the ID, it is not possible from userspace to see that MY_ATTR
> and MY_OTHER_ATTR refer to the same object, right?

There is an ID, and if you reference the same sub-policy multiple times
for nested / nested array attribute types (even at different levels of
nesting btw) then this sub-policy will only be dumped to userspace
multiple times, given an ID, and be referenced by that ID from the
appropriate attribute types in other root/sub-policies.

The only thing is that between kernel versions that ID may change as
it's computed while walking the policy graph, and that graph may change
and thus the walk may reach nodes in the graph in a different order and
thereby label them differently.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Pablo Neira Ayuso]

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

On Fri, Apr 26, 2019 at 09:37:43PM +0200, Johannes Berg wrote:
> On Fri, 2019-04-26 at 21:20 +0200, Pablo Neira Ayuso wrote:
> 
[...]
> So ... I guess I need to think about this a bit more.
> 
> My approach to this was that as long as you have a single enum
> (namespace) for attributes (e.g. enum nft_chain_attributes to pick a
> random example), you'd also want to have a single policy for these,
> describing the possibilities.
> 
> I do admit though that this doesn't cover the "only some of these
> attributes are valid for a given command" part, and I also admit that I
> had sort of punted that part.
> 
> We can solve that by having separate policies, so if you have two
> operations using NTFA_CHAIN_*, then perhaps an operation deletes a chain
> can only take a NFTA_CHAIN_HANDLE or NFTA_CHAIN_NAME, but an operations
> that sets things inside might take all the other attributes.
>
> HOWEVER, having separate policies then opens up an easy avenue for
> (mistakenly or not) having, well, separate policies! Even for the same
> attribute type. And that's something we really *don't* want.
>
> So I actually think that separating "type description", which is the
> policy today, from the "type validity description" is valuable, because
> ideally we do want each type (e.g. NFTA_CHAIN_HANDLE) to always be the
> same regardless of command, so that a hypothetical GET_STATS command
> won't take a U64 CHAIN_HANDLE while a (similarly hypothetical)
> DELETE_CHAIN command takes a U32 CHAIN_HANDLE!

If we want to avoid the extra code for the netlink description, and
reuse the existing policy objects, then we'll need policies for each
command that express what attributes make sense per command as you
describe above.

> Hence my thought of separating "this is the policy for the attributes
> (types)" and "this is the list of allowed types for this command". I do
> realize now that the latter becomes difficult with nested attributes
> though, but we're probably better off finding ways to express that than
> having entirely different policies (also, the data would be smaller).

The netlink description is telling what attributes are available for
each command and policies, so policy definitions are a subset of the
netlink description.

If I understand well, your concern is that you would like that we
use reuse / extend the existing policy structures to be used for the
netlink description. That's very reasonable indeed and a good idea.

> [[[ total aside: we could do something like the "list of allowed types"
> being an array of
> union valid_type {
>    u16 attr;
>    struct nested *nested;
> };
> 
> with
> 
> struct nested {
>   u16 attr;
>   union valid_type *valid_nested_types;
> };
>
> I think we can probably safely assume that pointers are always >>MAX_U16
> but if not we can either make that larger or find other ways of
> disambiguating, e.g. by alignment (and making ATTR_TO_PTR(attr)
> something like "(attr << 1) | 0x1" since "struct nested" requires 2-byte 
> alignment.
> 
> > > > If we use the list policies that you propose, then it's just an extra
> > > > enumeration to maintain for each command. And many commands will
> > > > likely reuse the same object ID.
> > > 
> > > A policy pointer, really.
> > 
> > Sort of, but it has to be stable along time for userspace, right?
> > Actually, it's an ID.
> 
> The ID in my policy export is not stable in any way. The only thing that
> it's used for is to identify which sub-policy is used while you're
> dumping it. It will be stable by virtue of the algorithm, but will
> change when you add different sub-policy links etc. You cannot rely on
> it being stable, but you also do not need to since it's only relevant
> that you are able to identify the nested policy while dumping.
>
> > > The list of policies is just built internally when you dump out a policy
> > > with its sub-policies for nested attributes/arrays.
> > 
> > If we expose these to userspace, we need that these object IDs are
> > stable, hence the enum. So userspace results in a simple program that
> > just makes look ups for the object ID that contains the description of
> > the attributes that are available.
> 
> Well, no.
> 
> You're now thinking of the "policy ID" I assigned for the wire format as
> the object ID, but really that's not what it is. The object ID that
> you're looking for is the attribute type of the nested attribute.

I'm attaching userspace code for the netlink description
infrastructure for libmnl. It should be easy to rework it to build a
flat table with these IDs. The IDs are helping to provide a easy way
to express this graph in a data structure that is easy to navigate
from userspace.

My usecase is this: I don't want to probe the kernel to guess if a
command / attribute is available for this kernel version or not.
I just want to dump the netlink description, then navigate this table
to search if that command / attribute is there.

> So if you have
> 
> struct nla_policy nested_policy[...] = { ... };
> 
> struct nla_policy policy[...] = {
>     [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
> };
> 
> and you dump out starting from "policy" then yes, "policy" will have ID
> 0, and "nested_policy" will have ID 1, but those are only temporary
> identifiers for the dump. What's really relevant is the attribute type
> "MY_ATTR".

Do you have userspace I can have a look? With runtime / dynamic ID
generation, I cannot see yet how to navigate such a table or how the
userspace representation would look like.

By using MY_ATTR as ID, I think you will have to build a graph
structure in userspace, I was trying to provide a simple flat table
representation for userspace.

Thanks!

[-- Attachment #2: nl-desc-libmnl.patch --]
[-- Type: text/x-diff, Size: 11833 bytes --]

commit 7826d6aa47d20bc09f7c8e33a457a5a338a8db55
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Tue Jan 16 00:05:37 2018 +0100

    examples: add netlink bus description
    
    Add nft-dump-desc-cmds.c and nft-dump-desc-obj.c to dump command and
    object descriptions.

diff --git a/examples/Makefile.am b/examples/Makefile.am
index e5cb052b315c..a8d4ba50f5ad 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -1 +1,12 @@
+include $(top_srcdir)/Make_global.am
+
 SUBDIRS = genl kobject netfilter rtnl
+
+check_PROGRAMS = nft-dump-desc-cmds \
+                 nft-dump-desc-objs
+
+nft_dump_desc_cmds_SOURCES = nft-dump-desc-cmds.c
+nft_dump_desc_cmds_LDADD = ../src/libmnl.la
+
+nft_dump_desc_objs_SOURCES = nft-dump-desc-objs.c
+nft_dump_desc_objs_LDADD = ../src/libmnl.la
diff --git a/examples/nft-dump-desc-cmds.c b/examples/nft-dump-desc-cmds.c
new file mode 100644
index 000000000000..cfb5276e911f
--- /dev/null
+++ b/examples/nft-dump-desc-cmds.c
@@ -0,0 +1,177 @@
+#include <endian.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <time.h>
+#include <arpa/inet.h>
+#include <inttypes.h>
+
+#include <linux/netlink.h>
+#include <linux/netfilter/nfnetlink.h>
+
+#include <libmnl/libmnl.h>
+
+struct nl_desc_cmd;
+struct nl_desc_attr;
+
+struct nl_desc {
+	uint32_t			num_cmds;
+	struct nl_desc_cmd		*cmds;
+};
+
+struct nl_desc_cmd {
+	uint32_t			id;
+	uint32_t			obj_id;
+};
+
+static struct nl_desc nl_desc;
+
+static int nla_desc_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_CMD_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NLA_DESC_CMD_ID:
+	case NLA_DESC_CMD_OBJ:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_cmd(const struct nlattr *nest, struct nl_desc_cmd *cmd)
+{
+	struct nlattr *tb[NLA_DESC_CMD_MAX + 1] = {};
+
+	mnl_attr_parse_nested(nest, nla_desc_attr_cb, tb);
+	if (tb[NLA_DESC_CMD_ID])
+		cmd->id = mnl_attr_get_u32(tb[NLA_DESC_CMD_ID]);
+	if (tb[NLA_DESC_CMD_OBJ])
+		cmd->obj_id = mnl_attr_get_u32(tb[NLA_DESC_CMD_OBJ]);
+}
+
+static void print_desc_cmds(const struct nlattr *nest, struct nl_desc_cmd *cmds)
+{
+	struct nlattr *pos;
+	int j = 1;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_cmd(pos, &cmds[j++]);
+}
+
+static int nla_desc_cmds_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_NUM_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int data_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[NLA_DESC_CMDS_MAX + 1] = {};
+
+	mnl_attr_parse(nlh, 0, nla_desc_cmds_cb, tb);
+	if (tb[NLA_DESC_CMDS_NUM]) {
+		nl_desc.num_cmds = mnl_attr_get_u32(tb[NLA_DESC_CMDS_NUM]);
+
+		nl_desc.cmds = calloc(nl_desc.num_cmds + 1, sizeof(struct nl_desc_cmd));
+		if (!nl_desc.cmds)
+			return MNL_CB_ERROR;
+	}
+
+	if (tb[NLA_DESC_CMDS])
+		print_desc_cmds(tb[NLA_DESC_CMDS], nl_desc.cmds);
+
+	return MNL_CB_OK;
+}
+
+#define NETLINK_DESC	23
+#define NLDESC_GET_CMDS	16
+
+int main(void)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	struct mnl_socket *nl;
+	struct nlmsghdr *nlh;
+	uint32_t seq, portid;
+	struct nlattr *nest;
+	int ret, i;
+
+	nl = mnl_socket_open(NETLINK_DESC);
+	if (nl == NULL) {
+		perror("mnl_socket_open");
+		exit(EXIT_FAILURE);
+	}
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		perror("mnl_socket_bind");
+		exit(EXIT_FAILURE);
+	}
+
+	nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = NLDESC_GET_CMDS;
+	nlh->nlmsg_flags = NLM_F_REQUEST|NLM_F_DUMP;
+	nlh->nlmsg_seq = seq = time(NULL);
+
+	mnl_attr_put_u32(nlh, NLA_DESC_REQ_BUS, NETLINK_NETFILTER);
+	nest = mnl_attr_nest_start(nlh, NLA_DESC_REQ_DATA);
+	mnl_attr_put_u32(nlh, NFNL_DESC_REQ_SUBSYS, NFNL_SUBSYS_NFTABLES);
+	mnl_attr_nest_end(nlh, nest);
+
+	ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+	if (ret == -1) {
+		perror("mnl_socket_sendto");
+		exit(EXIT_FAILURE);
+	}
+	portid = mnl_socket_get_portid(nl);
+
+	while (1) {
+		ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("mnl_socket_recvfrom");
+			exit(EXIT_FAILURE);
+		}
+		ret = mnl_cb_run(buf, ret, seq, portid, data_cb, &nl_desc);
+		if (ret == -1) {
+			perror("mnl_cb_run");
+			exit(EXIT_FAILURE);
+		} else if (ret <= MNL_CB_STOP)
+                        break;
+	}
+
+	mnl_socket_close(nl);
+
+	for (i = 1; nl_desc.cmds[i].obj_id; i++) {
+		printf("cmd = %d\n", nl_desc.cmds[i].id);
+		printf("obj_id = %d\n", nl_desc.cmds[i].obj_id);
+	}
+
+	return 0;
+}
diff --git a/examples/nft-dump-desc-objs.c b/examples/nft-dump-desc-objs.c
new file mode 100644
index 000000000000..8f5b365e3c64
--- /dev/null
+++ b/examples/nft-dump-desc-objs.c
@@ -0,0 +1,263 @@
+#include <endian.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <time.h>
+#include <arpa/inet.h>
+#include <inttypes.h>
+
+#include <linux/netlink.h>
+#include <linux/netfilter/nfnetlink.h>
+
+#include <libmnl/libmnl.h>
+
+struct n_desc_obj;
+struct n_desc_attr;
+
+struct nl_desc {
+	uint32_t			num_objs;
+	struct nl_desc_obj		*objs;
+};
+
+struct nl_desc_obj {
+	uint16_t			id;
+	uint16_t			max;
+	struct nl_desc_attr		*attrs;
+};
+
+struct nl_desc_attr {
+	uint16_t			nest_id;
+	uint16_t			num;
+	uint16_t			type;
+	uint16_t			len;
+	uint32_t			max;
+};
+
+static struct nl_desc nl_desc;
+
+static int nla_desc_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_ATTR_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NLA_DESC_ATTR_NUM:
+	case NLA_DESC_ATTR_TYPE:
+	case NLA_DESC_ATTR_LEN:
+	case NLA_DESC_ATTR_MAXVAL:
+	case NLA_DESC_ATTR_NEST_ID:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_attr(const struct nlattr *nest, struct nl_desc_attr *attr)
+{
+	struct nlattr *tb[NLA_DESC_ATTR_MAX + 1] = {};
+
+	mnl_attr_parse_nested(nest, nla_desc_attr_cb, tb);
+	if (tb[NLA_DESC_ATTR_NUM])
+		attr->num = mnl_attr_get_u32(tb[NLA_DESC_ATTR_NUM]);
+	if (tb[NLA_DESC_ATTR_TYPE])
+		attr->type = mnl_attr_get_u32(tb[NLA_DESC_ATTR_TYPE]);
+	if (tb[NLA_DESC_ATTR_LEN])
+		attr->len = mnl_attr_get_u32(tb[NLA_DESC_ATTR_LEN]);
+	if (tb[NLA_DESC_ATTR_MAXVAL])
+		attr->max = mnl_attr_get_u32(tb[NLA_DESC_ATTR_MAXVAL]);
+	if (tb[NLA_DESC_ATTR_NEST_ID])
+		attr->nest_id = mnl_attr_get_u32(tb[NLA_DESC_ATTR_NEST_ID]);
+}
+
+static void print_desc_attrs(const struct nlattr *nest, struct nl_desc_attr *attrs)
+{
+	struct nlattr *pos;
+	int j = 1;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_attr(pos, &attrs[j++]);
+}
+
+static int nla_desc_obj_attr_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_OBJ_ID:
+	case NLA_DESC_OBJ_ATTRS_MAX:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJ_ATTRS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static void print_desc_obj(const struct nlattr *nest)
+{
+	struct nlattr *tb[NLA_DESC_OBJ_MAX + 1] = {};
+	uint32_t id = 0, attrs_max;
+
+	mnl_attr_parse_nested(nest, nla_desc_obj_attr_cb, tb);
+	if (tb[NLA_DESC_OBJ_ID])
+		id = mnl_attr_get_u32(tb[NLA_DESC_OBJ_ID]);
+	if (tb[NLA_DESC_OBJ_ATTRS_MAX]) {
+		attrs_max = mnl_attr_get_u32(tb[NLA_DESC_OBJ_ATTRS_MAX]);
+
+		nl_desc.objs[id].attrs = calloc(attrs_max + 1, sizeof(struct nl_desc_attr));
+		if (!nl_desc.objs[id].attrs)
+			return;
+
+		nl_desc.objs[id].max = attrs_max;
+	}
+	if (tb[NLA_DESC_OBJ_ATTRS])
+		print_desc_attrs(tb[NLA_DESC_OBJ_ATTRS], nl_desc.objs[id].attrs);
+}
+
+static void print_desc_objs(const struct nlattr *nest)
+{
+	struct nlattr *pos;
+
+	mnl_attr_for_each_nested(pos, nest)
+		print_desc_obj(pos);
+}
+
+static int nla_desc_objs_cb(const struct nlattr *attr, void *data)
+{
+	const struct nlattr **tb = data;
+	int type = mnl_attr_get_type(attr);
+
+	if (mnl_attr_type_valid(attr, NLA_DESC_OBJ_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch(type) {
+	case NLA_DESC_NUM_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	case NLA_DESC_OBJS:
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0) {
+			perror("mnl_attr_validate");
+			return MNL_CB_ERROR;
+		}
+		break;
+	}
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int data_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[NLA_DESC_MAX + 1] = {};
+
+	mnl_attr_parse(nlh, 0, nla_desc_objs_cb, tb);
+	if (tb[NLA_DESC_NUM_OBJS]) {
+		nl_desc.num_objs = mnl_attr_get_u32(tb[NLA_DESC_NUM_OBJS]);
+
+		nl_desc.objs = calloc(nl_desc.num_objs + 1, sizeof(struct nl_desc_obj));
+		if (!nl_desc.objs)
+			return MNL_CB_ERROR;
+	}
+
+	if (tb[NLA_DESC_OBJS])
+		print_desc_objs(tb[NLA_DESC_OBJS]);
+
+	return MNL_CB_OK;
+}
+
+#define NETLINK_DESC	23
+#define NLDESC_GET_OBJS	18
+
+int main(void)
+{
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+	struct mnl_socket *nl;
+	struct nlmsghdr *nlh;
+	uint32_t seq, portid;
+	struct nlattr *nest;
+	int ret, i, j;
+
+	nl = mnl_socket_open(NETLINK_DESC);
+	if (nl == NULL) {
+		perror("mnl_socket_open");
+		exit(EXIT_FAILURE);
+	}
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		perror("mnl_socket_bind");
+		exit(EXIT_FAILURE);
+	}
+
+	nlh = mnl_nlmsg_put_header(buf);
+	nlh->nlmsg_type = NLDESC_GET_OBJS;
+	nlh->nlmsg_flags = NLM_F_REQUEST|NLM_F_DUMP;
+	nlh->nlmsg_seq = seq = time(NULL);
+
+	mnl_attr_put_u32(nlh, NLA_DESC_REQ_BUS, NETLINK_NETFILTER);
+	nest = mnl_attr_nest_start(nlh, NLA_DESC_REQ_DATA);
+	mnl_attr_put_u32(nlh, NFNL_DESC_REQ_SUBSYS, NFNL_SUBSYS_NFTABLES);
+	mnl_attr_nest_end(nlh, nest);
+
+	ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+	if (ret == -1) {
+		perror("mnl_socket_sendto");
+		exit(EXIT_FAILURE);
+	}
+	portid = mnl_socket_get_portid(nl);
+
+	while (1) {
+		ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("mnl_socket_recvfrom");
+			exit(EXIT_FAILURE);
+		}
+		ret = mnl_cb_run(buf, ret, seq, portid, data_cb, &nl_desc);
+		if (ret == -1) {
+			perror("mnl_cb_run");
+			exit(EXIT_FAILURE);
+		} else if (ret <= MNL_CB_STOP)
+                        break;
+	}
+
+	mnl_socket_close(nl);
+
+	for (i = 1; i <= nl_desc.num_objs; i++) {
+		printf("id = %d\n", i);
+		printf("attrs_max = %d\n", nl_desc.objs[i].max);
+		for (j = 1; j < nl_desc.objs[i].max + 1; j++) {
+			printf("\t---------\n");
+			printf("\tnum = %d\n", nl_desc.objs[i].attrs[j].num);
+			printf("\ttype = %d\n", nl_desc.objs[i].attrs[j].type);
+			if (nl_desc.objs[i].attrs[j].nest_id)
+				printf("\tnest_id = %d\n", nl_desc.objs[i].attrs[j].nest_id);
+			else {
+				printf("\tlen = %d\n", nl_desc.objs[i].attrs[j].len);
+				if (nl_desc.objs[i].attrs[j].max)
+					printf("\tmax = %d\n", nl_desc.objs[i].attrs[j].max);
+			}
+		}
+	}
+
+	return 0;
+}

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Sat, 2019-04-27 at 12:52 +0200, Pablo Neira Ayuso wrote:

> > So I actually think that separating "type description", which is the
> > policy today, from the "type validity description" is valuable, because
> > ideally we do want each type (e.g. NFTA_CHAIN_HANDLE) to always be the
> > same regardless of command, so that a hypothetical GET_STATS command
> > won't take a U64 CHAIN_HANDLE while a (similarly hypothetical)
> > DELETE_CHAIN command takes a U32 CHAIN_HANDLE!
> 
> If we want to avoid the extra code for the netlink description, and
> reuse the existing policy objects, then we'll need policies for each
> command that express what attributes make sense per command as you
> describe above.

Yes. Although when you say "policies", I'd argue that's not an
"nla_policy" but rather some sort of "list of permitted attributes".

> > Hence my thought of separating "this is the policy for the attributes
> > (types)" and "this is the list of allowed types for this command". I do
> > realize now that the latter becomes difficult with nested attributes
> > though, but we're probably better off finding ways to express that than
> > having entirely different policies (also, the data would be smaller).
> 
> The netlink description is telling what attributes are available for
> each command and policies, so policy definitions are a subset of the
> netlink description.
> 
> If I understand well, your concern is that you would like that we
> use reuse / extend the existing policy structures to be used for the
> netlink description. That's very reasonable indeed and a good idea.

Yes, mostly. My bigger concern isn't really that we reuse it, my bigger
concern is that if we *don't* we'll eventually end up with something
like:

struct nla_policy policy_for_one_command[] = {
   [NLA_MY_ATTR] = { .type = NLA_U32 },
   ...
};

struct nla_policy policy_for_another_command[] = {
   [NLA_MY_ATTR] = { .type = NLA_U16 },
   ...
};

and then we just have a big mess. So I'd like to separate the "type"
policy (what type is each attribute in a given netlink type enum) from
the "permissible" policy (which attributes are valid in a given context
like a command).

I've not actually started working on the second part at all yet, but
because I strongly believe it makes sense to separate them, I don't see
a need to interleave it with the rest of the patch series.

> > You're now thinking of the "policy ID" I assigned for the wire format as
> > the object ID, but really that's not what it is. The object ID that
> > you're looking for is the attribute type of the nested attribute.
> 
> I'm attaching userspace code for the netlink description
> infrastructure for libmnl. It should be easy to rework it to build a
> flat table with these IDs. The IDs are helping to provide a easy way
> to express this graph in a data structure that is easy to navigate
> from userspace.

Right.

> My usecase is this: I don't want to probe the kernel to guess if a
> command / attribute is available for this kernel version or not.
> I just want to dump the netlink description, then navigate this table
> to search if that command / attribute is there.

Sure.

The only difference between the "object ID" that you manually assign and
the "policy ID" that my code automatically assigns is that the latter is
somewhat ephemeral - a new kernel version may change it.

So, let's say you have
struct nla_policy policy[...] = {
  [NLA_MY_OBJECT] = NLA_POLICY_NESTED(sub_policy),
  [NLA_ANOTHER_OBJ] = NLA_POLICY_NESTED_ARRAY(sub_policy),
};

Then in one kernel version you might get reported to userspace:

NLA_MY_OBJECT: nested - policy 7
NLA_ANOTHER_OBJ: nested array - policy 7

and in a new kernel version the value "7" there might change to be 8 or
6 or whatever depending on changes in any possible nested policy for
other attributes, but it would still always be the same number (unless
you actually changed the fact that both link to "sub_policy" in the C
code description), and the contents of that referenced policy would
still all be the same as before. Just the ID that links them together at
runtime would be more ephemeral than the object ID you proposed.

I don't, however, think that this is a problem as I don't see any value
in the object ID itself. It's just an ID to tell you what kind of sub-
attributes are allowed.

> > So if you have
> > 
> > struct nla_policy nested_policy[...] = { ... };
> > 
> > struct nla_policy policy[...] = {
> >     [MY_ATTR] = NLA_POLICY_NESTED(nested_policy),
> > };
> > 
> > and you dump out starting from "policy" then yes, "policy" will have ID
> > 0, and "nested_policy" will have ID 1, but those are only temporary
> > identifiers for the dump. What's really relevant is the attribute type
> > "MY_ATTR".
> 
> Do you have userspace I can have a look? With runtime / dynamic ID
> generation, I cannot see yet how to navigate such a table or how the
> userspace representation would look like.

I posted a very hacky patch before, let me see if I can find it...

https://p.sipsolutions.net/4c674acaf8d6ca71.txt

> By using MY_ATTR as ID, I think you will have to build a graph
> structure in userspace, I was trying to provide a simple flat table
> representation for userspace.

I'm not using MY_ATTR as ID. I'm just making up an (ephemeral) object ID
at runtime based on the policy pointer. See above though.

johannes

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Michal Kubecek]

On Fri, Apr 26, 2019 at 09:14:43PM +0200, Johannes Berg wrote:
> Yeah, ok. Each object you had is basically its own policy. I just
> *removed* having a separate policy for each command in generic netlink,
> as  ;-)

But that only affects genetlink users who let genetlink validate and
parse messages for them. Those validating/parsing the messages
themselves can still have different policy (and completely different set
of attributes) for each command.

Michal

Re: [PATCH RFC 4/4] netfilter: nf_tables: add netlink description

[Johannes Berg]

On Fri, 2019-04-26 at 22:47 +0200, Michal Kubecek wrote:
> On Fri, Apr 26, 2019 at 09:14:43PM +0200, Johannes Berg wrote:
> > Yeah, ok. Each object you had is basically its own policy. I just
> > *removed* having a separate policy for each command in generic netlink,
> > as  ;-)
> 
> But that only affects genetlink users who let genetlink validate and
> parse messages for them. Those validating/parsing the messages
> themselves can still have different policy (and completely different set
> of attributes) for each command.

Sure. I already argued elsewhere though that you should only have one
policy for each set of attributes, but if you do in fact have different
attributes then that's perfectly valid.

Not sure whether I think it all that reasonable, since you then burden
userspace with having to know even more intricate detail and not being
able to share code well between commands, but hey :-)

johannes