netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
@ 2025-07-15 15:15 Phil Sutter
  2025-07-16  9:44 ` Florian Westphal
  0 siblings, 1 reply; 10+ messages in thread
From: Phil Sutter @ 2025-07-15 15:15 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
helpers converting to and from the human-readable asterisk-suffix
notation.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
No changes since v1 apart from a rebase onto current HEAD.
This single patch supersedes the previous series "Support wildcard
netdev hooks and events" as without NEWDEV/DELDEV kernel notifications,
this is the only change needed by libnftnl to support ifname-based
hooks.
---
 include/utils.h |  4 ++++
 src/chain.c     |  8 +++++---
 src/flowtable.c |  2 +-
 src/str_array.c |  2 +-
 src/utils.c     | 30 ++++++++++++++++++++++++++++++
 5 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 247d99d19dd7f..c8e890eae2ffd 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -6,6 +6,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <stdlib.h>
+#include <libmnl/libmnl.h>
 #include <libnftnl/common.h>
 
 #include "config.h"
@@ -83,4 +84,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
 int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
 		       uint16_t attr, const void *data, uint32_t data_len);
 
+void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname);
+const char *mnl_attr_get_ifname(struct nlattr *attr);
+
 #endif
diff --git a/src/chain.c b/src/chain.c
index 895108cddad51..17ae5568609c4 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -457,14 +457,14 @@ void nftnl_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nftnl_ch
 		mnl_attr_put_u32(nlh, NFTA_HOOK_PRIORITY, htonl(c->prio));
 
 	if (c->flags & (1 << NFTNL_CHAIN_DEV))
-		mnl_attr_put_strz(nlh, NFTA_HOOK_DEV, c->dev);
+		mnl_attr_put_ifname(nlh, NFTA_HOOK_DEV, c->dev);
 	else if (c->flags & (1 << NFTNL_CHAIN_DEVICES)) {
 		struct nlattr *nest_dev;
 		const char *dev;
 
 		nest_dev = mnl_attr_nest_start(nlh, NFTA_HOOK_DEVS);
 		nftnl_str_array_foreach(dev, &c->dev_array, i)
-			mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
+			mnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
 		mnl_attr_nest_end(nlh, nest_dev);
 	}
 
@@ -648,7 +648,9 @@ static int nftnl_chain_parse_hook(struct nlattr *attr, struct nftnl_chain *c)
 		c->flags |= (1 << NFTNL_CHAIN_PRIO);
 	}
 	if (tb[NFTA_HOOK_DEV]) {
-		c->dev = strdup(mnl_attr_get_str(tb[NFTA_HOOK_DEV]));
+		if (c->flags & (1 << NFTNL_CHAIN_DEV))
+			xfree(c->dev);
+		c->dev = strdup(mnl_attr_get_ifname(tb[NFTA_HOOK_DEV]));
 		if (!c->dev)
 			return -1;
 		c->flags |= (1 << NFTNL_CHAIN_DEV);
diff --git a/src/flowtable.c b/src/flowtable.c
index fbbe0a866368d..75e86fb6d1e55 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -299,7 +299,7 @@ void nftnl_flowtable_nlmsg_build_payload(struct nlmsghdr *nlh,
 
 		nest_dev = mnl_attr_nest_start(nlh, NFTA_FLOWTABLE_HOOK_DEVS);
 		nftnl_str_array_foreach(dev, &c->dev_array, i)
-			mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
+			mnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
 		mnl_attr_nest_end(nlh, nest_dev);
 	}
 
diff --git a/src/str_array.c b/src/str_array.c
index 5669c6154d394..bb75e04cb20da 100644
--- a/src/str_array.c
+++ b/src/str_array.c
@@ -56,7 +56,7 @@ int nftnl_parse_devs(struct nftnl_str_array *sa, const struct nlattr *nest)
 		return -1;
 
 	mnl_attr_for_each_nested(attr, nest) {
-		sa->array[sa->len] = strdup(mnl_attr_get_str(attr));
+		sa->array[sa->len] = strdup(mnl_attr_get_ifname(attr));
 		if (!sa->array[sa->len]) {
 			nftnl_str_array_clear(sa);
 			return -1;
diff --git a/src/utils.c b/src/utils.c
index 5f2c5bff7c650..bcace0cd72788 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -13,8 +13,11 @@
 #include <errno.h>
 #include <inttypes.h>
 
+#include <libmnl/libmnl.h>
+
 #include <libnftnl/common.h>
 
+#include <linux/if.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter/nf_tables.h>
 
@@ -146,3 +149,30 @@ int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
 	*flags |= (1 << attr);
 	return 0;
 }
+
+void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname)
+{
+	int len = strlen(ifname) + 1;
+
+	if (ifname[len - 2] == '*')
+		len -= 2;
+
+	mnl_attr_put(nlh, attr, len, ifname);
+}
+
+const char *mnl_attr_get_ifname(struct nlattr *attr)
+{
+	size_t slen = mnl_attr_get_payload_len(attr);
+	const char *dev = mnl_attr_get_str(attr);
+	static char buf[IFNAMSIZ];
+
+	if (dev[slen - 1] == '\0')
+		return dev;
+
+	if (slen > IFNAMSIZ - 2)
+		slen = IFNAMSIZ - 2;
+
+	memcpy(buf, dev, slen);
+	memcpy(buf + slen, "*\0", 2);
+	return buf;
+}
-- 
2.49.0


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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-15 15:15 [libnftnl PATCH v2] utils: Add helpers for interface name wildcards Phil Sutter
@ 2025-07-16  9:44 ` Florian Westphal
  2025-07-16 10:01   ` Phil Sutter
  2025-07-16 14:38   ` Pablo Neira Ayuso
  0 siblings, 2 replies; 10+ messages in thread
