All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2 libnftnl v2] src: use uint64_t for flags fields
@ 2025-05-27 19:34 Fernando Fernandez Mancera
  2025-05-27 19:34 ` [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options Fernando Fernandez Mancera
  0 siblings, 1 reply; 12+ messages in thread
From: Fernando Fernandez Mancera @ 2025-05-27 19:34 UTC (permalink / raw)
  To: netfilter-devel; +Cc: coreteam, Fernando Fernandez Mancera

The flags for the object tunnel already reach out 31, therefore, in
order to be able to extend the flags field must be uint64_t. Otherwise,
we will shift by more of the type size.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/obj.h   |  2 +-
 include/rule.h  |  2 +-
 include/set.h   |  2 +-
 include/utils.h |  2 +-
 src/chain.c     |  2 +-
 src/flowtable.c |  2 +-
 src/object.c    | 10 +++++-----
 src/table.c     |  2 +-
 src/utils.c     |  2 +-
 9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/obj.h b/include/obj.h
index d217737..fc78e2a 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -19,7 +19,7 @@ struct nftnl_obj {
 	uint32_t		family;
 	uint32_t		use;
 
-	uint32_t		flags;
+	uint64_t		flags;
 	uint64_t		handle;
 
 	struct {
diff --git a/include/rule.h b/include/rule.h
index 036c722..6432786 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -4,7 +4,7 @@
 struct nftnl_rule {
 	struct list_head head;
 
-	uint32_t	flags;
+	uint64_t	flags;
 	uint32_t	family;
 	const char	*table;
 	const char	*chain;
diff --git a/include/set.h b/include/set.h
index 55018b6..179f6ad 100644
--- a/include/set.h
+++ b/include/set.h
@@ -30,7 +30,7 @@ struct nftnl_set {
 	} desc;
 	struct list_head	element_list;
 
-	uint32_t		flags;
+	uint64_t		flags;
 	uint32_t		gc_interval;
 	uint64_t		timeout;
 	struct list_head	expr_list;
diff --git a/include/utils.h b/include/utils.h
index eed6127..5da2ddb 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -79,7 +79,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
 			  	     uint32_t cmd, uint32_t type,
 				     uint32_t flags));
 
-int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
+int nftnl_set_str_attr(const char **dptr, uint64_t *flags,
 		       uint16_t attr, const void *data, uint32_t data_len);
 
 #endif
diff --git a/src/chain.c b/src/chain.c
index 895108c..a9e18dc 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -43,7 +43,7 @@ struct nftnl_chain {
 	uint64_t	packets;
 	uint64_t	bytes;
 	uint64_t	handle;
-	uint32_t	flags;
+	uint64_t	flags;
 	uint32_t	chain_id;
 
 	struct {
diff --git a/src/flowtable.c b/src/flowtable.c
index fbbe0a8..c52ba0e 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -29,7 +29,7 @@ struct nftnl_flowtable {
 	struct nftnl_str_array	dev_array;
 	uint32_t		ft_flags;
 	uint32_t		use;
-	uint32_t		flags;
+	uint64_t		flags;
 	uint64_t		handle;
 };
 
diff --git a/src/object.c b/src/object.c
index bfcceb9..f307815 100644
--- a/src/object.c
+++ b/src/object.c
@@ -62,13 +62,13 @@ void nftnl_obj_free(const struct nftnl_obj *obj)
 EXPORT_SYMBOL(nftnl_obj_is_set);
 bool nftnl_obj_is_set(const struct nftnl_obj *obj, uint16_t attr)
 {
-	return obj->flags & (1 << attr);
+	return obj->flags & (1ULL << attr);
 }
 
 EXPORT_SYMBOL(nftnl_obj_unset);
 void nftnl_obj_unset(struct nftnl_obj *obj, uint16_t attr)
 {
-	if (!(obj->flags & (1 << attr)))
+	if (!(obj->flags & (1ULL << attr)))
 		return;
 
 	switch (attr) {
@@ -90,7 +90,7 @@ void nftnl_obj_unset(struct nftnl_obj *obj, uint16_t attr)
 		break;
 	}
 
-	obj->flags &= ~(1 << attr);
+	obj->flags &= ~(1ULL << attr);
 }
 
 static uint32_t nftnl_obj_validate[NFTNL_OBJ_MAX + 1] = {
@@ -153,7 +153,7 @@ int nftnl_obj_set_data(struct nftnl_obj *obj, uint16_t attr,
 		if (obj->ops->set(obj, attr, data, data_len) < 0)
 			return -1;
 	}
-	obj->flags |= (1 << attr);
+	obj->flags |= (1ULL << attr);
 	return 0;
 }
 
@@ -197,7 +197,7 @@ EXPORT_SYMBOL(nftnl_obj_get_data);
 const void *nftnl_obj_get_data(const struct nftnl_obj *obj, uint16_t attr,
 			       uint32_t *data_len)
 {
-	if (!(obj->flags & (1 << attr)))
+	if (!(obj->flags & (1ULL << attr)))
 		return NULL;
 
 	switch(attr) {
diff --git a/src/table.c b/src/table.c
index 9870dca..e183e2e 100644
--- a/src/table.c
+++ b/src/table.c
@@ -29,7 +29,7 @@ struct nftnl_table {
 	uint32_t	table_flags;
 	uint64_t 	handle;
 	uint32_t	use;
-	uint32_t	flags;
+	uint64_t	flags;
 	uint32_t	owner;
 	struct {
 		void		*data;
diff --git a/src/utils.c b/src/utils.c
index 5f2c5bf..7942d67 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -133,7 +133,7 @@ void __noreturn __abi_breakage(const char *file, int line, const char *reason)
        exit(EXIT_FAILURE);
 }
 
-int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
+int nftnl_set_str_attr(const char **dptr, uint64_t *flags,
 		       uint16_t attr, const void *data, uint32_t data_len)
 {
 	if (*flags & (1 << attr))
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-27 19:34 [PATCH 1/2 libnftnl v2] src: use uint64_t for flags fields Fernando Fernandez Mancera
@ 2025-05-27 19:34 ` Fernando Fernandez Mancera
  2025-05-28  0:34   ` Florian Westphal
  2025-05-28 10:55   ` Florian Westphal
  0 siblings, 2 replies; 12+ messages in thread
From: Fernando Fernandez Mancera @ 2025-05-27 19:34 UTC (permalink / raw)
  To: netfilter-devel; +Cc: coreteam, Fernando Fernandez Mancera

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/libnftnl/object.h |   7 ++
 include/obj.h             |   9 +++
 src/libnftnl.map          |   4 +
 src/obj/tunnel.c          | 156 ++++++++++++++++++++++++++++++--------
 src/object.c              |  10 +++
 5 files changed, 156 insertions(+), 30 deletions(-)

diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
index 9930355..14a42cd 100644
--- a/include/libnftnl/object.h
+++ b/include/libnftnl/object.h
@@ -117,15 +117,19 @@ enum {
 	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
 	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
 	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
+	NFTNL_OBJ_TUNNEL_GENEVE_OPTS,
 	__NFTNL_OBJ_TUNNEL_MAX,
 };
 
+#define NFTNL_TUNNEL_GENEVE_DATA_MAXLEN	127
+
 enum {
 	NFTNL_OBJ_SECMARK_CTX	= NFTNL_OBJ_BASE,
 	__NFTNL_OBJ_SECMARK_MAX,
 };
 
 struct nftnl_obj;
+struct nftnl_obj_tunnel_geneve;
 
 struct nftnl_obj *nftnl_obj_alloc(void);
 void nftnl_obj_free(const struct nftnl_obj *ne);
@@ -171,6 +175,9 @@ void nftnl_obj_list_del(struct nftnl_obj *t);
 int nftnl_obj_list_foreach(struct nftnl_obj_list *table_list,
 			   int (*cb)(struct nftnl_obj *t, void *data),
 			   void *data);
+int nftnl_obj_tunnel_geneve_opts_foreach(struct nftnl_obj *e,
+					 int (*cb)(struct nftnl_obj_tunnel_geneve *e, void *data),
+					 void *data);
 
 struct nftnl_obj_list_iter;
 struct nftnl_obj_list_iter *nftnl_obj_list_iter_create(struct nftnl_obj_list *l);
diff --git a/include/obj.h b/include/obj.h
index fc78e2a..7c9898e 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -9,6 +9,14 @@ struct nlattr;
 struct nlmsghdr;
 struct nftnl_obj;
 
+struct nftnl_obj_tunnel_geneve {
+	struct list_head	list;
+	uint16_t		geneve_class;
+	uint8_t			type;
+	uint8_t			data[NFTNL_TUNNEL_GENEVE_DATA_MAXLEN];
+	uint32_t		data_len;
+};
+
 struct nftnl_obj {
 	struct list_head	head;
 	struct obj_ops		*ops;
@@ -92,6 +100,7 @@ struct nftnl_obj {
 						} v2;
 					} u;
 				} tun_erspan;
+				struct list_head	tun_geneve;
 			} u;
 		} tunnel;
 		struct nftnl_obj_secmark {
diff --git a/src/libnftnl.map b/src/libnftnl.map
index 8fffff1..e125017 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -383,3 +383,7 @@ LIBNFTNL_16 {
 LIBNFTNL_17 {
   nftnl_set_elem_nlmsg_build;
 } LIBNFTNL_16;
+
+LIBNFTNL_18 {
+  nftnl_obj_tunnel_geneve_opts_foreach;
+} LIBNFTNL_17;
diff --git a/src/obj/tunnel.c b/src/obj/tunnel.c
index 8941e39..b8c07b7 100644
--- a/src/obj/tunnel.c
+++ b/src/obj/tunnel.c
@@ -22,6 +22,7 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
 		     const void *data, uint32_t data_len)
 {
 	struct nftnl_obj_tunnel *tun = nftnl_obj_data(e);
+	struct nftnl_obj_tunnel_geneve *geneve;
 
 	switch (type) {
 	case NFTNL_OBJ_TUNNEL_ID:
@@ -72,6 +73,15 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
 		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
 		break;
+	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
+		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
+		memcpy(geneve, data, data_len);
+
+		if (!(e->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS)))
+			INIT_LIST_HEAD(&tun->u.tun_geneve);
+
+		list_add_tail(&geneve->list, &tun->u.tun_geneve);
+		break;
 	}
 	return 0;
 }
