* [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-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
* 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
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.