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

[Pablo Neira Ayuso]

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 ..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 
> > +
> > +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!
>From 7826d6aa47d20bc09f7c8e33a457a5a338a8db55 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso 
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 ..cfb5276e911f
--- /dev/null
+++ b/examples/nft-dump-desc-cmds.c
@@ -0,0 +1,177 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+
+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, [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 

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 
> ---
>  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 ..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 
> +
> +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

[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 
---
 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_headdev_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 ..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 
+
+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_headlist;
+   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