@@ -135,6 +145,23 @@ nftnl_obj_tunnel_get(const struct nftnl_obj *e, uint16_t type,
 	return NULL;
 }
 
+EXPORT_SYMBOL(nftnl_obj_tunnel_geneve_opts_foreach);
+int nftnl_obj_tunnel_geneve_opts_foreach(struct nftnl_obj *e,
+					 int (*cb)(struct nftnl_obj_tunnel_geneve *e, void *data),
+					 void *data)
+{
+	struct nftnl_obj_tunnel *tun = nftnl_obj_data(e);
+	struct nftnl_obj_tunnel_geneve *cur, *tmp;
+	int ret;
+
+	list_for_each_entry_safe(cur, tmp, &tun->u.tun_geneve, list) {
+		ret = cb(cur, data);
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
+}
+
 static int nftnl_obj_tunnel_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -240,6 +267,23 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 		mnl_attr_nest_end(nlh, nest_inner);
 		mnl_attr_nest_end(nlh, nest);
 	}
+	if (e->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS)) {
+		struct nftnl_obj_tunnel_geneve *geneve;
+
+		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+		list_for_each_entry(geneve, &tun->u.tun_geneve, list) {
+			nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_GENEVE);
+			mnl_attr_put_u16(nlh, NFTA_TUNNEL_KEY_GENEVE_CLASS,
+					 htons(geneve->geneve_class));
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_GENEVE_TYPE,
+					geneve->type);
+			mnl_attr_put(nlh, NFTA_TUNNEL_KEY_GENEVE_DATA,
+				     geneve->data_len,
+				     geneve->data);
+			mnl_attr_nest_end(nlh, nest_inner);
+		}
+		mnl_attr_nest_end(nlh, nest);
+	}
 }
 
 static int nftnl_obj_tunnel_ip_cb(const struct nlattr *attr, void *data)
@@ -335,6 +379,72 @@ static int nftnl_obj_tunnel_parse_ip6(struct nftnl_obj *e, struct nlattr *attr,
 	return 0;
 }
 
+static int nftnl_obj_tunnel_geneve_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, NFTA_TUNNEL_KEY_GENEVE_MAX) < 0)
+		return MNL_CB_OK;
+
+	switch (type) {
+	case NFTA_TUNNEL_KEY_GENEVE_CLASS:
+		if (mnl_attr_validate(attr, MNL_TYPE_U16) < 0)
+			abi_breakage();
+		break;
+	case NFTA_TUNNEL_KEY_GENEVE_TYPE:
+		if (mnl_attr_validate(attr, MNL_TYPE_U8) < 0)
+			abi_breakage();
+		break;
+	case NFTA_TUNNEL_KEY_GENEVE_DATA:
+		if (mnl_attr_validate(attr, MNL_TYPE_BINARY) < 0)
+			abi_breakage();
+		break;
+	}
+
+	tb[type] = attr;
+	return MNL_CB_OK;
+}
+
+static int
+nftnl_obj_tunnel_parse_geneve(struct nftnl_obj *e, struct nlattr *attr,
+			      struct nftnl_obj_tunnel *tun)
+{
+	struct nlattr *tb[NFTA_TUNNEL_KEY_GENEVE_MAX + 1] = {};
+	struct nftnl_obj_tunnel_geneve *geneve;
+
+	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_geneve_cb, tb) < 0)
+		return -1;
+
+	geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
+
+	if (!(e->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS))) {
+		INIT_LIST_HEAD(&tun->u.tun_geneve);
+		e->flags |= (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS);
+	}
+
+	if (tb[NFTA_TUNNEL_KEY_GENEVE_CLASS])
+		geneve->geneve_class =
+			ntohs(mnl_attr_get_u16(tb[NFTA_TUNNEL_KEY_GENEVE_CLASS]));
+
+	if (tb[NFTA_TUNNEL_KEY_GENEVE_TYPE])
+		geneve->type =
+			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_GENEVE_TYPE]);
+
+	if (tb[NFTA_TUNNEL_KEY_GENEVE_DATA]) {
+		uint32_t len = mnl_attr_get_payload_len(tb[NFTA_TUNNEL_KEY_GENEVE_DATA]);
+
+		memcpy(geneve->data,
+		       mnl_attr_get_payload(tb[NFTA_TUNNEL_KEY_GENEVE_DATA]),
+		       len);
+		geneve->data_len = len;
+	}
+
+	list_add_tail(&geneve->list, &tun->u.tun_geneve);
+
+	return 0;
+}
+
 static int nftnl_obj_tunnel_vxlan_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -430,42 +540,28 @@ nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
 	return 0;
 }
 
-static int nftnl_obj_tunnel_opts_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, NFTA_TUNNEL_KEY_OPTS_MAX) < 0)
-		return MNL_CB_OK;
-
-	switch (type) {
-	case NFTA_TUNNEL_KEY_OPTS_VXLAN:
-	case NFTA_TUNNEL_KEY_OPTS_ERSPAN:
-		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
-			abi_breakage();
-		break;
-	}
-
-	tb[type] = attr;
-	return MNL_CB_OK;
-}
-
 static int
-nftnl_obj_tunnel_parse_opts(struct nftnl_obj *e, struct nlattr *attr,
+nftnl_obj_tunnel_parse_opts(struct nftnl_obj *e, struct nlattr *nest,
 			    struct nftnl_obj_tunnel *tun)
 {
-	struct nlattr *tb[NFTA_TUNNEL_KEY_OPTS_MAX + 1] = {};
+	struct nlattr *attr;
 	int err = 0;
 
-	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_opts_cb, tb) < 0)
-		return -1;
+	mnl_attr_for_each_nested(attr, nest) {
+		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
+			abi_breakage();
 
-	if (tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]) {
-		err = nftnl_obj_tunnel_parse_vxlan(e, tb[NFTA_TUNNEL_KEY_OPTS_VXLAN],
-						   tun);
-	} else if (tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]) {
-		err = nftnl_obj_tunnel_parse_erspan(e, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN],
-						    tun);
+		switch (mnl_attr_get_type(attr)) {
+			case NFTA_TUNNEL_KEY_OPTS_VXLAN:
+				err = nftnl_obj_tunnel_parse_vxlan(e, attr, tun);
+				break;
+			case NFTA_TUNNEL_KEY_OPTS_ERSPAN:
+				err = nftnl_obj_tunnel_parse_erspan(e, attr, tun);
+				break;
+			case NFTA_TUNNEL_KEY_OPTS_GENEVE:
+				err = nftnl_obj_tunnel_parse_geneve(e, attr, tun);
+				break;
+		}
 	}
 
 	return err;