From: Florian Westphal @ 2025-07-16  9:44 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
>  #include <string.h>
>  #include <stdlib.h>
> +#include <libmnl/libmnl.h>

Why is this include needed?

>  #include <libnftnl/common.h>
>  
>  #include "config.h"
> @@ -83,4 +84,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
>  int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
>  		       uint16_t attr, const void *data, uint32_t data_len);
>  
> +void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname);
> +const char *mnl_attr_get_ifname(struct nlattr *attr);
> +

nftnl_attr_put_ifname, nftnl_attr_get_ifname?
Using mnl_ prefix seems wrong, that should be reserved for libmnl.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16  9:44 ` Florian Westphal
@ 2025-07-16 10:01   ` Phil Sutter
  2025-07-16 10:07     ` Florian Westphal
  2025-07-16 14:39     ` Pablo Neira Ayuso
  2025-07-16 14:38   ` Pablo Neira Ayuso
  1 sibling, 2 replies; 10+ messages in thread
From: Phil Sutter @ 2025-07-16 10:01 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel

Hi,

On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> >  #include <string.h>
> >  #include <stdlib.h>
> > +#include <libmnl/libmnl.h>
> 
> Why is this include needed?

Because of:

| In file included from udata.c:9:
| ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
|    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
|       |                                        ^~~~~~

> >  #include <libnftnl/common.h>
> >  
> >  #include "config.h"
> > @@ -83,4 +84,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
> >  int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
> >  		       uint16_t attr, const void *data, uint32_t data_len);
> >  
> > +void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname);
> > +const char *mnl_attr_get_ifname(struct nlattr *attr);
> > +
> 
> nftnl_attr_put_ifname, nftnl_attr_get_ifname?
> Using mnl_ prefix seems wrong, that should be reserved for libmnl.

I chose the prefix since they 1:1 replace calls to mnl_attr_put_strz() and
mnl_attr_get_str(). You're right, since they are implemented by libnftnl the
prefix is wrong. I was a bit undecided whether to put them into libmnl, but
they are a bit too specific for that. Will fix the prefix and respin!

Thanks, Phil

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 10:01   ` Phil Sutter
@ 2025-07-16 10:07     ` Florian Westphal
  2025-07-16 10:11       ` Phil Sutter
  2025-07-16 14:39     ` Pablo Neira Ayuso
  1 sibling, 1 reply; 10+ messages in thread
From: Florian Westphal @ 2025-07-16 10:07 UTC (permalink / raw)
  To: Phil Sutter, Pablo Neira Ayuso, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > >  #include <string.h>
> > >  #include <stdlib.h>
> > > +#include <libmnl/libmnl.h>
> > 
> > Why is this include needed?
> 
> Because of:
> 
> | In file included from udata.c:9:
> | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> |       |                                        ^~~~~~

That struct is defined in linux/netlink.h.

But you can add a foward declaration for this, i.e.:

struct nlattr;

As the layout isn't needed for the function declaration.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 10:07     ` Florian Westphal
@ 2025-07-16 10:11       ` Phil Sutter
  0 siblings, 0 replies; 10+ messages in thread
From: Phil Sutter @ 2025-07-16 10:11 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel

On Wed, Jul 16, 2025 at 12:07:59PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > >  #include <string.h>
> > > >  #include <stdlib.h>
> > > > +#include <libmnl/libmnl.h>
> > > 
> > > Why is this include needed?
> > 
> > Because of:
> > 
> > | In file included from udata.c:9:
> > | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> > |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> > |       |                                        ^~~~~~
> 
> That struct is defined in linux/netlink.h.
> 
> But you can add a foward declaration for this, i.e.:
> 
> struct nlattr;
> 
> As the layout isn't needed for the function declaration.

ACK, will do!

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16  9:44 ` Florian Westphal
  2025-07-16 10:01   ` Phil Sutter
@ 2025-07-16 14:38   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-16 14:38 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Phil Sutter, netfilter-devel

On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> >  #include <string.h>
> >  #include <stdlib.h>
> > +#include <libmnl/libmnl.h>
> 
> Why is this include needed?
> 
> >  #include <libnftnl/common.h>
> >  
> >  #include "config.h"
> > @@ -83,4 +84,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
> >  int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
> >  		       uint16_t attr, const void *data, uint32_t data_len);
> >  
> > +void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname);
> > +const char *mnl_attr_get_ifname(struct nlattr *attr);
> > +
> 
> nftnl_attr_put_ifname, nftnl_attr_get_ifname?
> Using mnl_ prefix seems wrong, that should be reserved for libmnl.

Agreed, mnl_nft_put_ifname, I would suggest, for consistency.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 10:01   ` Phil Sutter
  2025-07-16 10:07     ` Florian Westphal
@ 2025-07-16 14:39     ` Pablo Neira Ayuso
  2025-07-16 14:42       ` Pablo Neira Ayuso
  1 sibling, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-16 14:39 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

On Wed, Jul 16, 2025 at 12:01:55PM +0200, Phil Sutter wrote:
> Hi,
> 
> On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > >  #include <string.h>
> > >  #include <stdlib.h>
> > > +#include <libmnl/libmnl.h>
> > 
> > Why is this include needed?
> 
> Because of:
> 
> | In file included from udata.c:9:
> | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> |       |                                        ^~~~~~

I think this helper belongs to src/netlink.c

> 
> > >  #include <libnftnl/common.h>
> > >  
> > >  #include "config.h"
> > > @@ -83,4 +84,7 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
> > >  int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
> > >  		       uint16_t attr, const void *data, uint32_t data_len);
> > >  
> > > +void mnl_attr_put_ifname(struct nlmsghdr *nlh, int attr, const char *ifname);
> > > +const char *mnl_attr_get_ifname(struct nlattr *attr);
> > > +
> > 
> > nftnl_attr_put_ifname, nftnl_attr_get_ifname?
> > Using mnl_ prefix seems wrong, that should be reserved for libmnl.
> 
> I chose the prefix since they 1:1 replace calls to mnl_attr_put_strz() and
> mnl_attr_get_str(). You're right, since they are implemented by libnftnl the
> prefix is wrong. I was a bit undecided whether to put them into libmnl, but
> they are a bit too specific for that. Will fix the prefix and respin!

I think they are fine in nftables only.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 14:39     ` Pablo Neira Ayuso
@ 2025-07-16 14:42       ` Pablo Neira Ayuso
  2025-07-16 14:45         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-16 14:42 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

On Wed, Jul 16, 2025 at 04:39:54PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jul 16, 2025 at 12:01:55PM +0200, Phil Sutter wrote:
> > Hi,
> > 
> > On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > >  #include <string.h>
> > > >  #include <stdlib.h>
> > > > +#include <libmnl/libmnl.h>
> > > 
> > > Why is this include needed?
> > 
> > Because of:
> > 
> > | In file included from udata.c:9:
> > | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> > |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> > |       |                                        ^~~~~~
> 
> I think this helper belongs to src/netlink.c

Not a deal breaker, Florian's proposal is also fine.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 14:42       ` Pablo Neira Ayuso
@ 2025-07-16 14:45         ` Pablo Neira Ayuso
  2025-07-16 16:12           ` Phil Sutter
  0 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-16 14:45 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

