[PATCH libnftnl v4 0/3] Attributes for concatenated ranges

[PATCH libnftnl v4 0/3] Attributes for concatenated ranges

[Stefano Brivio]

This series adds support for NFTA_SET_DESC_CONCAT set attribute and
the additional key passed as NFTA_SET_ELEM_KEY_END to denote the
upper bound of a range in a generic way, as suggested by Pablo.

v4: Sync nf_tables.h header, move NFTNL_SET_DESC_CONCAT to avoid
    ABI breakage (Pablo Neira Ayuso)

v3: Support for separate "end" key added as 2/2, reworked 1/2 to
    use set description data for length of concatenation fields

Stefano Brivio (3):
  include: resync nf_tables.h cache copy
  set: Add support for NFTA_SET_DESC_CONCAT attributes
  set_elem: Introduce support for NFTNL_SET_ELEM_KEY_END

 include/libnftnl/set.h              |   2 +
 include/linux/netfilter/nf_tables.h |  17 +++++
 include/set.h                       |   2 +
 include/set_elem.h                  |   1 +
 src/set.c                           | 111 +++++++++++++++++++++++-----
 src/set_elem.c                      |  24 ++++++
 6 files changed, 138 insertions(+), 19 deletions(-)

-- 
2.24.1

[PATCH libnftnl v4 1/3] include: resync nf_tables.h cache copy

[Stefano Brivio]