diff --git a/src/object.c b/src/object.c
index f307815..b5bdbbf 100644
--- a/src/object.c
+++ b/src/object.c
@@ -49,12 +49,22 @@ struct nftnl_obj *nftnl_obj_alloc(void)
 EXPORT_SYMBOL(nftnl_obj_free);
 void nftnl_obj_free(const struct nftnl_obj *obj)
 {
+	struct nftnl_obj_tunnel_geneve *geneve, *next;
+
 	if (obj->flags & (1 << NFTNL_OBJ_TABLE))
 		xfree(obj->table);
 	if (obj->flags & (1 << NFTNL_OBJ_NAME))
 		xfree(obj->name);
 	if (obj->flags & (1 << NFTNL_OBJ_USERDATA))
 		xfree(obj->user.data);
+	if (obj->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS)) {
+		list_for_each_entry_safe(geneve, next,
+					 &obj->data.tunnel.u.tun_geneve,
+					 list) {
+			list_del(&geneve->list);
+			xfree(geneve);
+		}
+	}
 
 	xfree(obj);
 }
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-27 19:34 ` [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options Fernando Fernandez Mancera
@ 2025-05-28  0:34   ` Florian Westphal
  2025-05-28  9:27     ` Fernando Fernandez Mancera
  2025-06-10  0:28     ` Pablo Neira Ayuso
  2025-05-28 10:55   ` Florian Westphal
  1 sibling, 2 replies; 12+ messages in thread
From: Florian Westphal @ 2025-05-28  0:34 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel, coreteam

Fernando Fernandez Mancera <fmancera@suse.de> wrote:

Hi Fernando

Thanks for working on this, I got inquiries as to nft_tunnel.c
and how to make use of this stuff...

> diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
> index 9930355..14a42cd 100644
> --- a/include/libnftnl/object.h
> +++ b/include/libnftnl/object.h
> @@ -117,15 +117,19 @@ enum {
>  	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
>  	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
>  	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
> +	NFTNL_OBJ_TUNNEL_GENEVE_OPTS,

If every flavour gets its own flag in the tunnel namespace we'll run
out of u64 in no time.

AFAICS these are mutually exclusive, e.g.
NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX and NFTNL_OBJ_TUNNEL_VXLAN_GBP cannot
be active at the same time.

Is there a way to re-use the internal flag namespace depending on the tunnel
subtype?

Or to have distinct tunnel object types?

object -> tunnel -> {vxlan, erspan, ...} ?

As-is, how is this API supposed to be used?  The internal union seems to
be asking for trouble later, when e.g. 'getting' NFTNL_OBJ_TUNNEL_GENEVE_OPTS
on something that was instantiated as vxlan tunnel and fields aliasing to
unexpected values.

Perhaps the first use of any of the NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX
etc values in a setter should interally "lock" the object to the given
subtype?

That might allow to NOT use ->flags for those enum values and instead
keep track of them via overlapping bits.

We'd need some internal 'enum nft_obj_tunnel_type' that marks which
part of the union is useable/instantiated so we can reject requests
to set bits that are not available for the specific tunnel type.

>  	switch (type) {
>  	case NFTNL_OBJ_TUNNEL_ID:
> @@ -72,6 +73,15 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
>  	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
>  		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
>  		break;
> +	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
> +		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));

No null check.  Applies to a few other spots too.

> +		memcpy(geneve, data, data_len);

Hmm, this looks like the API leaks internal data layout from nftables to
libnftnl and vice versa?  IMO thats a non-starter, sorry.

I see that options are essentially unlimited values, so perhaps nftables
should build the netlink blob(s) directly, similar to nftnl_udata()?

Pablo, any better idea?

I don't like exposing the struct and also the list abstraction getting
exposed to libnftnl users.

As-is, there is:

json/bison -> struct tunnel_geneve -> nftnl_obj_set_data(&obj, NFTNL_OBJ_TUNNEL_GENEVE_OPTS, &blob);
 -> nftnl_obj_tunnel_build -> mnl_attr_put_u16( ...

I think it will be better if we make this internal to nftables and have
pass-through for the entire geneve blob,i.e.

json/bison -> struct tunnel_geneve ->
 (re)alloc buffer: mnl_nlmsg_put_header(), mnl_attr_put_u16 -> etc.
 nftnl_obj_set_data(obj, NFTNL_OBJ_TUNNEL_GENEVE_OPTS, blob) ->
 nftnl_obj_tunnel_build -> memcpy into NFTA_TUNNEL_KEY_OPTS nest
 container.

Less elegant but I have a hard time coming up with an api that is
extensible and doesn't need shared binary structures.

Another option would be to use udata in the frontend and then
make libnftnl xlate the udata to netlink proper.  But I don't
like this because a) udata is supposed to be "DO NOT TOUCH"
and b) it means doubling the work compared to just doing mnl_attr_put()
calls in nftables.

> +	if (tb[NFTA_TUNNEL_KEY_GENEVE_DATA]) {
> +		uint32_t len = mnl_attr_get_payload_len(tb[NFTA_TUNNEL_KEY_GENEVE_DATA]);
> +
> +		memcpy(geneve->data,
> +		       mnl_attr_get_payload(tb[NFTA_TUNNEL_KEY_GENEVE_DATA]),
> +		       len);

This should cap 'len' by sizeof(geneve->data).
But I'm not sure this deserialization should be done in libnftnl in the
first place, see above.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-28  0:34   ` Florian Westphal
@ 2025-05-28  9:27     ` Fernando Fernandez Mancera
  2025-05-28 10:52       ` Florian Westphal
  2025-06-10  0:28     ` Pablo Neira Ayuso
  1 sibling, 1 reply; 12+ messages in thread
From: Fernando Fernandez Mancera @ 2025-05-28  9:27 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, coreteam



On 5/28/25 2:34 AM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> 
> Hi Fernando
> 
> Thanks for working on this, I got inquiries as to nft_tunnel.c
> and how to make use of this stuff...
> 
>> diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
>> index 9930355..14a42cd 100644
>> --- a/include/libnftnl/object.h
>> +++ b/include/libnftnl/object.h
>> @@ -117,15 +117,19 @@ enum {
>>   	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
>>   	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
>>   	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
>> +	NFTNL_OBJ_TUNNEL_GENEVE_OPTS,
> 
> If every flavour gets its own flag in the tunnel namespace we'll run
> out of u64 in no time.
> 
> AFAICS these are mutually exclusive, e.g.
> NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX and NFTNL_OBJ_TUNNEL_VXLAN_GBP cannot
> be active at the same time.
> 
> Is there a way to re-use the internal flag namespace depending on the tunnel
> subtype?
> 
> Or to have distinct tunnel object types?
> 
> object -> tunnel -> {vxlan, erspan, ...} ?
> 

IMHO, this could be done by providing libnftnl its own object tunnel 
API. Although that would require to expose more functions, but tunnel 
object could have its own flags field.

In addition, I am afraid that ERSPAN and VXLAN enum fields have been 
exposed in a released version so removing them would be a breaking 
change. I am not sure whether that is acceptable or not.

> As-is, how is this API supposed to be used?  The internal union seems to
> be asking for trouble later, when e.g. 'getting' NFTNL_OBJ_TUNNEL_GENEVE_OPTS
> on something that was instantiated as vxlan tunnel and fields aliasing to
> unexpected values.
> 
> Perhaps the first use of any of the NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX
> etc values in a setter should interally "lock" the object to the given
> subtype?
> 
> That might allow to NOT use ->flags for those enum values and instead
> keep track of them via overlapping bits.
> 
> We'd need some internal 'enum nft_obj_tunnel_type' that marks which
> part of the union is useable/instantiated so we can reject requests
> to set bits that are not available for the specific tunnel type.
> 

Yes, that would help but isn't that the reason why the flags field is 
there? I mean, currently the same problem would exist with the different 
object variants e.g the union of "struct nftnl_obj_*". When trying to 
get the attribute we always check that the flag is set.

>>   	switch (type) {
>>   	case NFTNL_OBJ_TUNNEL_ID:
>> @@ -72,6 +73,15 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
>>   	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
>>   		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
>>   		break;
>> +	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
>> +		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
> 
> No null check.  Applies to a few other spots too.
> 

Ah right. Yes, will fix that. Thanks.

>> +		memcpy(geneve, data, data_len);
> 
> Hmm, this looks like the API leaks internal data layout from nftables to
> libnftnl and vice versa?  IMO thats a non-starter, sorry.
> 

I agree that exposing the list abstraction to the user is problematic 
and shouldn't be done. As you mentioned below, I couldn't come up with a 
better API on libnftnl. Thinking about it, we could expose only the 
relevant fields by putting them in a isolated struct.

> I see that options are essentially unlimited values, so perhaps nftables
> should build the netlink blob(s) directly, similar to nftnl_udata()?
> 
> Pablo, any better idea?
> 
> I don't like exposing the struct and also the list abstraction getting
> exposed to libnftnl users.
> 
> As-is, there is:
> 
> json/bison -> struct tunnel_geneve -> nftnl_obj_set_data(&obj, NFTNL_OBJ_TUNNEL_GENEVE_OPTS, &blob);
>   -> nftnl_obj_tunnel_build -> mnl_attr_put_u16( ...
> 
> I think it will be better if we make this internal to nftables and have
> pass-through for the entire geneve blob,i.e.
> 
> json/bison -> struct tunnel_geneve ->
>   (re)alloc buffer: mnl_nlmsg_put_header(), mnl_attr_put_u16 -> etc.
>   nftnl_obj_set_data(obj, NFTNL_OBJ_TUNNEL_GENEVE_OPTS, blob) ->
>   nftnl_obj_tunnel_build -> memcpy into NFTA_TUNNEL_KEY_OPTS nest
>   container.
> 

I am fine with this, although just as a note I chose the linked list 
path mainly to make it more flexible for users. AFAICS, libnftnl users 
usually do not need to build the netlink blob themselves.

So, the problem in essence is the shared binary structure. If I do not 
expose the list abstraction, copying the allocated struct would still be 
problematic, right?

Thank you for all the comments, Florian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-28  9:27     ` Fernando Fernandez Mancera
@ 2025-05-28 10:52       ` Florian Westphal
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Westphal @ 2025-05-28 10:52 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel, coreteam

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> > If every flavour gets its own flag in the tunnel namespace we'll run
> > out of u64 in no time.
> > 
> > AFAICS these are mutually exclusive, e.g.
> > NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX and NFTNL_OBJ_TUNNEL_VXLAN_GBP cannot
> > be active at the same time.
> > 
> > Is there a way to re-use the internal flag namespace depending on the tunnel
> > subtype?
> > 
> > Or to have distinct tunnel object types?
> > 
> > object -> tunnel -> {vxlan, erspan, ...} ?
> > 
> 
> IMHO, this could be done by providing libnftnl its own object tunnel 
> API. Although that would require to expose more functions, but tunnel 
> object could have its own flags field.
> 
> In addition, I am afraid that ERSPAN and VXLAN enum fields have been 
> exposed in a released version so removing them would be a breaking 
> change. I am not sure whether that is acceptable or not.

Yep, looks like ERSPAN and VXLAN have to remain in place.

> > As-is, how is this API supposed to be used?  The internal union seems to
> > be asking for trouble later, when e.g. 'getting' NFTNL_OBJ_TUNNEL_GENEVE_OPTS
> > on something that was instantiated as vxlan tunnel and fields aliasing to
> > unexpected values.
> > 
> > Perhaps the first use of any of the NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX
> > etc values in a setter should interally "lock" the object to the given
> > subtype?
> > 
> > That might allow to NOT use ->flags for those enum values and instead
> > keep track of them via overlapping bits.
> > 
> > We'd need some internal 'enum nft_obj_tunnel_type' that marks which
> > part of the union is useable/instantiated so we can reject requests
> > to set bits that are not available for the specific tunnel type.
> > 
> 
> Yes, that would help but isn't that the reason why the flags field is 
> there? I mean, currently the same problem would exist with the different 
> object variants e.g the union of "struct nftnl_obj_*". When trying to 
> get the attribute we always check that the flag is set.

Right :-/

> >> +		memcpy(geneve, data, data_len);
> > 
> > Hmm, this looks like the API leaks internal data layout from nftables to
> > libnftnl and vice versa?  IMO thats a non-starter, sorry.
> > 
> 
> I agree that exposing the list abstraction to the user is problematic 
> and shouldn't be done. As you mentioned below, I couldn't come up with a 
> better API on libnftnl. Thinking about it, we could expose only the 
> relevant fields by putting them in a isolated struct.

Looking at the nftables patches, I think some of what is done there
should be moved to libnftl, e.g. the $<obj>0->tunnel.type = TUNNEL_GENEVE;
thing.  I mean, it makes sense to track the type of the object but
wouldn't it make more sense to do this in libnftnl?

Thinking about it some more, I'm not sure if NFT_OBJECT_TUNNEL was a good idea.

We have:
  NFT_OBJECT_CT_HELPER
  NFT_OBJECT_CT_TIMEOUT
  NFT_OBJECT_CT_EXPECT

What about following that model, e.g.:

#define NFT_OBJECT_TUNNEL_GENEVE	11
#define NFT_OBJECT_TUNNEL_ERSPAN	12
...

?
Pablo, what do you think?  Maybe you tried this before and it wasn't
good either because of lots of duplication with common tunnel
attributes?

Back to the geneve option handling, I think it would be best to
expose all the fields in the struct via deticated setters/getters, i.e.
from

struct tunnel_geneve {
       struct list_head        list;
       uint16_t                geneve_class;
       uint8_t                 type;
       uint8_t                 data[NFTNL_TUNNEL_GENEVE_DATA_MAXLEN];
       uint32_t                data_len;
};

expose: geneve_class, type, and data.

This could be done by moving some of the nftables code you added to
libnftnl.

Instead of this in nftables:
  list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) {
        nftnl_obj_set_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_OPTS,
                           geneve, sizeof(struct tunnel_geneve));
        }
  }

do this (error handling omitted for brevity):

  list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) {
	struct nftnl_obj_tunnel_geneve_opts *geneve;

	geneve = nftnl_obj_tunnel_geneve_opts_alloc();

	nftnl_obj_tunnel_geneve_opts_set_u32(geneve, NFT_OBJECT_TUNNEL_GENEVE_OPT_CLASS, geneve->geneve_class);
	nftnl_obj_tunnel_geneve_opts_set_u32(geneve, NFT_OBJECT_TUNNEL_GENEVE_OPT_TYPE, geneve->geneve_type);
        nftnl_obj_tunnel_geneve_opts_set_data(geneve, NFT_OBJECT_TUNNEL_GENEVE_OPT_DATA,
					      geneve->data, geneve->data_len);

        nftnl_obj_tunnel_add_geneve_opts(nlo, geneve);

(nlo/nftnl_tunnel and nftnl_obj_tunnel_geneve_opts would be similar to
 nftnl_chain vs nftnl_rule).

The major downside is a lot of more API calls that need to be added to
libnftnl :-(

I don't see any other solutions at this time.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-27 19:34 ` [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options Fernando Fernandez Mancera
  2025-05-28  0:34   ` Florian Westphal
@ 2025-05-28 10:55   ` Florian Westphal
  2025-05-28 11:31     ` Fernando Fernandez Mancera
  1 sibling, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2025-05-28 10:55 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel, coreteam

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> +	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
> +		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
> +		memcpy(geneve, data, data_len);
> +
> +		if (!(e->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS)))
> +			INIT_LIST_HEAD(&tun->u.tun_geneve);
> +
> +		list_add_tail(&geneve->list, &tun->u.tun_geneve);
> +		break;

I missed this earlier. Do we have precedence for this?

AFAIK for all existing setters, if you do

nftnl_foo_set_data(obj, OPT_FOO, &d, len);
nftnl_foo_set_data(obj, OPT_FOO, &d2, len2);

Then d2 replaces d, it doesn't silently append/add to the
object.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-28 10:55   ` Florian Westphal
@ 2025-05-28 11:31     ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 12+ messages in thread
From: Fernando Fernandez Mancera @ 2025-05-28 11:31 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, coreteam

On 5/28/25 12:55 PM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>> +	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
>> +		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
>> +		memcpy(geneve, data, data_len);
>> +
>> +		if (!(e->flags & (1ULL << NFTNL_OBJ_TUNNEL_GENEVE_OPTS)))
>> +			INIT_LIST_HEAD(&tun->u.tun_geneve);
>> +
>> +		list_add_tail(&geneve->list, &tun->u.tun_geneve);
>> +		break;
> 
> I missed this earlier. Do we have precedence for this?
> 
> AFAIK for all existing setters, if you do
> 
> nftnl_foo_set_data(obj, OPT_FOO, &d, len);
> nftnl_foo_set_data(obj, OPT_FOO, &d2, len2);
> 
> Then d2 replaces d, it doesn't silently append/add to the
> object.
> 

Good catch. I thought yes, but no, there is no precedence for this. 
Other similar situations expose a dedicated function to append items to 
a list.

So we should do the same thing on this situation.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-05-28  0:34   ` Florian Westphal
  2025-05-28  9:27     ` Fernando Fernandez Mancera
@ 2025-06-10  0:28     ` Pablo Neira Ayuso
  2025-06-10  6:01       ` Florian Westphal
  1 sibling, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2025-06-10  0:28 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Fernando Fernandez Mancera, netfilter-devel, coreteam

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

Hi,

On Wed, May 28, 2025 at 02:34:10AM +0200, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> 
> Hi Fernando
> 
> Thanks for working on this, I got inquiries as to nft_tunnel.c
> and how to make use of this stuff...
> 
> > diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
> > index 9930355..14a42cd 100644
> > --- a/include/libnftnl/object.h
> > +++ b/include/libnftnl/object.h
> > @@ -117,15 +117,19 @@ enum {
> >  	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
> >  	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
> >  	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
> > +	NFTNL_OBJ_TUNNEL_GENEVE_OPTS,
> 
> If every flavour gets its own flag in the tunnel namespace we'll run
> out of u64 in no time.
> 
> AFAICS these are mutually exclusive, e.g.
> NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX and NFTNL_OBJ_TUNNEL_VXLAN_GBP cannot
> be active at the same time.
> 
> Is there a way to re-use the internal flag namespace depending on the tunnel
> subtype?
> 
> Or to have distinct tunnel object types?
> 
> object -> tunnel -> {vxlan, erspan, ...} ?
> 
> As-is, how is this API supposed to be used?  The internal union seems to
> be asking for trouble later, when e.g. 'getting' NFTNL_OBJ_TUNNEL_GENEVE_OPTS
> on something that was instantiated as vxlan tunnel and fields aliasing to
> unexpected values.
> 
> Perhaps the first use of any of the NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX
> etc values in a setter should interally "lock" the object to the given
> subtype?
> 
> That might allow to NOT use ->flags for those enum values and instead
> keep track of them via overlapping bits.
> 
> We'd need some internal 'enum nft_obj_tunnel_type' that marks which
> part of the union is useable/instantiated so we can reject requests
> to set bits that are not available for the specific tunnel type.
> 
> >  	switch (type) {
> >  	case NFTNL_OBJ_TUNNEL_ID:
> > @@ -72,6 +73,15 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
> >  	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
> >  		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
> >  		break;
> > +	case NFTNL_OBJ_TUNNEL_GENEVE_OPTS:
> > +		geneve = malloc(sizeof(struct nftnl_obj_tunnel_geneve));
> 
> No null check.  Applies to a few other spots too.
> 
> > +		memcpy(geneve, data, data_len);
> 
> Hmm, this looks like the API leaks internal data layout from nftables to
> libnftnl and vice versa?  IMO thats a non-starter, sorry.
> 
> I see that options are essentially unlimited values, so perhaps nftables
> should build the netlink blob(s) directly, similar to nftnl_udata()?
> 
> Pablo, any better idea?

Maybe this API for tunnel options are proposed in this patch?

Consider this a sketch/proposal, this is compiled tested only.

struct obj_ops also needs a .free interface to release the tunnel
options object.

[-- Attachment #2: 0001-tunnel-rework-options.patch --]
[-- Type: text/x-diff, Size: 15594 bytes --]

From 56362a22008911873bd8b8a2f55e68406e55e0de Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 10 Jun 2025 01:43:28 +0200
Subject: [PATCH libnftnl] tunnel: rework options

Only vxlan gbp can work before this patch because
NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR is off by one in the internal object
flags.

Replace them by NFTNL_OBJ_TUNNEL_OPTS and add a new opaque
nftnl_tunnel_opts struct and nftnl_tunnel_opts_set() to set up
tunnel options.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/libnftnl/object.h |  32 ++++-
 include/obj.h             |  16 +--
 src/obj/tunnel.c          | 275 +++++++++++++++++++++++++++-----------
 3 files changed, 222 insertions(+), 101 deletions(-)

diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
index 9930355bb8f0..0331cf7ac5d8 100644
--- a/include/libnftnl/object.h
+++ b/include/libnftnl/object.h
@@ -112,14 +112,36 @@ enum {
 	NFTNL_OBJ_TUNNEL_FLAGS,
 	NFTNL_OBJ_TUNNEL_TOS,
 	NFTNL_OBJ_TUNNEL_TTL,
-	NFTNL_OBJ_TUNNEL_VXLAN_GBP,
-	NFTNL_OBJ_TUNNEL_ERSPAN_VERSION,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
+	NFTNL_OBJ_TUNNEL_OPTS,
 	__NFTNL_OBJ_TUNNEL_MAX,
 };
 
+#define NFTNL_TUNNEL_TYPE	0
+#define NFTNL_TUNNEL_BASE	4
+
+enum nftnl_tunnel_type {
+	NFTNL_TUNNEL_TYPE_VXLAN,
+	NFTNL_TUNNEL_TYPE_ERSPAN,
+};
+
+enum {
+	NFTNL_TUNNEL_VXLAN_GBP		= NFTNL_TUNNEL_BASE,
+	__NFTNL_TUNNEL_VXLAN_MAX,
+};
+
+enum {
+	NFTNL_TUNNEL_ERSPAN_VERSION	= NFTNL_TUNNEL_BASE,
+	NFTNL_TUNNEL_ERSPAN_V1_INDEX,
+	NFTNL_TUNNEL_ERSPAN_V2_HWID,
+	NFTNL_TUNNEL_ERSPAN_V2_DIR,
+	__NFTNL_TUNNEL_ERSPAN_MAX,
+};
+
+struct nftnl_tunnel_opts;
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type);
+int nftnl_tunnel_opts_set(struct nftnl_tunnel_opts *opts, uint16_t type,
+			  const void *data, uint32_t data_len);
+
 enum {
 	NFTNL_OBJ_SECMARK_CTX	= NFTNL_OBJ_BASE,
 	__NFTNL_OBJ_SECMARK_MAX,
diff --git a/include/obj.h b/include/obj.h
index d2177377860d..5d3c4eced199 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -78,21 +78,7 @@ struct nftnl_obj {
 			uint32_t	tun_flags;
 			uint8_t		tun_tos;
 			uint8_t		tun_ttl;
-			union {
-				struct {
-					uint32_t	gbp;
-				} tun_vxlan;
-				struct {
-					uint32_t	version;
-					union {
-						uint32_t	v1_index;
-						struct {
-							uint8_t	hwid;
-							uint8_t	dir;
-						} v2;
-					} u;
-				} tun_erspan;
-			} u;
+			struct nftnl_tunnel_opts *tun_opts;
 		} tunnel;
 		struct nftnl_obj_secmark {
 			char		ctx[NFT_SECMARK_CTX_MAXLEN];
diff --git a/src/obj/tunnel.c b/src/obj/tunnel.c
index 8941e39ffb03..80199928d954 100644
--- a/src/obj/tunnel.c
+++ b/src/obj/tunnel.c
@@ -57,20 +57,8 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_TTL:
 		memcpy(&tun->tun_ttl, data, data_len);
 		break;
-	case NFTNL_OBJ_TUNNEL_VXLAN_GBP:
-		memcpy(&tun->u.tun_vxlan.gbp, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_VERSION:
-		memcpy(&tun->u.tun_erspan.version, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX:
-		memcpy(&tun->u.tun_erspan.u.v1_index, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID:
-		memcpy(&tun->u.tun_erspan.u.v2.hwid, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
-		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
+	case NFTNL_OBJ_TUNNEL_OPTS:
+		memcpy(&tun->tun_opts, data, data_len);
 		break;
 	}
 	return 0;
@@ -116,21 +104,9 @@ nftnl_obj_tunnel_get(const struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_TTL:
 		*data_len = sizeof(tun->tun_ttl);
 		return &tun->tun_ttl;
-	case NFTNL_OBJ_TUNNEL_VXLAN_GBP:
-		*data_len = sizeof(tun->u.tun_vxlan.gbp);
-		return &tun->u.tun_vxlan.gbp;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_VERSION:
-		*data_len = sizeof(tun->u.tun_erspan.version);
-		return &tun->u.tun_erspan.version;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX:
-		*data_len = sizeof(tun->u.tun_erspan.u.v1_index);
-		return &tun->u.tun_erspan.u.v1_index;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID:
-		*data_len = sizeof(tun->u.tun_erspan.u.v2.hwid);
-		return &tun->u.tun_erspan.u.v2.hwid;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
-		*data_len = sizeof(tun->u.tun_erspan.u.v2.dir);
-		return &tun->u.tun_erspan.u.v2.dir;
+	case NFTNL_OBJ_TUNNEL_OPTS:
+		*data_len = sizeof(tun->tun_opts);
+		return &tun->tun_opts;
 	}
 	return NULL;
 }
@@ -171,11 +147,14 @@ static int nftnl_obj_tunnel_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+static void nftnl_tunnel_opts_build(struct nlmsghdr *nlh,
+				    struct nftnl_tunnel_opts *opts);
+
 static void
 nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 {
 	struct nftnl_obj_tunnel *tun = nftnl_obj_data(e);
-	struct nlattr *nest, *nest_inner;
+	struct nlattr *nest;
 
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ID))
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ID, htonl(tun->id));
@@ -212,34 +191,8 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 		mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_TTL, tun->tun_ttl);
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_FLAGS))
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_FLAGS, htonl(tun->tun_flags));
-	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP)) {
-		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
-		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_VXLAN);
-		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_VXLAN_GBP,
-				 htonl(tun->u.tun_vxlan.gbp));
-		mnl_attr_nest_end(nlh, nest_inner);
-		mnl_attr_nest_end(nlh, nest);
-	}
-	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION) &&
-	    (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX) ||
-	     (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID) &&
-	      e->flags & (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR)))) {
-		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
-		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
-		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
-				 htonl(tun->u.tun_erspan.version));
-		if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX))
-			mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
-					 htonl(tun->u.tun_erspan.u.v1_index));
-		if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID))
-			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_HWID,
-					tun->u.tun_erspan.u.v2.hwid);
-		if (e->flags & (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR))
-			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_DIR,
-					tun->u.tun_erspan.u.v2.dir);
-		mnl_attr_nest_end(nlh, nest_inner);
-		mnl_attr_nest_end(nlh, nest);
-	}
+	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_OPTS))
+		nftnl_tunnel_opts_build(nlh, tun->tun_opts);
 }
 
 static int nftnl_obj_tunnel_ip_cb(const struct nlattr *attr, void *data)