On Wed, Jul 16, 2025 at 04:43:02PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jul 16, 2025 at 04:39:54PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Jul 16, 2025 at 12:01:55PM +0200, Phil Sutter wrote:
> > > Hi,
> > > 
> > > On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > > > Phil Sutter <phil@nwl.cc> wrote:
> > > > >  #include <string.h>
> > > > >  #include <stdlib.h>
> > > > > +#include <libmnl/libmnl.h>
> > > > 
> > > > Why is this include needed?
> > > 
> > > Because of:
> > > 
> > > | In file included from udata.c:9:
> > > | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> > > |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> > > |       |                                        ^~~~~~
> > 
> > I think this helper belongs to src/netlink.c
> 
> Not a deal breaker, Florian's proposal is also fine.

Actually, my suggestion would be to add it to nftables/src/mnl.c with
mnl_nft_ prefix (not to src/netlink.c), if it turns out this is useful
to other projects, it could be moved there to libnftnl.

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

* Re: [libnftnl PATCH v2] utils: Add helpers for interface name wildcards
  2025-07-16 14:45         ` Pablo Neira Ayuso
@ 2025-07-16 16:12           ` Phil Sutter
  0 siblings, 0 replies; 10+ messages in thread
From: Phil Sutter @ 2025-07-16 16:12 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