Get this header in sync with nf-next as of merge commit
b3a608222336 (5.6-rc1-ish).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v4: New patch

 include/linux/netfilter/nf_tables.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 261864736b26..065218a20bb7 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -48,6 +48,7 @@ enum nft_registers {
 
 #define NFT_REG_SIZE	16
 #define NFT_REG32_SIZE	4
+#define NFT_REG32_COUNT	(NFT_REG32_15 - NFT_REG32_00 + 1)
 
 /**
  * enum nft_verdicts - nf_tables internal verdicts
@@ -301,14 +302,28 @@ enum nft_set_policies {
  * enum nft_set_desc_attributes - set element description
  *
  * @NFTA_SET_DESC_SIZE: number of elements in set (NLA_U32)
+ * @NFTA_SET_DESC_CONCAT: description of field concatenation (NLA_NESTED)
  */
 enum nft_set_desc_attributes {
 	NFTA_SET_DESC_UNSPEC,
 	NFTA_SET_DESC_SIZE,
+	NFTA_SET_DESC_CONCAT,
 	__NFTA_SET_DESC_MAX
 };
 #define NFTA_SET_DESC_MAX	(__NFTA_SET_DESC_MAX - 1)
 
+/**
+ * enum nft_set_field_attributes - attributes of concatenated fields
+ *
+ * @NFTA_SET_FIELD_LEN: length of single field, in bits (NLA_U32)
+ */
+enum nft_set_field_attributes {
+	NFTA_SET_FIELD_UNSPEC,
+	NFTA_SET_FIELD_LEN,
+	__NFTA_SET_FIELD_MAX
+};
+#define NFTA_SET_FIELD_MAX	(__NFTA_SET_FIELD_MAX - 1)
+
 /**
  * enum nft_set_attributes - nf_tables set netlink attributes
  *
@@ -370,6 +385,7 @@ enum nft_set_elem_flags {
  * @NFTA_SET_ELEM_USERDATA: user data (NLA_BINARY)
  * @NFTA_SET_ELEM_EXPR: expression (NLA_NESTED: nft_expr_attributes)
  * @NFTA_SET_ELEM_OBJREF: stateful object reference (NLA_STRING)
+ * @NFTA_SET_ELEM_KEY_END: closing key value (NLA_NESTED: nft_data)
  */
 enum nft_set_elem_attributes {
 	NFTA_SET_ELEM_UNSPEC,
@@ -382,6 +398,7 @@ enum nft_set_elem_attributes {
 	NFTA_SET_ELEM_EXPR,
 	NFTA_SET_ELEM_PAD,
 	NFTA_SET_ELEM_OBJREF,
+	NFTA_SET_ELEM_KEY_END,
 	__NFTA_SET_ELEM_MAX
 };
 #define NFTA_SET_ELEM_MAX	(__NFTA_SET_ELEM_MAX - 1)
-- 
2.24.1

[PATCH libnftnl v4 2/3] set: Add support for NFTA_SET_DESC_CONCAT attributes

[Stefano Brivio]

If NFTNL_SET_DESC_CONCAT data is passed, pass that to the kernel
as NFTA_SET_DESC_CONCAT attributes: it describes the length of
single concatenated fields, in bytes.

Similarly, parse NFTA_SET_DESC_CONCAT attributes if received
from the kernel.

This is the libnftnl counterpart for nftables patch:
  src: Add support for NFTNL_SET_DESC_CONCAT

v4:
 - move NFTNL_SET_DESC_CONCAT to the end of enum nftnl_set_attr
   to avoid ABI breakage (Pablo Neira Ayuso)
v3:
 - use NFTNL_SET_DESC_CONCAT and NFTA_SET_DESC_CONCAT instead of a
   stand-alone NFTA_SET_SUBKEY attribute (Pablo Neira Ayuso)
 - pass field length in bytes instead of bits, fields would get
   unnecessarily big otherwise
v2:
 - fixed grammar in commit message
 - removed copy of array bytes in nftnl_set_nlmsg_build_subkey_payload(),
   we're simply passing values to htonl() (Phil Sutter)

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 include/libnftnl/set.h |   1 +
 include/set.h          |   2 +
 src/set.c              | 111 ++++++++++++++++++++++++++++++++++-------
 3 files changed, 95 insertions(+), 19 deletions(-)

diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
index db3fa686d60a..bbbf58ded99a 100644
--- a/include/libnftnl/set.h
+++ b/include/libnftnl/set.h
@@ -29,6 +29,7 @@ enum nftnl_set_attr {
 	NFTNL_SET_USERDATA,
 	NFTNL_SET_OBJ_TYPE,
 	NFTNL_SET_HANDLE,
+	NFTNL_SET_DESC_CONCAT,
 	__NFTNL_SET_MAX
 };
 #define NFTNL_SET_MAX (__NFTNL_SET_MAX - 1)
diff --git a/include/set.h b/include/set.h
index 446acd24ca7c..895ffdb48bdb 100644
--- a/include/set.h
+++ b/include/set.h
@@ -25,6 +25,8 @@ struct nftnl_set {
 	enum nft_set_policies	policy;
 	struct {
 		uint32_t	size;
+		uint8_t		field_len[NFT_REG32_COUNT];
+		uint8_t		field_count;
 	} desc;
 	struct list_head	element_list;
 
diff --git a/src/set.c b/src/set.c
index 78447c676f51..651dcfa56022 100644
--- a/src/set.c
+++ b/src/set.c
@@ -89,6 +89,7 @@ void nftnl_set_unset(struct nftnl_set *s, uint16_t attr)
 	case NFTNL_SET_ID:
 	case NFTNL_SET_POLICY:
 	case NFTNL_SET_DESC_SIZE:
+	case NFTNL_SET_DESC_CONCAT:
 	case NFTNL_SET_TIMEOUT:
 	case NFTNL_SET_GC_INTERVAL:
 		break;
@@ -174,6 +175,10 @@ int nftnl_set_set_data(struct nftnl_set *s, uint16_t attr, const void *data,
 	case NFTNL_SET_DESC_SIZE:
 		memcpy(&s->desc.size, data, sizeof(s->desc.size));
 		break;
+	case NFTNL_SET_DESC_CONCAT:
+		memcpy(&s->desc.field_len, data, data_len);
+		while (s->desc.field_len[++s->desc.field_count]);
+		break;
 	case NFTNL_SET_TIMEOUT:
 		memcpy(&s->timeout, data, sizeof(s->timeout));
 		break;
@@ -266,6 +271,9 @@ const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 	case NFTNL_SET_DESC_SIZE:
 		*data_len = sizeof(uint32_t);
 		return &s->desc.size;
+	case NFTNL_SET_DESC_CONCAT:
+		*data_len = s->desc.field_count;
+		return s->desc.field_len;
 	case NFTNL_SET_TIMEOUT:
 		*data_len = sizeof(uint64_t);
 		return &s->timeout;
@@ -351,13 +359,42 @@ err:
 	return NULL;
 }
 
+static void nftnl_set_nlmsg_build_desc_size_payload(struct nlmsghdr *nlh,
+						    struct nftnl_set *s)
+{
+	mnl_attr_put_u32(nlh, NFTA_SET_DESC_SIZE, htonl(s->desc.size));
+}
+
+static void nftnl_set_nlmsg_build_desc_concat_payload(struct nlmsghdr *nlh,
+						      struct nftnl_set *s)
+{
+	struct nlattr *nest;
+	int i;
+
+	nest = mnl_attr_nest_start(nlh, NFTA_SET_DESC_CONCAT);
+	for (i = 0; i < NFT_REG32_COUNT && i < s->desc.field_count; i++) {
+		struct nlattr *nest_elem;
+
+		nest_elem = mnl_attr_nest_start(nlh, NFTA_LIST_ELEM);
+		mnl_attr_put_u32(nlh, NFTA_SET_FIELD_LEN,
+				 htonl(s->desc.field_len[i]));
+		mnl_attr_nest_end(nlh, nest_elem);
+	}
+	mnl_attr_nest_end(nlh, nest);
+}
+
 static void
 nftnl_set_nlmsg_build_desc_payload(struct nlmsghdr *nlh, struct nftnl_set *s)
 {
 	struct nlattr *nest;
 
 	nest = mnl_attr_nest_start(nlh, NFTA_SET_DESC);
-	mnl_attr_put_u32(nlh, NFTA_SET_DESC_SIZE, htonl(s->desc.size));
+
+	if (s->flags & (1 << NFTNL_SET_DESC_SIZE))
+		nftnl_set_nlmsg_build_desc_size_payload(nlh, s);
+	if (s->flags & (1 << NFTNL_SET_DESC_CONCAT))
+		nftnl_set_nlmsg_build_desc_concat_payload(nlh, s);
+
 	mnl_attr_nest_end(nlh, nest);
 }
 
@@ -387,7 +424,7 @@ void nftnl_set_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_set *s)
 		mnl_attr_put_u32(nlh, NFTA_SET_ID, htonl(s->id));
 	if (s->flags & (1 << NFTNL_SET_POLICY))
 		mnl_attr_put_u32(nlh, NFTA_SET_POLICY, htonl(s->policy));
-	if (s->flags & (1 << NFTNL_SET_DESC_SIZE))
+	if (s->flags & (1 << NFTNL_SET_DESC_SIZE | 1 << NFTNL_SET_DESC_CONCAT))
 		nftnl_set_nlmsg_build_desc_payload(nlh, s);
 	if (s->flags & (1 << NFTNL_SET_TIMEOUT))
 		mnl_attr_put_u64(nlh, NFTA_SET_TIMEOUT, htobe64(s->timeout));
@@ -445,39 +482,75 @@ static int nftnl_set_parse_attr_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
-static int nftnl_set_desc_parse_attr_cb(const struct nlattr *attr, void *data)
+static int
+nftnl_set_desc_concat_field_parse_attr_cb(const struct nlattr *attr, void *data)
+{
+	int type = mnl_attr_get_type(attr);
+	struct nftnl_set *s = data;
+
+	if (type != NFTA_SET_FIELD_LEN)
+		return MNL_CB_OK;
+
+	if (mnl_attr_validate(attr, MNL_TYPE_U32))
+		return MNL_CB_ERROR;
+
+	s->desc.field_len[s->desc.field_count] = ntohl(mnl_attr_get_u32(attr));
+	s->desc.field_count++;
+
+	return MNL_CB_OK;
+}
+
+static int
+nftnl_set_desc_concat_parse_attr_cb(const struct nlattr *attr, void *data)
 {
-	const struct nlattr **tb = data;
 	int type = mnl_attr_get_type(attr);
+	struct nftnl_set *s = data;
+
+	if (type != NFTA_LIST_ELEM)
+		return MNL_CB_OK;
+
+	return mnl_attr_parse_nested(attr,
+				     nftnl_set_desc_concat_field_parse_attr_cb,
+				     s);
+}
+
+static int nftnl_set_desc_parse_attr_cb(const struct nlattr *attr, void *data)
+{
+	int type = mnl_attr_get_type(attr), err;
+	struct nftnl_set *s = data;
 
 	if (mnl_attr_type_valid(attr, NFTA_SET_DESC_MAX) < 0)
 		return MNL_CB_OK;
 
 	switch (type) {
 	case NFTA_SET_DESC_SIZE:
-		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
+		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) {
 			abi_breakage();
+			break;
+		}
+
+		s->desc.size = ntohl(mnl_attr_get_u32(attr));
+		s->flags |= (1 << NFTNL_SET_DESC_SIZE);
+		break;
+	case NFTA_SET_DESC_CONCAT:
+		err = mnl_attr_parse_nested(attr,
+					    nftnl_set_desc_concat_parse_attr_cb,
+					    s);
+		if (err != MNL_CB_OK)
+			abi_breakage();
+
+		s->flags |= (1 << NFTNL_SET_DESC_CONCAT);
+		break;
+	default:
 		break;
 	}
 
-	tb[type] = attr;
 	return MNL_CB_OK;
 }
 
-static int nftnl_set_desc_parse(struct nftnl_set *s,
-			      const struct nlattr *attr)
+static int nftnl_set_desc_parse(struct nftnl_set *s, const struct nlattr *attr)
 {
-	struct nlattr *tb[NFTA_SET_DESC_MAX + 1] = {};
-
-	if (mnl_attr_parse_nested(attr, nftnl_set_desc_parse_attr_cb, tb) < 0)
-		return -1;
-
-	if (tb[NFTA_SET_DESC_SIZE]) {
-		s->desc.size = ntohl(mnl_attr_get_u32(tb[NFTA_SET_DESC_SIZE]));
-		s->flags |= (1 << NFTNL_SET_DESC_SIZE);
-	}
-
-	return 0;
+	return mnl_attr_parse_nested(attr, nftnl_set_desc_parse_attr_cb, s);
 }
 
 EXPORT_SYMBOL(nftnl_set_nlmsg_parse);
-- 
2.24.1

[PATCH libnftnl v4 3/3] set_elem: Introduce support for NFTNL_SET_ELEM_KEY_END

[Stefano Brivio]

The new set element attribute maps to the netlink attribute
NFTA_SET_ELEM_KEY_END in the same way as NFTNL_SET_ELEM_KEY
maps to NFTA_SET_ELEM_KEY, and represents the key data used
to express the upper bound of a range, in concatenations.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v4: No changes
v3: New patch

 include/libnftnl/set.h |  1 +
 include/set_elem.h     |  1 +
 src/set_elem.c         | 24 ++++++++++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
index bbbf58ded99a..6843adfa0c1e 100644
--- a/include/libnftnl/set.h
+++ b/include/libnftnl/set.h
@@ -105,6 +105,7 @@ enum {
 	NFTNL_SET_ELEM_USERDATA,
 	NFTNL_SET_ELEM_EXPR,
 	NFTNL_SET_ELEM_OBJREF,
+	NFTNL_SET_ELEM_KEY_END,
 	__NFTNL_SET_ELEM_MAX
 };
 #define NFTNL_SET_ELEM_MAX (__NFTNL_SET_ELEM_MAX - 1)
diff --git a/include/set_elem.h b/include/set_elem.h
index cc4d52943272..52f185aa11be 100644
--- a/include/set_elem.h
+++ b/include/set_elem.h
@@ -8,6 +8,7 @@ struct nftnl_set_elem {
 	uint32_t		set_elem_flags;
 	uint32_t		flags;
 	union nftnl_data_reg	key;
+	union nftnl_data_reg	key_end;
 	union nftnl_data_reg	data;
 	struct nftnl_expr	*expr;
 	uint64_t		timeout;
diff --git a/src/set_elem.c b/src/set_elem.c
index d3ce807d838c..22031938ebbc 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -75,6 +75,7 @@ void nftnl_set_elem_unset(struct nftnl_set_elem *s, uint16_t attr)
 		break;
 	case NFTNL_SET_ELEM_FLAGS:
 	case NFTNL_SET_ELEM_KEY:	/* NFTA_SET_ELEM_KEY */
+	case NFTNL_SET_ELEM_KEY_END:	/* NFTA_SET_ELEM_KEY_END */
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
 	case NFTNL_SET_ELEM_DATA:	/* NFTA_SET_ELEM_DATA */
 	case NFTNL_SET_ELEM_TIMEOUT:	/* NFTA_SET_ELEM_TIMEOUT */
@@ -118,6 +119,10 @@ int nftnl_set_elem_set(struct nftnl_set_elem *s, uint16_t attr,
 		memcpy(&s->key.val, data, data_len);
 		s->key.len = data_len;
 		break;
+	case NFTNL_SET_ELEM_KEY_END:	/* NFTA_SET_ELEM_KEY_END */
+		memcpy(&s->key_end.val, data, data_len);
+		s->key_end.len = data_len;
+		break;
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
 		memcpy(&s->data.verdict, data, sizeof(s->data.verdict));
 		break;
@@ -193,6 +198,9 @@ const void *nftnl_set_elem_get(struct nftnl_set_elem *s, uint16_t attr, uint32_t
 	case NFTNL_SET_ELEM_KEY:	/* NFTA_SET_ELEM_KEY */
 		*data_len = s->key.len;
 		return &s->key.val;
+	case NFTNL_SET_ELEM_KEY_END:	/* NFTA_SET_ELEM_KEY_END */
+		*data_len = s->key_end.len;
+		return &s->key_end.val;
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
 		*data_len = sizeof(s->data.verdict);
 		return &s->data.verdict;
@@ -287,6 +295,14 @@ void nftnl_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh,
 		mnl_attr_put(nlh, NFTA_DATA_VALUE, e->key.len, e->key.val);
 		mnl_attr_nest_end(nlh, nest1);
 	}
+	if (e->flags & (1 << NFTNL_SET_ELEM_KEY_END)) {
+		struct nlattr *nest1;
+
+		nest1 = mnl_attr_nest_start(nlh, NFTA_SET_ELEM_KEY_END);
+		mnl_attr_put(nlh, NFTA_DATA_VALUE, e->key_end.len,
+			     e->key_end.val);
+		mnl_attr_nest_end(nlh, nest1);
+	}
 	if (e->flags & (1 << NFTNL_SET_ELEM_VERDICT)) {
 		struct nlattr *nest1, *nest2;
 
@@ -373,6 +389,7 @@ static int nftnl_set_elem_parse_attr_cb(const struct nlattr *attr, void *data)
 			abi_breakage();
 		break;
 	case NFTA_SET_ELEM_KEY:
+	case NFTA_SET_ELEM_KEY_END:
 	case NFTA_SET_ELEM_DATA:
 	case NFTA_SET_ELEM_EXPR:
 		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
@@ -421,6 +438,13 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 			goto out_set_elem;
 		e->flags |= (1 << NFTNL_SET_ELEM_KEY);
         }
+	if (tb[NFTA_SET_ELEM_KEY_END]) {
+		ret = nftnl_parse_data(&e->key_end, tb[NFTA_SET_ELEM_KEY_END],
+				       &type);
+		if (ret < 0)
+			goto out_set_elem;
+		e->flags |= (1 << NFTNL_SET_ELEM_KEY_END);
+	}
         if (tb[NFTA_SET_ELEM_DATA]) {
 		ret = nftnl_parse_data(&e->data, tb[NFTA_SET_ELEM_DATA], &type);
 		if (ret < 0)
-- 
2.24.1

Re: [PATCH libnftnl v4 0/3] Attributes for concatenated ranges

[Pablo Neira Ayuso]

On Thu, Jan 30, 2020 at 01:16:31AM +0100, Stefano Brivio wrote:
> This series adds support for NFTA_SET_DESC_CONCAT set attribute and
> the additional key passed as NFTA_SET_ELEM_KEY_END to denote the
> upper bound of a range in a generic way, as suggested by Pablo.

Applied, thanks.