@@ -335,6 +288,26 @@ static int nftnl_obj_tunnel_parse_ip6(struct nftnl_obj *e, struct nlattr *attr,
 	return 0;
 }
 
+struct nftnl_tunnel_opts {
+	enum nftnl_tunnel_type			type;
+	uint32_t				flags;
+	union {
+		struct {
+			uint32_t		gbp;
+		} vxlan;
+		struct {
+			uint32_t		version;
+			struct {
+				uint32_t	index;
+			} v1;
+			struct {
+				uint8_t		hwid;
+				uint8_t		dir;
+			} v2;
+		} erspan;
+	};
+};
+
 static int nftnl_obj_tunnel_vxlan_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -355,8 +328,7 @@ static int nftnl_obj_tunnel_vxlan_cb(const struct nlattr *attr, void *data)
 }
 
 static int
-nftnl_obj_tunnel_parse_vxlan(struct nftnl_obj *e, struct nlattr *attr,
-			     struct nftnl_obj_tunnel *tun)
+nftnl_obj_tunnel_parse_vxlan(struct nftnl_tunnel_opts *opts, struct nlattr *attr)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_VXLAN_MAX + 1] = {};
 
@@ -364,9 +336,9 @@ nftnl_obj_tunnel_parse_vxlan(struct nftnl_obj *e, struct nlattr *attr,
 		return -1;
 
 	if (tb[NFTA_TUNNEL_KEY_VXLAN_GBP]) {
-		tun->u.tun_vxlan.gbp =
+		opts->vxlan.gbp =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP);
+		opts->flags |= (1 << NFTNL_TUNNEL_VXLAN_GBP);
 	}
 
 	return 0;