On Wed, Jul 16, 2025 at 04:45:11PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jul 16, 2025 at 04:43:02PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Jul 16, 2025 at 04:39:54PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, Jul 16, 2025 at 12:01:55PM +0200, Phil Sutter wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Jul 16, 2025 at 11:44:59AM +0200, Florian Westphal wrote:
> > > > > Phil Sutter <phil@nwl.cc> wrote:
> > > > > >  #include <string.h>
> > > > > >  #include <stdlib.h>
> > > > > > +#include <libmnl/libmnl.h>
> > > > > 
> > > > > Why is this include needed?
> > > > 
> > > > Because of:
> > > > 
> > > > | In file included from udata.c:9:
> > > > | ../include/utils.h:88:40: warning: 'struct nlattr' declared inside parameter list will not be visible outside of this definition or declaration
> > > > |    88 | const char *mnl_attr_get_ifname(struct nlattr *attr);
> > > > |       |                                        ^~~~~~
> > > 
> > > I think this helper belongs to src/netlink.c

This was about libnftnl, I put the helpers into src/utils.c there with
nftnl_ prefix.

> > Not a deal breaker, Florian's proposal is also fine.
> 
> Actually, my suggestion would be to add it to nftables/src/mnl.c with
> mnl_nft_ prefix (not to src/netlink.c), if it turns out this is useful
> to other projects, it could be moved there to libnftnl.

I also added it to nftables' src/mnl.c with mnl_ prefix, will change
that to mnl_nft_ as suggested.

Just realized nft.8 needs an update, too. Luckily, documentation is
pretty forgiving when it comes to typos and other goofs. ;)

Cheers, Phil

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

end of thread, other threads:[~2025-07-16 16:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15 15:15 [libnftnl PATCH v2] utils: Add helpers for interface name wildcards Phil Sutter
2025-07-16  9:44 ` Florian Westphal
2025-07-16 10:01   ` Phil Sutter
2025-07-16 10:07     ` Florian Westphal
2025-07-16 10:11       ` Phil Sutter
2025-07-16 14:39     ` Pablo Neira Ayuso
2025-07-16 14:42       ` Pablo Neira Ayuso
2025-07-16 14:45         ` Pablo Neira Ayuso
2025-07-16 16:12           ` Phil Sutter
2025-07-16 14:38   ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).