@@ -398,8 +370,7 @@ static int nftnl_obj_tunnel_erspan_cb(const struct nlattr *attr, void *data)
 }
 
 static int
-nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
-			      struct nftnl_obj_tunnel *tun)
+nftnl_obj_tunnel_parse_erspan(struct nftnl_tunnel_opts *opts, struct nlattr *attr)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {};
 
@@ -407,24 +378,24 @@ nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
 		return -1;
 
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]) {
-		tun->u.tun_erspan.version =
+		opts->erspan.version =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION);
+		opts->flags |= (1 << NFTNL_TUNNEL_ERSPAN_VERSION);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]) {
-		tun->u.tun_erspan.u.v1_index =
+		opts->erspan.v1.index =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX);
+		opts->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]) {
-		tun->u.tun_erspan.u.v2.hwid =
+		opts->erspan.v2.hwid =
 			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]);
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID);
+		opts->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]) {
-		tun->u.tun_erspan.u.v2.dir =
+		opts->erspan.v2.dir =
 			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]);
-		e->flags |= (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR);
+		opts->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR);
 	}
 
 	return 0;
@@ -450,22 +421,36 @@ static int nftnl_obj_tunnel_opts_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type);
+
 static int
 nftnl_obj_tunnel_parse_opts(struct nftnl_obj *e, struct nlattr *attr,
 			    struct nftnl_obj_tunnel *tun)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_OPTS_MAX + 1] = {};
+	struct nftnl_tunnel_opts *opts = NULL;
 	int err = 0;
 
 	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_opts_cb, tb) < 0)
 		return -1;
 
 	if (tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]) {
-		err = nftnl_obj_tunnel_parse_vxlan(e, tb[NFTA_TUNNEL_KEY_OPTS_VXLAN],
-						   tun);
+		opts = nftnl_tunnel_opts_alloc(NFTNL_TUNNEL_TYPE_VXLAN);
+		if (!opts)
+			return -1;
+
+		err = nftnl_obj_tunnel_parse_vxlan(opts, tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]);
 	} else if (tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]) {
-		err = nftnl_obj_tunnel_parse_erspan(e, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN],
-						    tun);
+		opts = nftnl_tunnel_opts_alloc(NFTNL_TUNNEL_TYPE_ERSPAN);
+		if (!opts)
+			return -1;
+
+		err = nftnl_obj_tunnel_parse_erspan(opts, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]);
+	}
+
+	if (opts) {
+		tun->tun_opts = opts;
+		e->flags |= (1 << NFTNL_OBJ_TUNNEL_OPTS);
 	}
 
 	return err;
@@ -532,6 +517,138 @@ static int nftnl_obj_tunnel_snprintf(char *buf, size_t len,
 	return snprintf(buf, len, "id %u ", tun->id);
 }
 
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type)
+{
+	struct nftnl_tunnel_opts *opts;
+
+	switch (type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return NULL;
+	}
+
+	opts = calloc(1, sizeof(struct nftnl_tunnel_opts));
+	if (!opts)
+		return NULL;
+
+	opts->type = type;
+
+	return opts;
+}
+
+static int nftnl_tunnel_opts_vxlan_set(struct nftnl_tunnel_opts *opts, uint16_t type,
+				       const void *data, uint32_t data_len)
+{
+	switch (type) {
+	case NFTNL_TUNNEL_VXLAN_GBP:
+		memcpy(&opts->vxlan.gbp, data, data_len);
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+static int nftnl_tunnel_opts_erspan_set(struct nftnl_tunnel_opts *opts, uint16_t type,
+					const void *data, uint32_t data_len)
+{
+	switch (type) {
+	case NFTNL_TUNNEL_ERSPAN_VERSION:
+		memcpy(&opts->erspan.version, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V1_INDEX:
+		memcpy(&opts->erspan.v1.index, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V2_HWID:
+		memcpy(&opts->erspan.v2.hwid, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V2_DIR:
+		memcpy(&opts->erspan.v2.dir, data, data_len);
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+int nftnl_tunnel_opts_set(struct nftnl_tunnel_opts *opts, uint16_t type,
+			  const void *data, uint32_t data_len)
+{
+	switch (opts->type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+		return nftnl_tunnel_opts_vxlan_set(opts, type, data, data_len);
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		return nftnl_tunnel_opts_erspan_set(opts, type, data, data_len);
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+static void nftnl_tunnel_opts_build_vxlan(struct nlmsghdr *nlh,
+					  struct nftnl_tunnel_opts *opts)
+{
+	struct nlattr *nest, *nest_inner;
+
+	if (opts->flags & (1 << NFTNL_TUNNEL_VXLAN_GBP)) {
+		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_VXLAN);
+		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_VXLAN_GBP,
+				 htonl(opts->vxlan.gbp));
+		mnl_attr_nest_end(nlh, nest_inner);
+		mnl_attr_nest_end(nlh, nest);
+	}
+}
+
+static void nftnl_tunnel_opts_build_erspan(struct nlmsghdr *nlh,
+					   struct nftnl_tunnel_opts *opts)
+{
+	struct nlattr *nest, *nest_inner;
+
+	if (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_VERSION) &&
+	    (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX) ||
+	     (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID) &&
+	      opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR)))) {
+		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
+		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
+				 htonl(opts->erspan.version));
+		if (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX))
+			mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
+					 htonl(opts->erspan.v1.index));
+		if (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID))
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_HWID,
+					opts->erspan.v2.hwid);
+		if (opts->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR))
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_DIR,
+					opts->erspan.v2.dir);
+		mnl_attr_nest_end(nlh, nest_inner);
+		mnl_attr_nest_end(nlh, nest);
+	}
+}
+
+void nftnl_tunnel_opts_build(struct nlmsghdr *nlh,
+			     struct nftnl_tunnel_opts *opts)
+{
+	switch (opts->type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+		nftnl_tunnel_opts_build_vxlan(nlh, opts);
+		break;
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		nftnl_tunnel_opts_build_erspan(nlh, opts);
+		break;
+	}
+}
+
 static struct attr_policy obj_tunnel_attr_policy[__NFTNL_OBJ_TUNNEL_MAX] = {
 	[NFTNL_OBJ_TUNNEL_ID]		= { .maxlen = sizeof(uint32_t) },
 	[NFTNL_OBJ_TUNNEL_IPV4_SRC]	= { .maxlen = sizeof(uint32_t) },
@@ -544,11 +661,7 @@ static struct attr_policy obj_tunnel_attr_policy[__NFTNL_OBJ_TUNNEL_MAX] = {
 	[NFTNL_OBJ_TUNNEL_FLAGS]	= { .maxlen = sizeof(uint32_t) },
 	[NFTNL_OBJ_TUNNEL_TOS]		= { .maxlen = sizeof(uint8_t) },
 	[NFTNL_OBJ_TUNNEL_TTL]		= { .maxlen = sizeof(uint8_t) },
-	[NFTNL_OBJ_TUNNEL_VXLAN_GBP]	= { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_VERSION] = { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX] = { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID] = { .maxlen = sizeof(uint8_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR] = { .maxlen = sizeof(uint8_t) },
+	[NFTNL_OBJ_TUNNEL_OPTS]		= { .maxlen = sizeof(struct nftnl_tunnel_opts *) },
 };
 
 struct obj_ops obj_ops_tunnel = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-06-10  0:28     ` Pablo Neira Ayuso
@ 2025-06-10  6:01       ` Florian Westphal
  2025-06-10  9:47         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2025-06-10  6:01 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Fernando Fernandez Mancera, netfilter-devel, coreteam

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hmm, this looks like the API leaks internal data layout from nftables to
> > libnftnl and vice versa?  IMO thats a non-starter, sorry.
> > 
> > I see that options are essentially unlimited values, so perhaps nftables
> > should build the netlink blob(s) directly, similar to nftnl_udata()?
> > 
> > Pablo, any better idea?
> 
> Maybe this API for tunnel options are proposed in this patch?

Looks good, thanks Pablo!

> Consider this a sketch/proposal, this is compiled tested only.
> 
> struct obj_ops also needs a .free interface to release the tunnel
> options object.

nftnl_tunnel_opts_set() seems to be useable for erspan and vxlan.

Do you have a suggestion for the geneve case where 'infinite' options
get added?

Maybe add nftnl_tunnel_opts_append() ? Or nftnl_tunnel_opts_add(), so
api user can push multiple option objects to a tunnel, similar to how
rules get added to chains?

Would probably require a few more api calls including iterators.

Fernando, do you spot anything else thats missing for your use cases?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-06-10  6:01       ` Florian Westphal
@ 2025-06-10  9:47         ` Pablo Neira Ayuso
  2025-06-12 13:01           ` Fernando Fernandez Mancera
  0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2025-06-10  9:47 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Fernando Fernandez Mancera, netfilter-devel, coreteam

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

On Tue, Jun 10, 2025 at 08:01:41AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Hmm, this looks like the API leaks internal data layout from nftables to
> > > libnftnl and vice versa?  IMO thats a non-starter, sorry.
> > > 
> > > I see that options are essentially unlimited values, so perhaps nftables
> > > should build the netlink blob(s) directly, similar to nftnl_udata()?
> > > 
> > > Pablo, any better idea?
> > 
> > Maybe this API for tunnel options are proposed in this patch?
> 
> Looks good, thanks Pablo!
> 
> > Consider this a sketch/proposal, this is compiled tested only.
> > 
> > struct obj_ops also needs a .free interface to release the tunnel
> > options object.
> 
> nftnl_tunnel_opts_set() seems to be useable for erspan and vxlan.
> 
> Do you have a suggestion for the geneve case where 'infinite' options
> get added?
> 
> Maybe add nftnl_tunnel_opts_append() ? Or nftnl_tunnel_opts_add(), so
> api user can push multiple option objects to a tunnel, similar to how
> rules get added to chains?

nftnl_tunnel_opts_add() sounds good.

It should be possible to replace nftnl_tunnel_opts_set() by
nftnl_tunnel_opts_add(), then a single function for this purpose is
provided. As for vxlan and erpan, allow only one single call to
nftnl_tunnel_opts_add().

See attachment, compile tested only.

> Would probably require a few more api calls including iterators.
> 
> Fernando, do you spot anything else thats missing for your use cases?

[-- Attachment #2: 0001-tunnel-rework-options.patch --]
[-- Type: text/x-diff, Size: 17699 bytes --]

From a8bc6cc58ae4728134e7dc16bf4e553c2c58e176 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 10 Jun 2025 01:43:28 +0200
Subject: [PATCH libnftnl,v2,WIP] tunnel: rework options

Only vxlan gbp can work before this patch because
NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR is off by one in the internal object
flags.

Replace them by NFTNL_OBJ_TUNNEL_OPTS and add two new opaque
nftnl_tunnel_opts and nftnl_tunnel_opt structs to represent tunnel
options.

- nftnl_tunnel_opt_alloc() allocates one tunnel option.
- nftnl_tunnel_opt_set() to sets it up.

Then, to manage the list of options:

- nftnl_tunnel_opts_alloc() allocates a list of tunnel options.
- nftnl_tunnel_opts_add() adds a option to the list.

Although vxlan and erspan support for a single tunnel option at this
stage, this API prepares for supporting gevene which allows for more
tunnel options.

TODO: Missing .free interface for objects to release this list of
objects.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/libnftnl/object.h |  38 +++-
 include/obj.h             |  16 +-
 src/obj/tunnel.c          | 353 +++++++++++++++++++++++++++++---------
 3 files changed, 306 insertions(+), 101 deletions(-)

diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
index 9930355bb8f0..8ce62b7d97a2 100644
--- a/include/libnftnl/object.h
+++ b/include/libnftnl/object.h
@@ -112,14 +112,42 @@ enum {
 	NFTNL_OBJ_TUNNEL_FLAGS,
 	NFTNL_OBJ_TUNNEL_TOS,
 	NFTNL_OBJ_TUNNEL_TTL,
-	NFTNL_OBJ_TUNNEL_VXLAN_GBP,
-	NFTNL_OBJ_TUNNEL_ERSPAN_VERSION,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
-	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
+	NFTNL_OBJ_TUNNEL_OPTS,
 	__NFTNL_OBJ_TUNNEL_MAX,
 };
 
+#define NFTNL_TUNNEL_TYPE	0
+#define NFTNL_TUNNEL_BASE	4
+
+enum nftnl_tunnel_type {
+	NFTNL_TUNNEL_TYPE_VXLAN,
+	NFTNL_TUNNEL_TYPE_ERSPAN,
+};
+
+enum {
+	NFTNL_TUNNEL_VXLAN_GBP		= NFTNL_TUNNEL_BASE,
+	__NFTNL_TUNNEL_VXLAN_MAX,
+};
+
+enum {
+	NFTNL_TUNNEL_ERSPAN_VERSION	= NFTNL_TUNNEL_BASE,
+	NFTNL_TUNNEL_ERSPAN_V1_INDEX,
+	NFTNL_TUNNEL_ERSPAN_V2_HWID,
+	NFTNL_TUNNEL_ERSPAN_V2_DIR,
+	__NFTNL_TUNNEL_ERSPAN_MAX,
+};
+
+struct nftnl_tunnel_opt;
+struct nftnl_tunnel_opts;
+
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type);
+int nftnl_tunnel_opts_add(struct nftnl_tunnel_opts *opts,
+			  struct nftnl_tunnel_opt *opt);
+
+struct nftnl_tunnel_opt *nftnl_tunnel_opt_alloc(enum nftnl_tunnel_type type);
+int nftnl_tunnel_opt_set(struct nftnl_tunnel_opt *opt, uint16_t type,
+			 const void *data, uint32_t data_len);
+
 enum {
 	NFTNL_OBJ_SECMARK_CTX	= NFTNL_OBJ_BASE,
 	__NFTNL_OBJ_SECMARK_MAX,
diff --git a/include/obj.h b/include/obj.h
index d2177377860d..5d3c4eced199 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -78,21 +78,7 @@ struct nftnl_obj {
 			uint32_t	tun_flags;
 			uint8_t		tun_tos;
 			uint8_t		tun_ttl;
-			union {
-				struct {
-					uint32_t	gbp;
-				} tun_vxlan;
-				struct {
-					uint32_t	version;
-					union {
-						uint32_t	v1_index;
-						struct {
-							uint8_t	hwid;
-							uint8_t	dir;
-						} v2;
-					} u;
-				} tun_erspan;
-			} u;
+			struct nftnl_tunnel_opts *tun_opts;
 		} tunnel;
 		struct nftnl_obj_secmark {
 			char		ctx[NFT_SECMARK_CTX_MAXLEN];
diff --git a/src/obj/tunnel.c b/src/obj/tunnel.c
index 8941e39ffb03..0c901f4848c4 100644
--- a/src/obj/tunnel.c
+++ b/src/obj/tunnel.c
@@ -57,20 +57,8 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_TTL:
 		memcpy(&tun->tun_ttl, data, data_len);
 		break;
-	case NFTNL_OBJ_TUNNEL_VXLAN_GBP:
-		memcpy(&tun->u.tun_vxlan.gbp, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_VERSION:
-		memcpy(&tun->u.tun_erspan.version, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX:
-		memcpy(&tun->u.tun_erspan.u.v1_index, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID:
-		memcpy(&tun->u.tun_erspan.u.v2.hwid, data, data_len);
-		break;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
-		memcpy(&tun->u.tun_erspan.u.v2.dir, data, data_len);
+	case NFTNL_OBJ_TUNNEL_OPTS:
+		memcpy(&tun->tun_opts, data, data_len);
 		break;
 	}
 	return 0;
@@ -116,21 +104,9 @@ nftnl_obj_tunnel_get(const struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_TTL:
 		*data_len = sizeof(tun->tun_ttl);
 		return &tun->tun_ttl;
-	case NFTNL_OBJ_TUNNEL_VXLAN_GBP:
-		*data_len = sizeof(tun->u.tun_vxlan.gbp);
-		return &tun->u.tun_vxlan.gbp;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_VERSION:
-		*data_len = sizeof(tun->u.tun_erspan.version);
-		return &tun->u.tun_erspan.version;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX:
-		*data_len = sizeof(tun->u.tun_erspan.u.v1_index);
-		return &tun->u.tun_erspan.u.v1_index;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID:
-		*data_len = sizeof(tun->u.tun_erspan.u.v2.hwid);
-		return &tun->u.tun_erspan.u.v2.hwid;
-	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
-		*data_len = sizeof(tun->u.tun_erspan.u.v2.dir);
-		return &tun->u.tun_erspan.u.v2.dir;
+	case NFTNL_OBJ_TUNNEL_OPTS:
+		*data_len = sizeof(tun->tun_opts);
+		return &tun->tun_opts;
 	}
 	return NULL;
 }
@@ -171,11 +147,14 @@ static int nftnl_obj_tunnel_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+static void nftnl_tunnel_opts_build(struct nlmsghdr *nlh,
+				    struct nftnl_tunnel_opts *opts);
+
 static void
 nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 {
 	struct nftnl_obj_tunnel *tun = nftnl_obj_data(e);
-	struct nlattr *nest, *nest_inner;
+	struct nlattr *nest;
 
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ID))
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ID, htonl(tun->id));
@@ -212,34 +191,8 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 		mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_TTL, tun->tun_ttl);
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_FLAGS))
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_FLAGS, htonl(tun->tun_flags));
-	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP)) {
-		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
-		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_VXLAN);
-		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_VXLAN_GBP,
-				 htonl(tun->u.tun_vxlan.gbp));
-		mnl_attr_nest_end(nlh, nest_inner);
-		mnl_attr_nest_end(nlh, nest);
-	}
-	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION) &&
-	    (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX) ||
-	     (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID) &&
-	      e->flags & (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR)))) {
-		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
-		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
-		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
-				 htonl(tun->u.tun_erspan.version));
-		if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX))
-			mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
-					 htonl(tun->u.tun_erspan.u.v1_index));
-		if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID))
-			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_HWID,
-					tun->u.tun_erspan.u.v2.hwid);
-		if (e->flags & (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR))
-			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_DIR,
-					tun->u.tun_erspan.u.v2.dir);
-		mnl_attr_nest_end(nlh, nest_inner);
-		mnl_attr_nest_end(nlh, nest);
-	}
+	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_OPTS))
+		nftnl_tunnel_opts_build(nlh, tun->tun_opts);
 }
 
 static int nftnl_obj_tunnel_ip_cb(const struct nlattr *attr, void *data)
@@ -335,6 +288,35 @@ static int nftnl_obj_tunnel_parse_ip6(struct nftnl_obj *e, struct nlattr *attr,
 	return 0;
 }
 
+struct nftnl_tunnel_opt {
+	struct list_head			list;
+
+	enum nftnl_tunnel_type			type;
+	uint32_t				flags;
+
+	union {
+		struct {
+			uint32_t		gbp;
+		} vxlan;
+		struct {
+			uint32_t		version;
+			struct {
+				uint32_t	index;
+			} v1;
+			struct {
+				uint8_t		hwid;
+				uint8_t		dir;
+			} v2;
+		} erspan;
+	};
+};
+
+struct nftnl_tunnel_opts {
+	enum nftnl_tunnel_type			type;
+	uint32_t				num;
+	struct list_head			opts_list;
+};
+
 static int nftnl_obj_tunnel_vxlan_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -354,21 +336,29 @@ static int nftnl_obj_tunnel_vxlan_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+struct nftnl_tunnel_opt *nftnl_tunnel_opt_alloc(enum nftnl_tunnel_type type);
+
 static int
-nftnl_obj_tunnel_parse_vxlan(struct nftnl_obj *e, struct nlattr *attr,
-			     struct nftnl_obj_tunnel *tun)
+nftnl_obj_tunnel_parse_vxlan(struct nftnl_tunnel_opts *opts, struct nlattr *attr)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_VXLAN_MAX + 1] = {};
+	struct nftnl_tunnel_opt *opt;
 
 	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_vxlan_cb, tb) < 0)
 		return -1;
 
+	opt = nftnl_tunnel_opt_alloc(NFTNL_TUNNEL_TYPE_VXLAN);
+	if (!opt)
+		return -1;
+
 	if (tb[NFTA_TUNNEL_KEY_VXLAN_GBP]) {
-		tun->u.tun_vxlan.gbp =
+		opt->vxlan.gbp =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP);
+		opt->flags |= (1 << NFTNL_TUNNEL_VXLAN_GBP);
 	}
 
+	list_add_tail(&opt->list, &opts->opts_list);
+
 	return 0;
 }
 
@@ -398,35 +388,41 @@ static int nftnl_obj_tunnel_erspan_cb(const struct nlattr *attr, void *data)
 }
 
 static int
-nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
-			      struct nftnl_obj_tunnel *tun)
+nftnl_obj_tunnel_parse_erspan(struct nftnl_tunnel_opts *opts, struct nlattr *attr)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {};
+	struct nftnl_tunnel_opt *opt;
 
 	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_erspan_cb, tb) < 0)
 		return -1;
 
+	opt = nftnl_tunnel_opt_alloc(NFTNL_TUNNEL_TYPE_VXLAN);
+	if (!opt)
+		return -1;
+
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]) {
-		tun->u.tun_erspan.version =
+		opt->erspan.version =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION);
+		opt->flags |= (1 << NFTNL_TUNNEL_ERSPAN_VERSION);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]) {
-		tun->u.tun_erspan.u.v1_index =
+		opt->erspan.v1.index =
 			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]));
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX);
+		opt->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]) {
-		tun->u.tun_erspan.u.v2.hwid =
+		opt->erspan.v2.hwid =
 			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]);
-		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID);
+		opt->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]) {
-		tun->u.tun_erspan.u.v2.dir =
+		opt->erspan.v2.dir =
 			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]);
-		e->flags |= (1u << NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR);
+		opt->flags |= (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR);
 	}
 
+	list_add_tail(&opt->list, &opts->opts_list);
+
 	return 0;
 }
 
@@ -450,22 +446,36 @@ static int nftnl_obj_tunnel_opts_cb(const struct nlattr *attr, void *data)
 	return MNL_CB_OK;
 }
 
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type);
+
 static int
 nftnl_obj_tunnel_parse_opts(struct nftnl_obj *e, struct nlattr *attr,
 			    struct nftnl_obj_tunnel *tun)
 {
 	struct nlattr *tb[NFTA_TUNNEL_KEY_OPTS_MAX + 1] = {};
+	struct nftnl_tunnel_opts *opts = NULL;
 	int err = 0;
 
 	if (mnl_attr_parse_nested(attr, nftnl_obj_tunnel_opts_cb, tb) < 0)
 		return -1;
 
 	if (tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]) {
-		err = nftnl_obj_tunnel_parse_vxlan(e, tb[NFTA_TUNNEL_KEY_OPTS_VXLAN],
-						   tun);
+		opts = nftnl_tunnel_opts_alloc(NFTNL_TUNNEL_TYPE_VXLAN);
+		if (!opts)
+			return -1;
+
+		err = nftnl_obj_tunnel_parse_vxlan(opts, tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]);
 	} else if (tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]) {
-		err = nftnl_obj_tunnel_parse_erspan(e, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN],
-						    tun);
+		opts = nftnl_tunnel_opts_alloc(NFTNL_TUNNEL_TYPE_ERSPAN);
+		if (!opts)
+			return -1;
+
+		err = nftnl_obj_tunnel_parse_erspan(opts, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]);
+	}
+
+	if (opts) {
+		tun->tun_opts = opts;
+		e->flags |= (1 << NFTNL_OBJ_TUNNEL_OPTS);
 	}
 
 	return err;
@@ -532,6 +542,191 @@ static int nftnl_obj_tunnel_snprintf(char *buf, size_t len,
 	return snprintf(buf, len, "id %u ", tun->id);
 }
 
+struct nftnl_tunnel_opts *nftnl_tunnel_opts_alloc(enum nftnl_tunnel_type type)
+{
+	struct nftnl_tunnel_opts *opts;
+
+	switch (type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return NULL;
+	}
+
+	opts = calloc(1, sizeof(struct nftnl_tunnel_opts));
+	if (!opts)
+		return NULL;
+
+	opts->type = type;
+	INIT_LIST_HEAD(&opts->opts_list);
+
+	return opts;
+}
+
+int nftnl_tunnel_opts_add(struct nftnl_tunnel_opts *opts,
+			  struct nftnl_tunnel_opt *opt)
+{
+	if (opt->type != opts->type) {
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	switch (opts->type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		if (opts->num > 0) {
+			errno = EEXIST;
+			return -1;
+		}
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	list_add_tail(&opt->list, &opts->opts_list);
+
+	return 0;
+}
+
+struct nftnl_tunnel_opt *nftnl_tunnel_opt_alloc(enum nftnl_tunnel_type type)
+{
+	struct nftnl_tunnel_opt *opt;
+
+	switch (type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return NULL;
+	}
+
+	opt = calloc(1, sizeof(struct nftnl_tunnel_opt));
+	if (!opt)
+		return NULL;
+
+	opt->type = type;
+
+	return opt;
+}
+
+static int nftnl_tunnel_opt_vxlan_set(struct nftnl_tunnel_opt *opt, uint16_t type,
+				      const void *data, uint32_t data_len)
+{
+	switch (type) {
+	case NFTNL_TUNNEL_VXLAN_GBP:
+		memcpy(&opt->vxlan.gbp, data, data_len);
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+static int nftnl_tunnel_opt_erspan_set(struct nftnl_tunnel_opt *opt, uint16_t type,
+				       const void *data, uint32_t data_len)
+{
+	switch (type) {
+	case NFTNL_TUNNEL_ERSPAN_VERSION:
+		memcpy(&opt->erspan.version, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V1_INDEX:
+		memcpy(&opt->erspan.v1.index, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V2_HWID:
+		memcpy(&opt->erspan.v2.hwid, data, data_len);
+		break;
+	case NFTNL_TUNNEL_ERSPAN_V2_DIR:
+		memcpy(&opt->erspan.v2.dir, data, data_len);
+		break;
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+int nftnl_tunnel_opt_set(struct nftnl_tunnel_opt *opt, uint16_t type,
+			 const void *data, uint32_t data_len)
+{
+	switch (opt->type) {
+	case NFTNL_TUNNEL_TYPE_VXLAN:
+		return nftnl_tunnel_opt_vxlan_set(opt, type, data, data_len);
+	case NFTNL_TUNNEL_TYPE_ERSPAN:
+		return nftnl_tunnel_opt_erspan_set(opt, type, data, data_len);
+	default:
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	return 0;
+}
+
+static void nftnl_tunnel_opt_build_vxlan(struct nlmsghdr *nlh,
+					 const struct nftnl_tunnel_opt *opt)
+{
+	struct nlattr *nest_inner;
+
+	if (opt->flags & (1 << NFTNL_TUNNEL_VXLAN_GBP)) {
+		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_VXLAN);
+		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_VXLAN_GBP,
+				 htonl(opt->vxlan.gbp));
+		mnl_attr_nest_end(nlh, nest_inner);
+	}
+}
+
+static void nftnl_tunnel_opt_build_erspan(struct nlmsghdr *nlh,
+					  const struct nftnl_tunnel_opt *opt)
+{
+	struct nlattr *nest_inner;
+
+	if (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_VERSION) &&
+	    (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX) ||
+	     (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID) &&
+	      opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR)))) {
+		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
+		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
+				 htonl(opt->erspan.version));
+		if (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX))
+			mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
+					 htonl(opt->erspan.v1.index));
+		if (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID))
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_HWID,
+					opt->erspan.v2.hwid);
+		if (opt->flags & (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR))
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_V2_DIR,
+					opt->erspan.v2.dir);
+		mnl_attr_nest_end(nlh, nest_inner);
+	}
+}
+
+void nftnl_tunnel_opts_build(struct nlmsghdr *nlh,
+			     struct nftnl_tunnel_opts *opts)
+{
+	const struct nftnl_tunnel_opt *opt;
+	struct nlattr *nest;
+
+	nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+
+	list_for_each_entry(opt, &opts->opts_list, list) {
+		switch (opts->type) {
+		case NFTNL_TUNNEL_TYPE_VXLAN:
+			nftnl_tunnel_opt_build_vxlan(nlh, opt);
+			break;
+		case NFTNL_TUNNEL_TYPE_ERSPAN:
+			nftnl_tunnel_opt_build_erspan(nlh, opt);
+			break;
+		}
+	}
+	mnl_attr_nest_end(nlh, nest);
+}
+
 static struct attr_policy obj_tunnel_attr_policy[__NFTNL_OBJ_TUNNEL_MAX] = {
 	[NFTNL_OBJ_TUNNEL_ID]		= { .maxlen = sizeof(uint32_t) },
 	[NFTNL_OBJ_TUNNEL_IPV4_SRC]	= { .maxlen = sizeof(uint32_t) },
@@ -544,11 +739,7 @@ static struct attr_policy obj_tunnel_attr_policy[__NFTNL_OBJ_TUNNEL_MAX] = {
 	[NFTNL_OBJ_TUNNEL_FLAGS]	= { .maxlen = sizeof(uint32_t) },
 	[NFTNL_OBJ_TUNNEL_TOS]		= { .maxlen = sizeof(uint8_t) },
 	[NFTNL_OBJ_TUNNEL_TTL]		= { .maxlen = sizeof(uint8_t) },
-	[NFTNL_OBJ_TUNNEL_VXLAN_GBP]	= { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_VERSION] = { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX] = { .maxlen = sizeof(uint32_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID] = { .maxlen = sizeof(uint8_t) },
-	[NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR] = { .maxlen = sizeof(uint8_t) },
+	[NFTNL_OBJ_TUNNEL_OPTS]		= { .maxlen = sizeof(struct nftnl_tunnel_opts *) },
 };
 
 struct obj_ops obj_ops_tunnel = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-06-10  9:47         ` Pablo Neira Ayuso
@ 2025-06-12 13:01           ` Fernando Fernandez Mancera
  2025-06-12 19:18             ` Pablo Neira Ayuso
  0 siblings, 1 reply; 12+ messages in thread
From: Fernando Fernandez Mancera @ 2025-06-12 13:01 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal; +Cc: netfilter-devel, coreteam



On 6/10/25 11:47 AM, Pablo Neira Ayuso wrote:
> On Tue, Jun 10, 2025 at 08:01:41AM +0200, Florian Westphal wrote:
>> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>>> Hmm, this looks like the API leaks internal data layout from nftables to
>>>> libnftnl and vice versa?  IMO thats a non-starter, sorry.
>>>>
>>>> I see that options are essentially unlimited values, so perhaps nftables
>>>> should build the netlink blob(s) directly, similar to nftnl_udata()?
>>>>
>>>> Pablo, any better idea?
>>>
>>> Maybe this API for tunnel options are proposed in this patch?
>>
>> Looks good, thanks Pablo!
>>
>>> Consider this a sketch/proposal, this is compiled tested only.
>>>
>>> struct obj_ops also needs a .free interface to release the tunnel
>>> options object.
>>
>> nftnl_tunnel_opts_set() seems to be useable for erspan and vxlan.
>>
>> Do you have a suggestion for the geneve case where 'infinite' options
>> get added?
>>
>> Maybe add nftnl_tunnel_opts_append() ? Or nftnl_tunnel_opts_add(), so
>> api user can push multiple option objects to a tunnel, similar to how
>> rules get added to chains?
> 
> nftnl_tunnel_opts_add() sounds good.
> 
> It should be possible to replace nftnl_tunnel_opts_set() by
> nftnl_tunnel_opts_add(), then a single function for this purpose is
> provided. As for vxlan and erpan, allow only one single call to
> nftnl_tunnel_opts_add().
> 
> See attachment, compile tested only.
> 

This looks good to me. Let me include it on my series and extend the 
free interface.

Thanks Pablo!

>> Would probably require a few more api calls including iterators.
>>
>> Fernando, do you spot anything else thats missing for your use cases?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options
  2025-06-12 13:01           ` Fernando Fernandez Mancera
@ 2025-06-12 19:18             ` Pablo Neira Ayuso
  0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2025-06-12 19:18 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: Florian Westphal, netfilter-devel, coreteam

On Thu, Jun 12, 2025 at 03:01:44PM +0200, Fernando Fernandez Mancera wrote:
> 
> 
> On 6/10/25 11:47 AM, Pablo Neira Ayuso wrote:
> > On Tue, Jun 10, 2025 at 08:01:41AM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > Hmm, this looks like the API leaks internal data layout from nftables to
> > > > > libnftnl and vice versa?  IMO thats a non-starter, sorry.
> > > > > 
> > > > > I see that options are essentially unlimited values, so perhaps nftables
> > > > > should build the netlink blob(s) directly, similar to nftnl_udata()?
> > > > > 
> > > > > Pablo, any better idea?
> > > > 
> > > > Maybe this API for tunnel options are proposed in this patch?
> > > 
> > > Looks good, thanks Pablo!
> > > 
> > > > Consider this a sketch/proposal, this is compiled tested only.
> > > > 
> > > > struct obj_ops also needs a .free interface to release the tunnel
> > > > options object.
> > > 
> > > nftnl_tunnel_opts_set() seems to be useable for erspan and vxlan.
> > > 
> > > Do you have a suggestion for the geneve case where 'infinite' options
> > > get added?
> > > 
> > > Maybe add nftnl_tunnel_opts_append() ? Or nftnl_tunnel_opts_add(), so
> > > api user can push multiple option objects to a tunnel, similar to how
> > > rules get added to chains?
> > 
> > nftnl_tunnel_opts_add() sounds good.
> > 
> > It should be possible to replace nftnl_tunnel_opts_set() by
> > nftnl_tunnel_opts_add(), then a single function for this purpose is
> > provided. As for vxlan and erpan, allow only one single call to
> > nftnl_tunnel_opts_add().
> > 
> > See attachment, compile tested only.
> > 
> 
> This looks good to me. Let me include it on my series and extend the free
> interface.

Sure, go ahead, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-06-12 19:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-27 19:34 [PATCH 1/2 libnftnl v2] src: use uint64_t for flags fields Fernando Fernandez Mancera
2025-05-27 19:34 ` [PATCH 2/2 libnftnl v2] tunnel: add support to geneve options Fernando Fernandez Mancera
2025-05-28  0:34   ` Florian Westphal
2025-05-28  9:27     ` Fernando Fernandez Mancera
2025-05-28 10:52       ` Florian Westphal
2025-06-10  0:28     ` Pablo Neira Ayuso
2025-06-10  6:01       ` Florian Westphal
2025-06-10  9:47         ` Pablo Neira Ayuso
2025-06-12 13:01           ` Fernando Fernandez Mancera
2025-06-12 19:18             ` Pablo Neira Ayuso
2025-05-28 10:55   ` Florian Westphal
2025-05-28 11:31     ` Fernando Fernandez Mancera

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.