[nft RFC PATCH 0/6] events

[nft RFC PATCH 0/6] events

[Arturo Borrero Gonzalez]

This series implements basic event reporting in the nftables CLI tool.

The first patches are some neccesary code factorization changes.
The last patch is the event reporting itself.

Its quite simple, the syntax is:
 % nft event <all|table|chain|rule|set> [xml|json]

To quit, fire CTRL+C (^C).

Currently, 3 possible output formats:
 * A basic XML, provided by libnftnl.
 * A basic JSON, provided by libnftnl.
 * nft default-like syntax.

About this last format:

Rules are hard to print exactly as the user typed because sets and other
nuances. Possible solutions I've found:
 - assume that an anonymous set event will happen always before a new rule event.
 Cache the anon-set for the following rule event.
 Maybe there are many anon-sets per rule.
 - when a rule event happens, query for sets inside the event cb.
 - for this to run smoothly, we need to keep tables info in sync with the
 kernel, so in each relevant event, the netlink_ctx is needed to be updated and
 this allows to reuse netlink_delinearize_rule().

But I think this first approach is valid.

So, the format with this series is as follow:

 % nft event all
delete table ip6 filter
add table ip6 filter
add chain ip6 filter input { type filter hook input priority 0;}
add chain ip6 filter forward { type filter hook forward priority 0;}
add chain ip6 filter output { type filter hook output priority 0;}
add rule ip6 filter input handle 4
delete rule ip6 filter input handle 4
add set ip6 filter set1 {type ipv6_address}
delete chain ip6 filter input
delete set ip6 filter set1

NOTE: no way to add comments in JSON, so I decided to add that bash-like
comment by now and be consistent in both formats.

 % nft event all xml
<table><name>test</name><family>arp</family><flags>0</flags><use>0</use></table>        # add table
<chain><name>c</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>test</table><family>arp</family></chain>      # add chain
<chain><name>c</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>test</table><family>arp</family></chain>      # del chain
<table><name>test</name><family>arp</family><flags>0</flags><use>0</use></table>        # del table

 % nft event all json
{"table":{"name":"test","family":"arp","flags":0,"use":0}}      # add table
{"set":{"name":"set123","table":"test","flags":0,"family":"arp","key_type":7,"key_len":4}}      # add set

Please comment.

---

Arturo Borrero Gonzalez (6):
      rule: make family2str() public
      rule: allow to print sets in plain format
      netlink: add netlink_delinearize_set() func
      rule: generalize chain_print()
      netlink: add netlink_delinearize_rule() func
      src: add events reporting


 include/mnl.h             |    3 +
 include/netlink.h         |   15 +++
 include/rule.h            |    7 +
 src/evaluate.c            |    1 
 src/mnl.c                 |   45 +++++---
 src/netlink.c             |  268 ++++++++++++++++++++++++++++++++++++++-------
 src/netlink_delinearize.c |   57 ++++++++++
 src/parser.y              |   60 ++++++++++
 src/rule.c                |   93 +++++++++++++---
 src/scanner.l             |    2 
 10 files changed, 477 insertions(+), 74 deletions(-)

-- 
Arturo Borrero Gonzalez

[nft RFC PATCH 1/6] rule: make family2str() public

[Arturo Borrero Gonzalez]

Make family2str() public so the function can be called from
other files.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/rule.h |    2 ++
 src/rule.c     |    2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/rule.h b/include/rule.h
index e06444e..4891dc1 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -125,6 +125,8 @@ extern void chain_add_hash(struct chain *chain, struct table *table);
 extern struct chain *chain_lookup(const struct table *table,
 				  const struct handle *h);
 
+extern const char *family2str(unsigned int family);
+
 /**
  * struct rule - nftables rule
  *
diff --git a/src/rule.c b/src/rule.c
index ab96e62..9ff2dd3 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -279,7 +279,7 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
 	return NULL;
 }
 
-static const char *family2str(unsigned int family)
+const char *family2str(unsigned int family)
 {
 	switch (family) {
 		case NFPROTO_IPV4:

Re: [nft RFC PATCH 1/6] rule: make family2str() public

[Pablo Neira Ayuso]

On Tue, Feb 18, 2014 at 12:18:11AM +0100, Arturo Borrero Gonzalez wrote:
> Make family2str() public so the function can be called from
> other files.

This patch is too small and we follow a different policy when making
patches: Export function and first usage should happen in the same
patch.

I think you can collapse this patch with 1/2.

> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  include/rule.h |    2 ++
>  src/rule.c     |    2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index e06444e..4891dc1 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -125,6 +125,8 @@ extern void chain_add_hash(struct chain *chain, struct table *table);
>  extern struct chain *chain_lookup(const struct table *table,
>  				  const struct handle *h);
>  
> +extern const char *family2str(unsigned int family);
> +
>  /**
>   * struct rule - nftables rule
>   *
> diff --git a/src/rule.c b/src/rule.c
> index ab96e62..9ff2dd3 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -279,7 +279,7 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
>  	return NULL;
>  }
>  
> -static const char *family2str(unsigned int family)
> +const char *family2str(unsigned int family)
>  {
>  	switch (family) {
>  		case NFPROTO_IPV4:
> 

[nft RFC PATCH 2/6] rule: allow to print sets in plain format

[Arturo Borrero Gonzalez]

Allow to print sets with or without format.

This is useful in situations where we want to print exactly the same the user
typed (IOW, in one single line, and with family/table info).

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/rule.h |    1 +
 src/rule.c     |   44 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 4891dc1..b263593 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -195,6 +195,7 @@ extern void set_free(struct set *set);
 extern void set_add_hash(struct set *set, struct table *table);
 extern struct set *set_lookup(const struct table *table, const char *name);
 extern void set_print(const struct set *set);
+extern void set_print_plain(const struct set *s);
 
 /**
  * enum cmd_ops - command operations
diff --git a/src/rule.c b/src/rule.c
index 9ff2dd3..2c684d8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -90,21 +90,30 @@ struct set *set_lookup(const struct table *table, const char *name)
 	return NULL;
 }
 
-void set_print(const struct set *set)
+static void do_set_print(const struct set *set, const char *tab,
+			 const char *nl, const char *family, const char *table)
 {
 	const char *delim = "";
 	const char *type;
 
 	type = set->flags & SET_F_MAP ? "map" : "set";
-	printf("\t%s %s {\n", type, set->handle.set);
+	printf("%s%s", tab, type);
 
-	printf("\t\ttype %s", set->keytype->name);
+	if (family != NULL)
+		printf(" %s", family);
+
+	if (table != NULL)
+		printf(" %s", table);
+
+	printf(" %s {%s", set->handle.set, nl);
+
+	printf("%s%stype %s", tab, tab, set->keytype->name);
 	if (set->flags & SET_F_MAP)
 		printf(" : %s", set->datatype->name);
-	printf("\n");
+	printf("%s", nl);
 
 	if (set->flags & (SET_F_CONSTANT | SET_F_INTERVAL)) {
-		printf("\t\tflags ");
+		printf("%s%sflags ", tab, tab);
 		if (set->flags & SET_F_CONSTANT) {
 			printf("%sconstant", delim);
 			delim = ",";
@@ -113,15 +122,32 @@ void set_print(const struct set *set)
 			printf("%sinterval", delim);
 			delim = ",";
 		}
-		printf("\n");
+		printf("%s", nl);
 	}
 
 	if (set->init != NULL && set->init->size > 0) {
-		printf("\t\telements = ");
+		printf("%s%selements = ", tab, tab);
 		expr_print(set->init);
-		printf("\n");
+		printf("%s", nl);
 	}
-	printf("\t}\n");
+	printf("%s}%s", tab, nl);
+}
+
+void set_print(const struct set *s)
+{
+	const char *tab = "\t";
+	const char *nl = "\n";
+
+	do_set_print(s, tab, nl, NULL, NULL);
+}
+
+void set_print_plain(const struct set *s)
+{
+	const char *tab = "";
+	const char *nl = "";
+
+	do_set_print(s, tab, nl, family2str(s->handle.family),
+		     s->handle.table);
 }
 
 struct rule *rule_alloc(const struct location *loc, const struct handle *h)

Re: [nft RFC PATCH 2/6] rule: allow to print sets in plain format

[Patrick McHardy]

On Tue, Feb 18, 2014 at 12:18:17AM +0100, Arturo Borrero Gonzalez wrote:
> Allow to print sets with or without format.
> 
> This is useful in situations where we want to print exactly the same the user
> typed (IOW, in one single line, and with family/table info).

I like that, I wanted to make the indentation level variable anyway to
avoid deep indentation for set only output. Also I was considering a
pretty print function that neatly aligned all members.

Small note: we never know for sure we're generating what the user typed.
He might have used line continuations, bitwise operators etc.


> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -90,21 +90,30 @@ struct set *set_lookup(const struct table *table, const char *name)
>  	return NULL;
>  }
>  
> -void set_print(const struct set *set)
> +static void do_set_print(const struct set *set, const char *tab,
> +			 const char *nl, const char *family, const char *table)

Maybe encapsulate the tab/nl arguments into a struct. That will reduce
the needed changes when the indentation level is added.

Regarding newlines - it serves to purposes that probably need to be
handled differently.

- set family table name {\n

In this case its just for better readability.

- type bla\n

In this case its for making the output parsable. The parser expects
a stmt_seperator, which can be either \n or ;.

So I guess we need both a nl and a seperator argument.

Furthermore, I think a char (instead of a char *) should do fine.

[nft RFC PATCH 3/6] netlink: add netlink_delinearize_set() func

[Arturo Borrero Gonzalez]

Let's factorize this code, so it can be reused.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/netlink.h         |    4 ++++
 src/netlink.c             |   22 ++--------------------
 src/netlink_delinearize.c |   27 +++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/include/netlink.h b/include/netlink.h
index 4e3f8aa..13d01f0 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -70,6 +70,10 @@ extern int netlink_linearize_rule(struct netlink_ctx *ctx,
 extern struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 					     const struct nft_rule *r);
 
+extern const struct datatype *dtype_map_from_kernel(enum nft_data_types type);
+
+extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
+					   struct nft_set *nls);
 extern int netlink_add_rule(struct netlink_ctx *ctx, const struct handle *h,
 			    const struct rule *rule, uint32_t flags);
 extern int netlink_delete_rule(struct netlink_ctx *ctx, const struct handle *h,
diff --git a/src/netlink.c b/src/netlink.c
index 07af1cb..ea6611a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -731,7 +731,7 @@ static enum nft_data_types dtype_map_to_kernel(const struct datatype *dtype)
 	}
 }
 
-static const struct datatype *dtype_map_from_kernel(enum nft_data_types type)
+const struct datatype *dtype_map_from_kernel(enum nft_data_types type)
 {
 	switch (type) {
 	case NFT_DATA_VERDICT:
@@ -870,7 +870,6 @@ int netlink_get_set(struct netlink_ctx *ctx, const struct handle *h,
 		    const struct location *loc)
 {
 	struct nft_set *nls;
-	struct set *set;
 	int err;
 
 	nls = alloc_nft_set(h);
@@ -881,24 +880,7 @@ int netlink_get_set(struct netlink_ctx *ctx, const struct handle *h,
 					"Could not receive set from kernel: %s",
 					strerror(errno));
 
-	set = set_alloc(&netlink_location);
-	set->handle.family = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
-	set->handle.table  =
-		xstrdup(nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE));
-	set->handle.set    =
-		xstrdup(nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME));
-	set->keytype       =
-		 dtype_map_from_kernel(nft_set_attr_get_u32(nls, NFT_SET_ATTR_KEY_TYPE));
-	set->keylen        =
-		nft_set_attr_get_u32(nls, NFT_SET_ATTR_KEY_LEN) * BITS_PER_BYTE;
-	set->flags         = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
-	set->datatype      =
-		dtype_map_from_kernel(nft_set_attr_get_u32(nls, NFT_SET_ATTR_DATA_TYPE));
-	if (nft_set_attr_is_set(nls, NFT_SET_ATTR_DATA_LEN)) {
-		set->datalen =
-			nft_set_attr_get_u32(nls, NFT_SET_ATTR_DATA_LEN) * BITS_PER_BYTE;
-	}
-	list_add_tail(&set->list, &ctx->list);
+	netlink_delinearize_set(ctx, nls);
 	nft_set_free(nls);
 
 	return err;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 0e75c8a..65d1c80 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -895,3 +895,30 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 	rule_parse_postprocess(pctx, pctx->rule);
 	return pctx->rule;
 }
+
+struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
+				    struct nft_set *nls)
+{
+	struct set *set;
+
+	set = set_alloc(&internal_location);
+	set->handle.family = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
+	set->handle.table  =
+		xstrdup(nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE));
+	set->handle.set    =
+		xstrdup(nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME));
+	set->keytype       =
+		 dtype_map_from_kernel(nft_set_attr_get_u32(nls, NFT_SET_ATTR_KEY_TYPE));
+	set->keylen        =
+		nft_set_attr_get_u32(nls, NFT_SET_ATTR_KEY_LEN) * BITS_PER_BYTE;
+	set->flags         = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
+	set->datatype      =
+		dtype_map_from_kernel(nft_set_attr_get_u32(nls, NFT_SET_ATTR_DATA_TYPE));
+	if (nft_set_attr_is_set(nls, NFT_SET_ATTR_DATA_LEN)) {
+		set->datalen =
+			nft_set_attr_get_u32(nls, NFT_SET_ATTR_DATA_LEN) * BITS_PER_BYTE;
+	}
+	list_add_tail(&set->list, &ctx->list);
+
+	return set;
+}

Re: [nft RFC PATCH 3/6] netlink: add netlink_delinearize_set() func

[Patrick McHardy]

On Tue, Feb 18, 2014 at 12:18:22AM +0100, Arturo Borrero Gonzalez wrote:
> Let's factorize this code, so it can be reused.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  include/netlink.h         |    4 ++++
>  src/netlink.c             |   22 ++--------------------
>  src/netlink_delinearize.c |   27 +++++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/include/netlink.h b/include/netlink.h
> index 4e3f8aa..13d01f0 100644
> --- a/include/netlink.h
> +++ b/include/netlink.h
> @@ -70,6 +70,10 @@ extern int netlink_linearize_rule(struct netlink_ctx *ctx,
>  extern struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
>  					     const struct nft_rule *r);
>  
> +extern const struct datatype *dtype_map_from_kernel(enum nft_data_types type);
> +
> +extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
> +					   struct nft_set *nls);

Actually delinarization so far refers to the act of converting a linear
instruction stream into an abstract syntax tree. This is not really a
good term for sets, which are still pretty much linear.

I don't have a good suggestion right now though.

Re: [nft RFC PATCH 3/6] netlink: add netlink_delinearize_set() func

[Arturo Borrero Gonzalez]

On 18 February 2014 02:56, Patrick McHardy <kaber@trash.net> wrote:
>> +extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
>> +                                        struct nft_set *nls);
>
> Actually delinarization so far refers to the act of converting a linear
> instruction stream into an abstract syntax tree. This is not really a
> good term for sets, which are still pretty much linear.
>
> I don't have a good suggestion right now though.

At first, I used something like 'nls2set', but this delinearize thing
sounded more 'serious' :)

In think in all cases, we are converting netlink objects to the
internal nft object. We need a name for that, no?

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft RFC PATCH 3/6] netlink: add netlink_delinearize_set() func

[Patrick McHardy]

On Tue, Feb 18, 2014 at 10:11:52AM +0100, Arturo Borrero Gonzalez wrote:
> On 18 February 2014 02:56, Patrick McHardy <kaber@trash.net> wrote:
> >> +extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
> >> +                                        struct nft_set *nls);
> >
> > Actually delinarization so far refers to the act of converting a linear
> > instruction stream into an abstract syntax tree. This is not really a
> > good term for sets, which are still pretty much linear.
> >
> > I don't have a good suggestion right now though.
> 
> At first, I used something like 'nls2set', but this delinearize thing
> sounded more 'serious' :)
> 
> In think in all cases, we are converting netlink objects to the
> internal nft object. We need a name for that, no?

You *could* call it deserialization since we're converting the attribute
stream into an object and we should (not done so far) convert bitmask
values with more than one bit set into a tree of inclusive OR expressions.

Not sure. I guess you can keep that name until we come up with something
better, but I wouldn't move it to netlink_delinearize.c. This is meant to
mirror netlink_linearize.c, they both only take care of rules.

[nft RFC PATCH 4/6] rule: generalize chain_print()

[Arturo Borrero Gonzalez]

Lest generalize the chain_print() function, so we can print a plain chain
as the user typed in the basic CLI.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/rule.h |    1 +
 src/rule.c     |   33 +++++++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index b263593..9791cea 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -126,6 +126,7 @@ extern struct chain *chain_lookup(const struct table *table,
 				  const struct handle *h);
 
 extern const char *family2str(unsigned int family);
+extern void chain_print_plain(const struct chain *chain);
 
 /**
  * struct rule - nftables rule
diff --git a/src/rule.c b/src/rule.c
index 2c684d8..4301faa 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -364,21 +364,42 @@ static const char *hooknum2str(unsigned int family, unsigned int hooknum)
 	return "unknown";
 }
 
-static void chain_print(const struct chain *chain)
+static void do_chain_print(const struct chain *chain, const char *family,
+			   const char *table, const char *tab, const char *nl)
 {
 	struct rule *rule;
 
-	printf("\tchain %s {\n", chain->handle.chain);
+	printf("%schain", tab);
+
+	if (family != NULL)
+		printf(" %s", family);
+
+	if (table != NULL)
+		printf(" %s", table);
+
+	printf(" %s {%s", chain->handle.chain, nl);
 	if (chain->flags & CHAIN_F_BASECHAIN) {
-		printf("\t\t type %s hook %s priority %u;\n", chain->type,
+		printf("%s%s type %s hook %s priority %u;\%s", tab, tab,
+		       chain->type,
 		       hooknum2str(chain->handle.family, chain->hooknum),
-		       chain->priority);
+		       chain->priority, nl);
 	}
 	list_for_each_entry(rule, &chain->rules, list) {
-		printf("\t\t");
+		printf("%s%s", tab, tab);
 		rule_print(rule);
 	}
-	printf("\t}\n");
+	printf("%s}%s", tab, nl);
+}
+
+static void chain_print(const struct chain *chain)
+{
+	do_chain_print(chain, NULL, NULL, "\t", "\n");
+}
+
+void chain_print_plain(const struct chain *chain)
+{
+	do_chain_print(chain, family2str(chain->handle.family),
+		       chain->handle.table, "", "");
 }
 
 struct table *table_alloc(void)

[nft RFC PATCH 5/6] netlink: add netlink_delinearize_rule() func

[Arturo Borrero Gonzalez]

Let's make this code reusable.

Also, this patch fixes a hidden bug: the table in the chain's handle was being
set to the chain name.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/netlink.h         |    3 +++
 src/netlink.c             |   23 +----------------------
 src/netlink_delinearize.c |   30 ++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/include/netlink.h b/include/netlink.h
index 13d01f0..1e6655c 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -74,6 +74,9 @@ extern const struct datatype *dtype_map_from_kernel(enum nft_data_types type);
 
 extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 					   struct nft_set *nls);
+extern struct chain *netlink_delinearize_chain(struct netlink_ctx *ctx,
+					       struct nft_chain *nlc);
+
 extern int netlink_add_rule(struct netlink_ctx *ctx, const struct handle *h,
 			    const struct rule *rule, uint32_t flags);
 extern int netlink_delete_rule(struct netlink_ctx *ctx, const struct handle *h,
diff --git a/src/netlink.c b/src/netlink.c
index ea6611a..cd5e64a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -500,7 +500,6 @@ static int list_chain_cb(struct nft_chain *nlc, void *arg)
 {
 	struct netlink_ctx *ctx = arg;
 	const struct handle *h = ctx->data;
-	struct chain *chain;
 
 	if ((h->family != nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY)) ||
 	    strcmp(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE), h->table) != 0)
@@ -510,27 +509,7 @@ static int list_chain_cb(struct nft_chain *nlc, void *arg)
 	    strcmp(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME), h->chain) != 0)
 		return 0;
 
-	chain = chain_alloc(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME));
-	chain->handle.family =
-		nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY);
-	chain->handle.table  =
-		xstrdup(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME));
-	chain->handle.handle =
-		nft_chain_attr_get_u64(nlc, NFT_CHAIN_ATTR_HANDLE);
-
-	if (nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_HOOKNUM) &&
-	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_PRIO) &&
-	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_TYPE)) {
-		chain->hooknum       =
-			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_HOOKNUM);
-		chain->priority      =
-			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_PRIO);
-		chain->type          =
-			xstrdup(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TYPE));
-		chain->flags        |= CHAIN_F_BASECHAIN;
-	}
-	list_add_tail(&chain->list, &ctx->list);
-
+	netlink_delinearize_chain(ctx, nlc);
 	return 0;
 }
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 65d1c80..5c5c4b1 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -922,3 +922,33 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 
 	return set;
 }
+
+struct chain *netlink_delinearize_chain(struct netlink_ctx *ctx,
+					struct nft_chain *nlc)
+{
+	struct chain *chain;
+
+	chain = chain_alloc(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME));
+	chain->handle.family =
+		nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY);
+	chain->handle.table  =
+		xstrdup(nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE));
+	chain->handle.handle =
+		nft_chain_attr_get_u64(nlc, NFT_CHAIN_ATTR_HANDLE);
+
+	if (nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_HOOKNUM) &&
+	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_PRIO) &&
+	    nft_chain_attr_is_set(nlc, NFT_CHAIN_ATTR_TYPE)) {
+		chain->hooknum       =
+			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_HOOKNUM);
+		chain->priority      =
+			nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_PRIO);
+		chain->type          =
+			xstrdup(nft_chain_attr_get_str(nlc,
+						       NFT_CHAIN_ATTR_TYPE));
+		chain->flags        |= CHAIN_F_BASECHAIN;
+	}
+	list_add_tail(&chain->list, &ctx->list);
+
+	return chain;
+}

[nft RFC PATCH 6/6] src: add events reporting

[Arturo Borrero Gonzalez]

This patch adds a basic events reporting option to nft.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/mnl.h     |    3 +
 include/netlink.h |    8 ++
 include/rule.h    |    3 +
 src/evaluate.c    |    1 
 src/mnl.c         |   45 ++++++++---
 src/netlink.c     |  223 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/parser.y      |   60 ++++++++++++++
 src/rule.c        |   14 +++
 src/scanner.l     |    2 
 9 files changed, 343 insertions(+), 16 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index f4de27d..ece7ee7 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -67,4 +67,7 @@ int mnl_nft_setelem_get(struct mnl_socket *nf_sock, struct nft_set *nls);
 
 struct nft_ruleset *mnl_nft_ruleset_dump(struct mnl_socket *nf_sock,
 					 uint32_t family);
+int mnl_nft_event_listener(struct mnl_socket *nf_sock,
+			   int (*cb)(const struct nlmsghdr *nlh, void *data),
+			   void *cb_data);
 #endif /* _NFTABLES_MNL_H_ */
diff --git a/include/netlink.h b/include/netlink.h
index 1e6655c..70de811 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -149,4 +149,12 @@ extern int netlink_io_error(struct netlink_ctx *ctx,
 extern struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
 						const struct handle *h,
 						const struct location *loc);
+struct netlink_ev_handler {
+	uint32_t		selector;
+	uint32_t		format;
+	struct netlink_ctx	*ctx;
+	const struct location	*loc;
+};
+
+extern int netlink_events(struct netlink_ev_handler *evhandler);
 #endif /* NFTABLES_NETLINK_H */
diff --git a/include/rule.h b/include/rule.h
index 9791cea..49173df 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -210,6 +210,7 @@ extern void set_print_plain(const struct set *s);
  * @CMD_FLUSH:		flush container
  * @CMD_RENAME:		rename object
  * @CMD_EXPORT:		export the ruleset in a given format
+ * @CMD_EVENT:		event listener
  */
 enum cmd_ops {
 	CMD_INVALID,
@@ -221,6 +222,7 @@ enum cmd_ops {
 	CMD_FLUSH,
 	CMD_RENAME,
 	CMD_EXPORT,
+	CMD_EVENT,
 };
 
 /**
@@ -276,6 +278,7 @@ struct cmd {
 	};
 	const void		*arg;
 	uint32_t		format;
+	uint32_t		event_selector;
 };
 
 extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
diff --git a/src/evaluate.c b/src/evaluate.c
index f10d0d9..80d16f4 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1410,6 +1410,7 @@ int cmd_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
 	case CMD_FLUSH:
 	case CMD_RENAME:
 	case CMD_EXPORT:
+	case CMD_EVENT:
 		return 0;
 	default:
 		BUG("invalid command operation %u\n", cmd->op);
diff --git a/src/mnl.c b/src/mnl.c
index e825fb0..7e34b31 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -34,24 +34,16 @@ uint32_t mnl_seqnum_alloc(void)
 }
 
 static int
-mnl_talk(struct mnl_socket *nf_sock, const void *data, unsigned int len,
-	 int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
+mnl_talk_recv(struct mnl_socket *nf_sock, int seqnum, uint32_t portid,
+	      int (*cb)(const struct nlmsghdr *nlh, void *data),
+	      void *cb_data)
 {
-	char buf[MNL_SOCKET_BUFFER_SIZE];
-	uint32_t portid = mnl_socket_get_portid(nf_sock);
 	int ret;
-
-#ifdef DEBUG
-	if (debug_level & DEBUG_MNL)
-		mnl_nlmsg_fprintf(stdout, data, len, sizeof(struct nfgenmsg));
-#endif
-
-	if (mnl_socket_sendto(nf_sock, data, len) < 0)
-		return -1;
+	char buf[MNL_SOCKET_BUFFER_SIZE];
 
 	ret = mnl_socket_recvfrom(nf_sock, buf, sizeof(buf));
 	while (ret > 0) {
-		ret = mnl_cb_run(buf, ret, seq, portid, cb, cb_data);
+		ret = mnl_cb_run(buf, ret, seqnum, portid, cb, cb_data);
 		if (ret <= 0)
 			goto out;
 
@@ -64,6 +56,23 @@ out:
 	return ret;
 }
 
+static int
+mnl_talk(struct mnl_socket *nf_sock, const void *data, unsigned int len,
+	 int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
+{
+	uint32_t portid = mnl_socket_get_portid(nf_sock);
+
+#ifdef DEBUG
+	if (debug_level & DEBUG_MNL)
+		mnl_nlmsg_fprintf(stdout, data, len, sizeof(struct nfgenmsg));
+#endif
+
+	if (mnl_socket_sendto(nf_sock, data, len) < 0)
+		return -1;
+
+	return mnl_talk_recv(nf_sock, seq, portid, cb, cb_data);
+}
+
 /*
  * Batching
  */
@@ -805,3 +814,13 @@ out:
 	nft_ruleset_free(rs);
 	return NULL;
 }
+
+/*
+ * events
+ */
+int mnl_nft_event_listener(struct mnl_socket *nf_sock,
+			   int (*cb)(const struct nlmsghdr *nlh, void *data),
+			   void *cb_data)
+{
+	return mnl_talk_recv(nf_sock, 0, 0, cb, cb_data);
+}
diff --git a/src/netlink.c b/src/netlink.c
index cd5e64a..efa52bf 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -20,6 +20,7 @@
 #include <libnftnl/chain.h>
 #include <libnftnl/expr.h>
 #include <libnftnl/set.h>
+#include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nf_tables.h>
 #include <linux/netfilter.h>
 
@@ -1010,3 +1011,225 @@ struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
 
 	return rs;
 }
+
+static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
+				   struct netlink_ev_handler *evh)
+{
+	struct nft_table *nlt;
+	uint32_t family;
+	char buf[4096];
+
+	nlt = nft_table_alloc();
+	if (nlt == NULL)
+		memory_allocation_error();
+
+	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
+		netlink_io_error(evh->ctx, evh->loc,
+				"Could not parse table: %s",
+				strerror(errno));
+		goto err;
+	}
+
+	if (evh->format == NFT_OUTPUT_DEFAULT) {
+		if (type == NFT_MSG_NEWTABLE)
+			printf("add table ");
+		else
+			printf("delete table ");
+
+		family = nft_table_attr_get_u32(nlt, NFT_TABLE_ATTR_FAMILY);
+
+		printf("%s %s\n", family2str(family),
+		       nft_table_attr_get_str(nlt, NFT_TABLE_ATTR_NAME));
+	} else {
+		nft_table_snprintf(buf, sizeof(buf), nlt, evh->format, 0);
+		printf("%s\t# %s table\n", buf,
+		       type == NFT_MSG_NEWTABLE ? "add" : "del");
+	}
+
+err:
+	nft_table_free(nlt);
+	return MNL_CB_OK;
+}
+
+static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
+				   struct netlink_ev_handler *evh)
+{
+	struct nft_chain *nlc;
+	struct chain *c;
+	uint32_t family;
+	char buf[4096];
+
+	nlc = nft_chain_alloc();
+	if (nlc == NULL)
+		memory_allocation_error();
+
+	if (nft_chain_nlmsg_parse(nlh, nlc) < 0) {
+		netlink_io_error(evh->ctx, evh->loc,
+				 "Could not parse chain: %s",
+				 strerror(errno));
+		goto out;
+	}
+
+	if (evh->format == NFT_OUTPUT_DEFAULT) {
+		if (type == NFT_MSG_NEWCHAIN) {
+			printf("add ");
+			c = netlink_delinearize_chain(evh->ctx, nlc);
+			chain_print_plain(c);
+			printf("\n");
+			chain_free(c);
+			goto out;
+		}
+
+		printf("delete chain ");
+
+		family = nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY);
+		printf("%s %s %s\n", family2str(family),
+		       nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE),
+		       nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME));
+	} else {
+		nft_chain_snprintf(buf, sizeof(buf), nlc, evh->format, 0);
+		printf("%s\t# %s chain\n", buf,
+		       type == NFT_MSG_NEWCHAIN ? "add" : "del");
+	}
+
+out:
+	nft_chain_free(nlc);
+	return MNL_CB_OK;
+}
+
+static int netlink_events_set_cb(const struct nlmsghdr *nlh, int type,
+				 struct netlink_ev_handler *evh)
+{
+	struct nft_set *nls;
+	struct set *set;
+	uint32_t family, flags;
+	char buf[4096];
+
+	nls = nft_set_alloc();
+	if (nls == NULL)
+		memory_allocation_error();
+
+	if (nft_set_nlmsg_parse(nlh, nls) < 0) {
+		netlink_io_error(evh->ctx, evh->loc,
+				 "Could not parse set: %s",
+				 strerror(errno));
+		goto out;
+	}
+
+	flags = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
+	if (flags & SET_F_ANONYMOUS && type == NFT_MSG_DELSET)
+		goto out;
+
+	if (evh->format == NFT_OUTPUT_DEFAULT) {
+		set = netlink_delinearize_set(evh->ctx, nls);
+		family = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
+
+		if (type == NFT_MSG_NEWSET) {
+			printf("add ");
+			set_print_plain(set);
+		} else {
+			printf("delete set %s %s %s",
+			       family2str(family),
+			       nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE),
+			       nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME));
+		}
+
+		printf("\n");
+
+	} else {
+		nft_set_snprintf(buf, sizeof(buf), nls, evh->format, 0);
+		printf("%s\t# %s set\n", buf,
+		       type == NFT_MSG_NEWSET ? "add" : "del");
+	}
+
+out:
+	nft_set_free(nls);
+	return MNL_CB_OK;
+}
+
+static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
+				  struct netlink_ev_handler *evh)
+{
+	struct nft_rule *nlr;
+	uint32_t family, handle;
+	char buf[4096];
+
+	nlr = nft_rule_alloc();
+	if (nlr == NULL)
+		memory_allocation_error();
+
+	if (nft_rule_nlmsg_parse(nlh, nlr) < 0) {
+		netlink_io_error(evh->ctx, evh->loc,
+				 "Could not parse rule: %s",
+				 strerror(errno));
+		goto out;
+	}
+
+	if (evh->format == NFT_OUTPUT_DEFAULT) {
+		family = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_FAMILY);
+		handle = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_HANDLE);
+
+		if (type == NFT_MSG_NEWRULE)
+			printf("add rule ");
+		else
+			printf("delete rule ");
+
+		printf("%s %s %s handle %u\n", family2str(family),
+		       nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_TABLE),
+		       nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_CHAIN),
+		       handle);
+	} else {
+		nft_rule_snprintf(buf, sizeof(buf), nlr, evh->format, 0);
+		printf("%s\t# %s rule\n", buf,
+		       type == NFT_MSG_NEWRULE ? "add" : "del");
+	}
+
+out:
+	nft_rule_free(nlr);
+	return MNL_CB_OK;
+}
+
+static int netlink_events_cb(const struct nlmsghdr *nlh, void *data)
+{
+	int ret = MNL_CB_OK;
+	int type = nlh->nlmsg_type & 0xFF;
+	struct netlink_ev_handler *evh = (struct netlink_ev_handler *)data;
+
+	if (!(evh->selector & (1 << type)))
+		return ret;
+
+	switch (type) {
+	case NFT_MSG_NEWTABLE:
+	case NFT_MSG_DELTABLE:
+		ret = netlink_events_table_cb(nlh, type, evh);
+		break;
+	case NFT_MSG_NEWCHAIN:
+	case NFT_MSG_DELCHAIN:
+		ret = netlink_events_chain_cb(nlh, type, evh);
+		break;
+	case NFT_MSG_NEWSET:
+	case NFT_MSG_DELSET:
+		ret = netlink_events_set_cb(nlh, type, evh);
+		break;
+	case NFT_MSG_NEWRULE:
+	case NFT_MSG_DELRULE:
+		ret = netlink_events_rule_cb(nlh, type, evh);
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+int netlink_events(struct netlink_ev_handler *evhandler)
+{
+	if (mnl_socket_bind(nf_sock, (1 << (NFNLGRP_NFTABLES-1)),
+			    MNL_SOCKET_AUTOPID) < 0)
+		return netlink_io_error(evhandler->ctx, evhandler->loc,
+					"Could not bind to socket: %s",
+					strerror(errno));
+
+	fcntl(mnl_socket_get_fd(nf_sock), F_SETFL, 0);
+	return mnl_nft_event_listener(nf_sock, netlink_events_cb, evhandler);
+}
diff --git a/src/parser.y b/src/parser.y
index 07613e2..79a42ac 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -169,6 +169,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token ELEMENT			"element"
 %token MAP			"map"
 %token HANDLE			"handle"
+%token ALL			"all"
 
 %token INET			"inet"
 
@@ -181,6 +182,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token RENAME			"rename"
 %token DESCRIBE			"describe"
 %token EXPORT			"export"
+%token EVENT			"event"
 
 %token ACCEPT			"accept"
 %token DROP			"drop"
@@ -360,8 +362,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <cmd>			line
 %destructor { cmd_free($$); }	line
 
-%type <cmd>			base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd
-%destructor { cmd_free($$); }	base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd
+%type <cmd>			base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd event_cmd
+%destructor { cmd_free($$); }	base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd event_cmd
 
 %type <handle>			table_spec tables_spec chain_spec chain_identifier ruleid_spec
 %destructor { handle_free(&$$); } table_spec tables_spec chain_spec chain_identifier ruleid_spec
@@ -376,6 +378,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <rule>			rule
 %destructor { rule_free($$); }	rule
 
+%type <val>			event_selector
+
 %type <val>			set_flag_list	set_flag
 
 %type <set>			set_block_alloc set_block
@@ -484,7 +488,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %destructor { expr_free($$); }	ct_expr
 %type <val>			ct_key
 
-%type <val>			export_format
+%type <val>			export_format	event_format
 
 %%
 
@@ -584,6 +588,7 @@ base_cmd		:	/* empty */	add_cmd		{ $$ = $1; }
 			|	FLUSH		flush_cmd	{ $$ = $2; }
 			|	RENAME		rename_cmd	{ $$ = $2; }
 			|	EXPORT		export_cmd	{ $$ = $2; }
+			|	EVENT		event_cmd	{ $$ = $2; }
 			|	DESCRIBE	primary_expr
 			{
 				expr_describe($2);
@@ -751,6 +756,55 @@ export_cmd		:	export_format
 			}
 			;
 
+event_cmd		:	event_selector	event_format
+			{
+				struct handle h = { .family = NFPROTO_UNSPEC };
+				$$ = cmd_alloc(CMD_EVENT, CMD_OBJ_RULESET, &h, &@$, NULL);
+				$$->event_selector = $1;
+				$$->format = $2;
+			}
+			;
+
+event_selector		:	ALL
+			{
+				$$ |= (1 << NFT_MSG_NEWRULE);
+				$$ |= (1 << NFT_MSG_DELRULE);
+				$$ |= (1 << NFT_MSG_NEWSET);
+				$$ |= (1 << NFT_MSG_DELSET);
+				$$ |= (1 << NFT_MSG_NEWCHAIN);
+				$$ |= (1 << NFT_MSG_DELCHAIN);
+				$$ |= (1 << NFT_MSG_NEWTABLE);
+				$$ |= (1 << NFT_MSG_DELTABLE);
+			}
+			|	TABLE
+			{
+				$$ |= (1 << NFT_MSG_NEWTABLE);
+				$$ |= (1 << NFT_MSG_DELTABLE);
+			}
+			|	CHAIN
+			{
+				$$ |= (1 << NFT_MSG_NEWCHAIN);
+				$$ |= (1 << NFT_MSG_DELCHAIN);
+			}
+			|	SET
+			{
+				$$ |= (1 << NFT_MSG_NEWSET);
+				$$ |= (1 << NFT_MSG_DELSET);
+			}
+			|	RULE
+			{
+				$$ |= (1 << NFT_MSG_NEWRULE);
+				$$ |= (1 << NFT_MSG_DELRULE);
+			}
+			;
+
+event_format		:	/* empty */
+			{
+				$$ = NFT_OUTPUT_DEFAULT;
+			}
+			|	export_format
+			;
+
 table_block_alloc	:	/* empty */
 			{
 				$$ = table_alloc();
diff --git a/src/rule.c b/src/rule.c
index 4301faa..30a8529 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -790,6 +790,18 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 	return 0;
 }
 
+static int do_command_event(struct netlink_ctx *ctx, struct cmd *cmd)
+{
+	struct netlink_ev_handler evhandler;
+
+	evhandler.selector = cmd->event_selector;
+	evhandler.format = cmd->format;
+	evhandler.ctx = ctx;
+	evhandler.loc = &cmd->location;
+
+	return netlink_events(&evhandler);
+}
+
 int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	switch (cmd->op) {
@@ -809,6 +821,8 @@ int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
 		return do_command_rename(ctx, cmd);
 	case CMD_EXPORT:
 		return do_command_export(ctx, cmd);
+	case CMD_EVENT:
+		return do_command_event(ctx, cmd);
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
 	}
diff --git a/src/scanner.l b/src/scanner.l
index e4cb398..70b781a 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -238,6 +238,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "element"		{ return ELEMENT; }
 "map"			{ return MAP; }
 "handle"		{ return HANDLE; }
+"all"			{ return ALL; }
 
 "accept"		{ return ACCEPT; }
 "drop"			{ return DROP; }
@@ -256,6 +257,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "flush"			{ return FLUSH; }
 "rename"		{ return RENAME; }
 "export"		{ return EXPORT; }
+"event"			{ return EVENT; }
 
 "position"		{ return POSITION; }
 

Re: [nft RFC PATCH 6/6] src: add events reporting

[Pablo Neira Ayuso]

On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> This patch adds a basic events reporting option to nft.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  include/mnl.h     |    3 +
>  include/netlink.h |    8 ++
>  include/rule.h    |    3 +
>  src/evaluate.c    |    1 
>  src/mnl.c         |   45 ++++++++---
>  src/netlink.c     |  223 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/parser.y      |   60 ++++++++++++++
>  src/rule.c        |   14 +++
>  src/scanner.l     |    2 
>  9 files changed, 343 insertions(+), 16 deletions(-)
> 
> diff --git a/include/mnl.h b/include/mnl.h
> index f4de27d..ece7ee7 100644
> --- a/include/mnl.h
> +++ b/include/mnl.h
> @@ -67,4 +67,7 @@ int mnl_nft_setelem_get(struct mnl_socket *nf_sock, struct nft_set *nls);
>  
>  struct nft_ruleset *mnl_nft_ruleset_dump(struct mnl_socket *nf_sock,
>  					 uint32_t family);
> +int mnl_nft_event_listener(struct mnl_socket *nf_sock,
> +			   int (*cb)(const struct nlmsghdr *nlh, void *data),
> +			   void *cb_data);
>  #endif /* _NFTABLES_MNL_H_ */
> diff --git a/include/netlink.h b/include/netlink.h
> index 1e6655c..70de811 100644
> --- a/include/netlink.h
> +++ b/include/netlink.h
> @@ -149,4 +149,12 @@ extern int netlink_io_error(struct netlink_ctx *ctx,
>  extern struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
>  						const struct handle *h,
>  						const struct location *loc);
> +struct netlink_ev_handler {
> +	uint32_t		selector;
> +	uint32_t		format;
> +	struct netlink_ctx	*ctx;
> +	const struct location	*loc;
> +};
> +
> +extern int netlink_events(struct netlink_ev_handler *evhandler);
>  #endif /* NFTABLES_NETLINK_H */
> diff --git a/include/rule.h b/include/rule.h
> index 9791cea..49173df 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -210,6 +210,7 @@ extern void set_print_plain(const struct set *s);
>   * @CMD_FLUSH:		flush container
>   * @CMD_RENAME:		rename object
>   * @CMD_EXPORT:		export the ruleset in a given format
> + * @CMD_EVENT:		event listener
>   */
>  enum cmd_ops {
>  	CMD_INVALID,
> @@ -221,6 +222,7 @@ enum cmd_ops {
>  	CMD_FLUSH,
>  	CMD_RENAME,
>  	CMD_EXPORT,
> +	CMD_EVENT,
>  };
>  
>  /**
> @@ -276,6 +278,7 @@ struct cmd {
>  	};
>  	const void		*arg;
>  	uint32_t		format;
> +	uint32_t		event_selector;
>  };
>  
>  extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
> diff --git a/src/evaluate.c b/src/evaluate.c
> index f10d0d9..80d16f4 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1410,6 +1410,7 @@ int cmd_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
>  	case CMD_FLUSH:
>  	case CMD_RENAME:
>  	case CMD_EXPORT:
> +	case CMD_EVENT:
>  		return 0;
>  	default:
>  		BUG("invalid command operation %u\n", cmd->op);
> diff --git a/src/mnl.c b/src/mnl.c
> index e825fb0..7e34b31 100644
> --- a/src/mnl.c
> +++ b/src/mnl.c
> @@ -34,24 +34,16 @@ uint32_t mnl_seqnum_alloc(void)
>  }
>  
>  static int
> -mnl_talk(struct mnl_socket *nf_sock, const void *data, unsigned int len,
> -	 int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
> +mnl_talk_recv(struct mnl_socket *nf_sock, int seqnum, uint32_t portid,
> +	      int (*cb)(const struct nlmsghdr *nlh, void *data),
> +	      void *cb_data)
>  {
> -	char buf[MNL_SOCKET_BUFFER_SIZE];
> -	uint32_t portid = mnl_socket_get_portid(nf_sock);
>  	int ret;
> -
> -#ifdef DEBUG
> -	if (debug_level & DEBUG_MNL)
> -		mnl_nlmsg_fprintf(stdout, data, len, sizeof(struct nfgenmsg));
> -#endif
> -
> -	if (mnl_socket_sendto(nf_sock, data, len) < 0)
> -		return -1;
> +	char buf[MNL_SOCKET_BUFFER_SIZE];
>  
>  	ret = mnl_socket_recvfrom(nf_sock, buf, sizeof(buf));
>  	while (ret > 0) {
> -		ret = mnl_cb_run(buf, ret, seq, portid, cb, cb_data);
> +		ret = mnl_cb_run(buf, ret, seqnum, portid, cb, cb_data);
>  		if (ret <= 0)
>  			goto out;
>  
> @@ -64,6 +56,23 @@ out:
>  	return ret;
>  }
>  
> +static int
> +mnl_talk(struct mnl_socket *nf_sock, const void *data, unsigned int len,
> +	 int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
> +{
> +	uint32_t portid = mnl_socket_get_portid(nf_sock);
> +
> +#ifdef DEBUG
> +	if (debug_level & DEBUG_MNL)
> +		mnl_nlmsg_fprintf(stdout, data, len, sizeof(struct nfgenmsg));
> +#endif
> +
> +	if (mnl_socket_sendto(nf_sock, data, len) < 0)
> +		return -1;
> +
> +	return mnl_talk_recv(nf_sock, seq, portid, cb, cb_data);
> +}
> +
>  /*
>   * Batching
>   */
> @@ -805,3 +814,13 @@ out:
>  	nft_ruleset_free(rs);
>  	return NULL;
>  }
> +
> +/*
> + * events
> + */
> +int mnl_nft_event_listener(struct mnl_socket *nf_sock,
> +			   int (*cb)(const struct nlmsghdr *nlh, void *data),
> +			   void *cb_data)
> +{
> +	return mnl_talk_recv(nf_sock, 0, 0, cb, cb_data);
> +}
> diff --git a/src/netlink.c b/src/netlink.c
> index cd5e64a..efa52bf 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -20,6 +20,7 @@
>  #include <libnftnl/chain.h>
>  #include <libnftnl/expr.h>
>  #include <libnftnl/set.h>
> +#include <linux/netfilter/nfnetlink.h>
>  #include <linux/netfilter/nf_tables.h>
>  #include <linux/netfilter.h>
>  
> @@ -1010,3 +1011,225 @@ struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
>  
>  	return rs;
>  }
> +
> +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> +				   struct netlink_ev_handler *evh)
> +{
> +	struct nft_table *nlt;
> +	uint32_t family;
> +	char buf[4096];
> +
> +	nlt = nft_table_alloc();
> +	if (nlt == NULL)
> +		memory_allocation_error();
> +
> +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> +		netlink_io_error(evh->ctx, evh->loc,
> +				"Could not parse table: %s",
> +				strerror(errno));

I think you should exit on parsing errors.

> +		goto err;
> +	}
> +
> +	if (evh->format == NFT_OUTPUT_DEFAULT) {
> +		if (type == NFT_MSG_NEWTABLE)
> +			printf("add table ");
> +		else
> +			printf("delete table ");
> +
> +		family = nft_table_attr_get_u32(nlt, NFT_TABLE_ATTR_FAMILY);
> +
> +		printf("%s %s\n", family2str(family),
> +		       nft_table_attr_get_str(nlt, NFT_TABLE_ATTR_NAME));
> +	} else {
> +		nft_table_snprintf(buf, sizeof(buf), nlt, evh->format, 0);
> +		printf("%s\t# %s table\n", buf,
> +		       type == NFT_MSG_NEWTABLE ? "add" : "del");
> +	}
> +
> +err:
> +	nft_table_free(nlt);
> +	return MNL_CB_OK;
> +}
> +
> +static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
> +				   struct netlink_ev_handler *evh)
> +{
> +	struct nft_chain *nlc;
> +	struct chain *c;
> +	uint32_t family;
> +	char buf[4096];
> +
> +	nlc = nft_chain_alloc();
> +	if (nlc == NULL)
> +		memory_allocation_error();
> +
> +	if (nft_chain_nlmsg_parse(nlh, nlc) < 0) {
> +		netlink_io_error(evh->ctx, evh->loc,
> +				 "Could not parse chain: %s",
> +				 strerror(errno));
> +		goto out;
> +	}
> +
> +	if (evh->format == NFT_OUTPUT_DEFAULT) {
> +		if (type == NFT_MSG_NEWCHAIN) {

         Better use switch (type) {
                        case NFT_MSG_NEWCHAIN:
                                ...

> +			printf("add ");
> +			c = netlink_delinearize_chain(evh->ctx, nlc);
> +			chain_print_plain(c);
> +			printf("\n");
> +			chain_free(c);
> +			goto out;
> +		}
> +
> +		printf("delete chain ");
> +
> +		family = nft_chain_attr_get_u32(nlc, NFT_CHAIN_ATTR_FAMILY);
> +		printf("%s %s %s\n", family2str(family),
> +		       nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_TABLE),
> +		       nft_chain_attr_get_str(nlc, NFT_CHAIN_ATTR_NAME));
> +	} else {
> +		nft_chain_snprintf(buf, sizeof(buf), nlc, evh->format, 0);
> +		printf("%s\t# %s chain\n", buf,
> +		       type == NFT_MSG_NEWCHAIN ? "add" : "del");
> +	}
> +
> +out:
> +	nft_chain_free(nlc);
> +	return MNL_CB_OK;
> +}
> +
> +static int netlink_events_set_cb(const struct nlmsghdr *nlh, int type,
> +				 struct netlink_ev_handler *evh)
> +{
> +	struct nft_set *nls;
> +	struct set *set;
> +	uint32_t family, flags;
> +	char buf[4096];
> +
> +	nls = nft_set_alloc();
> +	if (nls == NULL)
> +		memory_allocation_error();
> +
> +	if (nft_set_nlmsg_parse(nlh, nls) < 0) {
> +		netlink_io_error(evh->ctx, evh->loc,
> +				 "Could not parse set: %s",
> +				 strerror(errno));
> +		goto out;
> +	}
> +
> +	flags = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FLAGS);
> +	if (flags & SET_F_ANONYMOUS && type == NFT_MSG_DELSET)
> +		goto out;
> +
> +	if (evh->format == NFT_OUTPUT_DEFAULT) {
> +		set = netlink_delinearize_set(evh->ctx, nls);
> +		family = nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY);
> +
> +		if (type == NFT_MSG_NEWSET) {
> +			printf("add ");
> +			set_print_plain(set);
> +		} else {
> +			printf("delete set %s %s %s",
> +			       family2str(family),
> +			       nft_set_attr_get_str(nls, NFT_SET_ATTR_TABLE),
> +			       nft_set_attr_get_str(nls, NFT_SET_ATTR_NAME));
> +		}
> +
> +		printf("\n");
> +
> +	} else {
> +		nft_set_snprintf(buf, sizeof(buf), nls, evh->format, 0);
> +		printf("%s\t# %s set\n", buf,
> +		       type == NFT_MSG_NEWSET ? "add" : "del");
> +	}
> +
> +out:
> +	nft_set_free(nls);
> +	return MNL_CB_OK;
> +}
> +
> +static int netlink_events_rule_cb(const struct nlmsghdr *nlh, int type,
> +				  struct netlink_ev_handler *evh)
> +{
> +	struct nft_rule *nlr;
> +	uint32_t family, handle;
> +	char buf[4096];
> +
> +	nlr = nft_rule_alloc();
> +	if (nlr == NULL)
> +		memory_allocation_error();
> +
> +	if (nft_rule_nlmsg_parse(nlh, nlr) < 0) {
> +		netlink_io_error(evh->ctx, evh->loc,
> +				 "Could not parse rule: %s",
> +				 strerror(errno));
> +		goto out;
> +	}
> +
> +	if (evh->format == NFT_OUTPUT_DEFAULT) {
> +		family = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_FAMILY);
> +		handle = nft_rule_attr_get_u32(nlr, NFT_RULE_ATTR_HANDLE);
> +
> +		if (type == NFT_MSG_NEWRULE)
> +			printf("add rule ");
> +		else
> +			printf("delete rule ");
> +
> +		printf("%s %s %s handle %u\n", family2str(family),
> +		       nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_TABLE),
> +		       nft_rule_attr_get_str(nlr, NFT_RULE_ATTR_CHAIN),
> +		       handle);
> +	} else {
> +		nft_rule_snprintf(buf, sizeof(buf), nlr, evh->format, 0);
> +		printf("%s\t# %s rule\n", buf,
> +		       type == NFT_MSG_NEWRULE ? "add" : "del");
> +	}
> +
> +out:
> +	nft_rule_free(nlr);
> +	return MNL_CB_OK;
> +}
> +
> +static int netlink_events_cb(const struct nlmsghdr *nlh, void *data)
> +{
> +	int ret = MNL_CB_OK;
> +	int type = nlh->nlmsg_type & 0xFF;
> +	struct netlink_ev_handler *evh = (struct netlink_ev_handler *)data;
> +
> +	if (!(evh->selector & (1 << type)))
> +		return ret;
> +
> +	switch (type) {
> +	case NFT_MSG_NEWTABLE:
> +	case NFT_MSG_DELTABLE:
> +		ret = netlink_events_table_cb(nlh, type, evh);
> +		break;
> +	case NFT_MSG_NEWCHAIN:
> +	case NFT_MSG_DELCHAIN:
> +		ret = netlink_events_chain_cb(nlh, type, evh);
> +		break;
> +	case NFT_MSG_NEWSET:
> +	case NFT_MSG_DELSET:
> +		ret = netlink_events_set_cb(nlh, type, evh);
> +		break;
> +	case NFT_MSG_NEWRULE:
> +	case NFT_MSG_DELRULE:
> +		ret = netlink_events_rule_cb(nlh, type, evh);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +int netlink_events(struct netlink_ev_handler *evhandler)
> +{
> +	if (mnl_socket_bind(nf_sock, (1 << (NFNLGRP_NFTABLES-1)),
> +			    MNL_SOCKET_AUTOPID) < 0)
> +		return netlink_io_error(evhandler->ctx, evhandler->loc,
> +					"Could not bind to socket: %s",
> +					strerror(errno));
> +
> +	fcntl(mnl_socket_get_fd(nf_sock), F_SETFL, 0);
> +	return mnl_nft_event_listener(nf_sock, netlink_events_cb, evhandler);
> +}
> diff --git a/src/parser.y b/src/parser.y
> index 07613e2..79a42ac 100644
> --- a/src/parser.y
> +++ b/src/parser.y
> @@ -169,6 +169,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  %token ELEMENT			"element"
>  %token MAP			"map"
>  %token HANDLE			"handle"
> +%token ALL			"all"
>  
>  %token INET			"inet"
>  
> @@ -181,6 +182,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  %token RENAME			"rename"
>  %token DESCRIBE			"describe"
>  %token EXPORT			"export"
> +%token EVENT			"event"
>  
>  %token ACCEPT			"accept"
>  %token DROP			"drop"
> @@ -360,8 +362,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  %type <cmd>			line
>  %destructor { cmd_free($$); }	line
>  
> -%type <cmd>			base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd
> -%destructor { cmd_free($$); }	base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd
> +%type <cmd>			base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd event_cmd
> +%destructor { cmd_free($$); }	base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd event_cmd
>  
>  %type <handle>			table_spec tables_spec chain_spec chain_identifier ruleid_spec
>  %destructor { handle_free(&$$); } table_spec tables_spec chain_spec chain_identifier ruleid_spec
> @@ -376,6 +378,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  %type <rule>			rule
>  %destructor { rule_free($$); }	rule
>  
> +%type <val>			event_selector
> +
>  %type <val>			set_flag_list	set_flag
>  
>  %type <set>			set_block_alloc set_block
> @@ -484,7 +488,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
>  %destructor { expr_free($$); }	ct_expr
>  %type <val>			ct_key
>  
> -%type <val>			export_format
> +%type <val>			export_format	event_format
>  
>  %%
>  
> @@ -584,6 +588,7 @@ base_cmd		:	/* empty */	add_cmd		{ $$ = $1; }
>  			|	FLUSH		flush_cmd	{ $$ = $2; }
>  			|	RENAME		rename_cmd	{ $$ = $2; }
>  			|	EXPORT		export_cmd	{ $$ = $2; }
> +			|	EVENT		event_cmd	{ $$ = $2; }
>  			|	DESCRIBE	primary_expr
>  			{
>  				expr_describe($2);
> @@ -751,6 +756,55 @@ export_cmd		:	export_format
>  			}
>  			;
>  
> +event_cmd		:	event_selector	event_format
> +			{
> +				struct handle h = { .family = NFPROTO_UNSPEC };
> +				$$ = cmd_alloc(CMD_EVENT, CMD_OBJ_RULESET, &h, &@$, NULL);
> +				$$->event_selector = $1;
> +				$$->format = $2;
> +			}
> +			;
> +
> +event_selector		:	ALL
> +			{
> +				$$ |= (1 << NFT_MSG_NEWRULE);
> +				$$ |= (1 << NFT_MSG_DELRULE);
> +				$$ |= (1 << NFT_MSG_NEWSET);
> +				$$ |= (1 << NFT_MSG_DELSET);
> +				$$ |= (1 << NFT_MSG_NEWCHAIN);
> +				$$ |= (1 << NFT_MSG_DELCHAIN);
> +				$$ |= (1 << NFT_MSG_NEWTABLE);
> +				$$ |= (1 << NFT_MSG_DELTABLE);
> +			}
> +			|	TABLE
> +			{
> +				$$ |= (1 << NFT_MSG_NEWTABLE);
> +				$$ |= (1 << NFT_MSG_DELTABLE);
> +			}
> +			|	CHAIN
> +			{
> +				$$ |= (1 << NFT_MSG_NEWCHAIN);
> +				$$ |= (1 << NFT_MSG_DELCHAIN);
> +			}
> +			|	SET
> +			{
> +				$$ |= (1 << NFT_MSG_NEWSET);
> +				$$ |= (1 << NFT_MSG_DELSET);
> +			}
> +			|	RULE
> +			{
> +				$$ |= (1 << NFT_MSG_NEWRULE);
> +				$$ |= (1 << NFT_MSG_DELRULE);
> +			}
> +			;
> +
> +event_format		:	/* empty */
> +			{
> +				$$ = NFT_OUTPUT_DEFAULT;
> +			}
> +			|	export_format
> +			;
> +
>  table_block_alloc	:	/* empty */
>  			{
>  				$$ = table_alloc();
> diff --git a/src/rule.c b/src/rule.c
> index 4301faa..30a8529 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -790,6 +790,18 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
>  	return 0;
>  }
>  
> +static int do_command_event(struct netlink_ctx *ctx, struct cmd *cmd)
> +{
> +	struct netlink_ev_handler evhandler;
> +
> +	evhandler.selector = cmd->event_selector;
> +	evhandler.format = cmd->format;
> +	evhandler.ctx = ctx;
> +	evhandler.loc = &cmd->location;
> +
> +	return netlink_events(&evhandler);
> +}
> +
>  int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
>  {
>  	switch (cmd->op) {
> @@ -809,6 +821,8 @@ int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
>  		return do_command_rename(ctx, cmd);
>  	case CMD_EXPORT:
>  		return do_command_export(ctx, cmd);
> +	case CMD_EVENT:
> +		return do_command_event(ctx, cmd);
>  	default:
>  		BUG("invalid command object type %u\n", cmd->obj);
>  	}
> diff --git a/src/scanner.l b/src/scanner.l
> index e4cb398..70b781a 100644
> --- a/src/scanner.l
> +++ b/src/scanner.l
> @@ -238,6 +238,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
>  "element"		{ return ELEMENT; }
>  "map"			{ return MAP; }
>  "handle"		{ return HANDLE; }
> +"all"			{ return ALL; }
>  
>  "accept"		{ return ACCEPT; }
>  "drop"			{ return DROP; }
> @@ -256,6 +257,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
>  "flush"			{ return FLUSH; }
>  "rename"		{ return RENAME; }
>  "export"		{ return EXPORT; }
> +"event"			{ return EVENT; }
>  
>  "position"		{ return POSITION; }
>  
> 

Re: [nft RFC PATCH 6/6] src: add events reporting

[Patrick McHardy]

On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > +				   struct netlink_ev_handler *evh)
> > +{
> > +	struct nft_table *nlt;
> > +	uint32_t family;
> > +	char buf[4096];
> > +
> > +	nlt = nft_table_alloc();
> > +	if (nlt == NULL)
> > +		memory_allocation_error();
> > +
> > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > +		netlink_io_error(evh->ctx, evh->loc,
> > +				"Could not parse table: %s",
> > +				strerror(errno));
> 
> I think you should exit on parsing errors.

I'm not so sure for event reporting. We should abort reporting the current
event, sure. But I don't see why we shouldn't continue listering for further
events.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Pablo Neira Ayuso]

On Tue, Feb 18, 2014 at 02:03:55AM +0000, Patrick McHardy wrote:
> On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > > +				   struct netlink_ev_handler *evh)
> > > +{
> > > +	struct nft_table *nlt;
> > > +	uint32_t family;
> > > +	char buf[4096];
> > > +
> > > +	nlt = nft_table_alloc();
> > > +	if (nlt == NULL)
> > > +		memory_allocation_error();
> > > +
> > > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > > +		netlink_io_error(evh->ctx, evh->loc,
> > > +				"Could not parse table: %s",
> > > +				strerror(errno));
> > 
> > I think you should exit on parsing errors.
> 
> I'm not so sure for event reporting. We should abort reporting the current
> event, sure. But I don't see why we shouldn't continue listering for further
> events.

I think if we fail to parse anything it means that there's some bug
that need to be fixed, eg. someone changed the length of an attribute
so validation fails. I think stopping there should help us to get
people reporting problems.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Patrick McHardy]

On Tue, Feb 18, 2014 at 10:28:11AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 18, 2014 at 02:03:55AM +0000, Patrick McHardy wrote:
> > On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> > > On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > > > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > > > +				   struct netlink_ev_handler *evh)
> > > > +{
> > > > +	struct nft_table *nlt;
> > > > +	uint32_t family;
> > > > +	char buf[4096];
> > > > +
> > > > +	nlt = nft_table_alloc();
> > > > +	if (nlt == NULL)
> > > > +		memory_allocation_error();
> > > > +
> > > > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > > > +		netlink_io_error(evh->ctx, evh->loc,
> > > > +				"Could not parse table: %s",
> > > > +				strerror(errno));
> > > 
> > > I think you should exit on parsing errors.
> > 
> > I'm not so sure for event reporting. We should abort reporting the current
> > event, sure. But I don't see why we shouldn't continue listering for further
> > events.
> 
> I think if we fail to parse anything it means that there's some bug
> that need to be fixed, eg. someone changed the length of an attribute
> so validation fails. I think stopping there should help us to get
> people reporting problems.

Well, it depends on what exactly we couldn't parse or handle. Basic things
like table names etc. can of course trigger a fatal error. But unknown
expression types shouldn't cause an error. Seemingly invalid ways
epressions are linked etc should probably also not do that since they
may simply be created by something that is not nft and we don't understand
them. So it really depends on where the error is happening.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Pablo Neira Ayuso]

On Tue, Feb 18, 2014 at 09:33:11AM +0000, Patrick McHardy wrote:
> On Tue, Feb 18, 2014 at 10:28:11AM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Feb 18, 2014 at 02:03:55AM +0000, Patrick McHardy wrote:
> > > On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> > > > On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > > > > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > > > > +				   struct netlink_ev_handler *evh)
> > > > > +{
> > > > > +	struct nft_table *nlt;
> > > > > +	uint32_t family;
> > > > > +	char buf[4096];
> > > > > +
> > > > > +	nlt = nft_table_alloc();
> > > > > +	if (nlt == NULL)
> > > > > +		memory_allocation_error();
> > > > > +
> > > > > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > > > > +		netlink_io_error(evh->ctx, evh->loc,
> > > > > +				"Could not parse table: %s",
> > > > > +				strerror(errno));
> > > > 
> > > > I think you should exit on parsing errors.
> > > 
> > > I'm not so sure for event reporting. We should abort reporting the current
> > > event, sure. But I don't see why we shouldn't continue listering for further
> > > events.
> > 
> > I think if we fail to parse anything it means that there's some bug
> > that need to be fixed, eg. someone changed the length of an attribute
> > so validation fails. I think stopping there should help us to get
> > people reporting problems.
> 
> Well, it depends on what exactly we couldn't parse or handle. Basic things
> like table names etc. can of course trigger a fatal error. But unknown
> expression types shouldn't cause an error.

As i said, unknown netlink attributes are skipped.

That function is not building *any* struct expr *, it's just
generating the libnftnl nft_table object that is used to obtain the
internal table object in a later stage. If it fails there's something
that need to be fixed in that code.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Patrick McHardy]

On Tue, Feb 18, 2014 at 10:43:20AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 18, 2014 at 09:33:11AM +0000, Patrick McHardy wrote:
> > On Tue, Feb 18, 2014 at 10:28:11AM +0100, Pablo Neira Ayuso wrote:
> > > On Tue, Feb 18, 2014 at 02:03:55AM +0000, Patrick McHardy wrote:
> > > > On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> > > > > On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > > > > > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > > > > > +				   struct netlink_ev_handler *evh)
> > > > > > +{
> > > > > > +	struct nft_table *nlt;
> > > > > > +	uint32_t family;
> > > > > > +	char buf[4096];
> > > > > > +
> > > > > > +	nlt = nft_table_alloc();
> > > > > > +	if (nlt == NULL)
> > > > > > +		memory_allocation_error();
> > > > > > +
> > > > > > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > > > > > +		netlink_io_error(evh->ctx, evh->loc,
> > > > > > +				"Could not parse table: %s",
> > > > > > +				strerror(errno));
> > > > > 
> > > > > I think you should exit on parsing errors.
> > > > 
> > > > I'm not so sure for event reporting. We should abort reporting the current
> > > > event, sure. But I don't see why we shouldn't continue listering for further
> > > > events.
> > > 
> > > I think if we fail to parse anything it means that there's some bug
> > > that need to be fixed, eg. someone changed the length of an attribute
> > > so validation fails. I think stopping there should help us to get
> > > people reporting problems.
> > 
> > Well, it depends on what exactly we couldn't parse or handle. Basic things
> > like table names etc. can of course trigger a fatal error. But unknown
> > expression types shouldn't cause an error.
> 
> As i said, unknown netlink attributes are skipped.
> 
> That function is not building *any* struct expr *, it's just
> generating the libnftnl nft_table object that is used to obtain the
> internal table object in a later stage. If it fails there's something
> that need to be fixed in that code.

Sure, just wanted to be clear about which types of errors may cause
a fatal error.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Pablo Neira Ayuso]

On Tue, Feb 18, 2014 at 09:52:10AM +0000, Patrick McHardy wrote:
> On Tue, Feb 18, 2014 at 10:43:20AM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Feb 18, 2014 at 09:33:11AM +0000, Patrick McHardy wrote:
> > > On Tue, Feb 18, 2014 at 10:28:11AM +0100, Pablo Neira Ayuso wrote:
> > > > On Tue, Feb 18, 2014 at 02:03:55AM +0000, Patrick McHardy wrote:
> > > > > On Tue, Feb 18, 2014 at 02:10:07AM +0100, Pablo Neira Ayuso wrote:
> > > > > > On Tue, Feb 18, 2014 at 12:18:38AM +0100, Arturo Borrero Gonzalez wrote:
> > > > > > > +static int netlink_events_table_cb(const struct nlmsghdr *nlh, int type,
> > > > > > > +				   struct netlink_ev_handler *evh)
> > > > > > > +{
> > > > > > > +	struct nft_table *nlt;
> > > > > > > +	uint32_t family;
> > > > > > > +	char buf[4096];
> > > > > > > +
> > > > > > > +	nlt = nft_table_alloc();
> > > > > > > +	if (nlt == NULL)
> > > > > > > +		memory_allocation_error();
> > > > > > > +
> > > > > > > +	if (nft_table_nlmsg_parse(nlh, nlt) < 0) {
> > > > > > > +		netlink_io_error(evh->ctx, evh->loc,
> > > > > > > +				"Could not parse table: %s",
> > > > > > > +				strerror(errno));
> > > > > > 
> > > > > > I think you should exit on parsing errors.
> > > > > 
> > > > > I'm not so sure for event reporting. We should abort reporting the current
> > > > > event, sure. But I don't see why we shouldn't continue listering for further
> > > > > events.
> > > > 
> > > > I think if we fail to parse anything it means that there's some bug
> > > > that need to be fixed, eg. someone changed the length of an attribute
> > > > so validation fails. I think stopping there should help us to get
> > > > people reporting problems.
> > > 
> > > Well, it depends on what exactly we couldn't parse or handle. Basic things
> > > like table names etc. can of course trigger a fatal error. But unknown
> > > expression types shouldn't cause an error.
> > 
> > As i said, unknown netlink attributes are skipped.
> > 
> > That function is not building *any* struct expr *, it's just
> > generating the libnftnl nft_table object that is used to obtain the
> > internal table object in a later stage. If it fails there's something
> > that need to be fixed in that code.
> 
> Sure, just wanted to be clear about which types of errors may cause
> a fatal error.

Talking about errors when building the higher level expression tree
from the netlink message, I think nft should output some low-level
expression if it fails to interpret it in a human readable way / nft
syntax way.

We already discussed that third party applications may decide to skip
nft as use the netlink interface to build sophisticated filters, in
that case, I think those tools should not break the output of nft if
it fails to understand what it gets from the kernel.

Re: [nft RFC PATCH 6/6] src: add events reporting

[Patrick McHardy]

On Tue, Feb 18, 2014 at 10:58:29AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 18, 2014 at 09:52:10AM +0000, Patrick McHardy wrote:
> > 
> > Sure, just wanted to be clear about which types of errors may cause
> > a fatal error.
> 
> Talking about errors when building the higher level expression tree
> from the netlink message, I think nft should output some low-level
> expression if it fails to interpret it in a human readable way / nft
> syntax way.

Not sure how exactly to do that. It will never really fail unless the
rule has real errors like using data that hasn't been loaded before.
It will always result in *some* expression, so how would be determine
that?

> We already discussed that third party applications may decide to skip
> nft as use the netlink interface to build sophisticated filters, in
> that case, I think those tools should not break the output of nft if
> it fails to understand what it gets from the kernel.

I'm actually not sure nft really could fail if the expression returned
from the kernel makes any sense at all. Worst case should be that it
translates it to literate expressions used by the kernel (IOW,
payload @raw-expression & val ^ val2 >= ... instead of some simplified
form).

Re: [nft RFC PATCH 6/6] src: add events reporting

[Arturo Borrero Gonzalez]

On 18 February 2014 11:12, Patrick McHardy <kaber@trash.net> wrote:
>
> I'm actually not sure nft really could fail if the expression returned
> from the kernel makes any sense at all.

I did a fast&small test.

What to do in the event reporting if this situation is reached?
Non-sense rule added to the kernel:

ip filter input 0 0
  [ cmp eq reg 1 0x00000006 ]
  [ payload load 1b @ network header + 9 => reg 1 ]
  [ payload load 2b @ transport header + 1 => reg 2 ]
  [ counter pkts 0 bytes 0 ]
  [ cmp eq reg 1 0x00001600 ]

% nft list table filter
table ip filter {
 [...]
}
netlink: Error: Relational expression has no left hand side


netlink: Error: Relational expression size mismatch

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft RFC PATCH 6/6] src: add events reporting

[Patrick McHardy]

On Tue, Feb 18, 2014 at 03:21:47PM +0100, Arturo Borrero Gonzalez wrote:
> On 18 February 2014 11:12, Patrick McHardy <kaber@trash.net> wrote:
> >
> > I'm actually not sure nft really could fail if the expression returned
> > from the kernel makes any sense at all.
> 
> I did a fast&small test.
> 
> What to do in the event reporting if this situation is reached?
> Non-sense rule added to the kernel:
> 
> ip filter input 0 0
>   [ cmp eq reg 1 0x00000006 ]
>   [ payload load 1b @ network header + 9 => reg 1 ]
>   [ payload load 2b @ transport header + 1 => reg 2 ]
>   [ counter pkts 0 bytes 0 ]
>   [ cmp eq reg 1 0x00001600 ]
> 
> % nft list table filter
> table ip filter {
>  [...]
> }
> netlink: Error: Relational expression has no left hand side
> 
> 
> netlink: Error: Relational expression size mismatch

Well, that's obviously a broken rule. There is no valid interpretation,
hence we don't need to display one. Reporting the error seems perfectly
fine to me, however event reporting shouldn't abort because of this.

Basically I think - netlink header and attribute parsing errors are
fine to exit. However just real parsing errors, not things like f.i.
unknown address families. Any error during interpretation of these
should only cause the error to be displayed, but nothing more.

Re: [nft RFC PATCH 0/6] events

[Pablo Neira Ayuso]

Hi Arturo,

On Tue, Feb 18, 2014 at 12:18:06AM +0100, Arturo Borrero Gonzalez wrote:
> This series implements basic event reporting in the nftables CLI tool.
> 
> The first patches are some neccesary code factorization changes.
> The last patch is the event reporting itself.
> 
> Its quite simple, the syntax is:
>  % nft event <all|table|chain|rule|set> [xml|json]

It would be good to allow matching based on the event type, ie. new /
delete.

> To quit, fire CTRL+C (^C).
> 
> Currently, 3 possible output formats:
>  * A basic XML, provided by libnftnl.
>  * A basic JSON, provided by libnftnl.
>  * nft default-like syntax.
> 
> About this last format:
> 
> Rules are hard to print exactly as the user typed because sets and other
> nuances. Possible solutions I've found:
>  - assume that an anonymous set event will happen always before a new rule event.
>  Cache the anon-set for the following rule event.

Yes, you will to listen to set events and initially create a cache
with the events that you already have.

>  Maybe there are many anon-sets per rule.
>  - when a rule event happens, query for sets inside the event cb.

This seems expensive to me as you will need send request to the kernel
per rule that points to a set.

>  - for this to run smoothly, we need to keep tables info in sync with the
>  kernel, so in each relevant event, the netlink_ctx is needed to be updated and
>  this allows to reuse netlink_delinearize_rule().
>
> But I think this first approach is valid.
> 
> So, the format with this series is as follow:
> 
>  % nft event all
> delete table ip6 filter
> add table ip6 filter
> add chain ip6 filter input { type filter hook input priority 0;}
> add chain ip6 filter forward { type filter hook forward priority 0;}
> add chain ip6 filter output { type filter hook output priority 0;}
> add rule ip6 filter input handle 4

I guess this is why you point to netlink_delinearize_rule above.

> delete rule ip6 filter input handle 4
> add set ip6 filter set1 {type ipv6_address}
> delete chain ip6 filter input
> delete set ip6 filter set1
> 
> NOTE: no way to add comments in JSON, so I decided to add that bash-like
> comment by now and be consistent in both formats.

I think you can wrap that information with the event type? ie.

<event type="new">...</event>

Please, check if you can add a similar wrapper to json.

>  % nft event all xml
> <table><name>test</name><family>arp</family><flags>0</flags><use>0</use></table>        # add table
> <chain><name>c</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>test</table><family>arp</family></chain>      # add chain
> <chain><name>c</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>test</table><family>arp</family></chain>      # del chain
> <table><name>test</name><family>arp</family><flags>0</flags><use>0</use></table>        # del table
> 
>  % nft event all json
> {"table":{"name":"test","family":"arp","flags":0,"use":0}}      # add table
> {"set":{"name":"set123","table":"test","flags":0,"family":"arp","key_type":7,"key_len":4}}      # add set
> 
> Please comment.
> 
> ---
> 
> Arturo Borrero Gonzalez (6):
>       rule: make family2str() public
>       rule: allow to print sets in plain format
>       netlink: add netlink_delinearize_set() func
>       rule: generalize chain_print()
>       netlink: add netlink_delinearize_rule() func
>       src: add events reporting
> 
> 
>  include/mnl.h             |    3 +
>  include/netlink.h         |   15 +++
>  include/rule.h            |    7 +
>  src/evaluate.c            |    1 
>  src/mnl.c                 |   45 +++++---
>  src/netlink.c             |  268 ++++++++++++++++++++++++++++++++++++++-------
>  src/netlink_delinearize.c |   57 ++++++++++
>  src/parser.y              |   60 ++++++++++
>  src/rule.c                |   93 +++++++++++++---
>  src/scanner.l             |    2 
>  10 files changed, 477 insertions(+), 74 deletions(-)
> 
> -- 
> Arturo Borrero Gonzalez

Re: [nft RFC PATCH 0/6] events

[Patrick McHardy]

On Tue, Feb 18, 2014 at 12:18:06AM +0100, Arturo Borrero Gonzalez wrote:
> This series implements basic event reporting in the nftables CLI tool.

Nice.

> The first patches are some neccesary code factorization changes.
> The last patch is the event reporting itself.
> 
> Its quite simple, the syntax is:
>  % nft event <all|table|chain|rule|set> [xml|json]

So far we're pretty much (except for shortcuts like default "add rule")
following the scheme "nft action object". "event" doesn't fit in this
scheme, so I'd propose to change this to "monitor". Also I guess object
specification could be optional so "all" wouldn't be needed, but that's
not too important.

> To quit, fire CTRL+C (^C).
> 
> Currently, 3 possible output formats:
>  * A basic XML, provided by libnftnl.
>  * A basic JSON, provided by libnftnl.
>  * nft default-like syntax.
> 
> About this last format:
> 
> Rules are hard to print exactly as the user typed because sets and other
> nuances. Possible solutions I've found:
>  - assume that an anonymous set event will happen always before a new rule event.
>  Cache the anon-set for the following rule event.
>  Maybe there are many anon-sets per rule.

That should work fine. But why only cache one set? Basically you can assume
all anonymous sets to be constant once they have been bound.

>  - when a rule event happens, query for sets inside the event cb.
>  - for this to run smoothly, we need to keep tables info in sync with the
>  kernel, so in each relevant event, the netlink_ctx is needed to be updated and
>  this allows to reuse netlink_delinearize_rule().
> 
> But I think this first approach is valid.
> 
> So, the format with this series is as follow:
> 
>  % nft event all
> delete table ip6 filter
> add table ip6 filter
> add chain ip6 filter input { type filter hook input priority 0;}
> add chain ip6 filter forward { type filter hook forward priority 0;}
> add chain ip6 filter output { type filter hook output priority 0;}
> add rule ip6 filter input handle 4

No expressions in the output? Why the set caching then?

> delete rule ip6 filter input handle 4
> add set ip6 filter set1 {type ipv6_address}

Inconsistent wrt. chain blocks. We should have spaces between {} and
the content and too make things parsable it should contain a statement
separator (;) in the single line output.

> delete chain ip6 filter input
> delete set ip6 filter set1

Re: [nft RFC PATCH 0/6] events

[Arturo Borrero Gonzalez]

On 18 February 2014 02:43, Patrick McHardy <kaber@trash.net> wrote:
>>
>> Its quite simple, the syntax is:
>>  % nft event <all|table|chain|rule|set> [xml|json]
>
> So far we're pretty much (except for shortcuts like default "add rule")
> following the scheme "nft action object". "event" doesn't fit in this
> scheme, so I'd propose to change this to "monitor". Also I guess object
> specification could be optional so "all" wouldn't be needed, but that's
> not too important.
>

Ok, so I propose the syntax:

 % nft monitor [table|chain|rule|set] [new|delete] [xml|json]

>> add rule ip6 filter input handle 4
>
> No expressions in the output? Why the set caching then?
>

Well, in this first approach I was unable to achieve that :-(
I needed some feedback (this RFC).

The intention is of course print out all the expressions.

regards.

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft RFC PATCH 0/6] events

[Patrick McHardy]

On Tue, Feb 18, 2014 at 10:20:46AM +0100, Arturo Borrero Gonzalez wrote:
> On 18 February 2014 02:43, Patrick McHardy <kaber@trash.net> wrote:
> >>
> >> Its quite simple, the syntax is:
> >>  % nft event <all|table|chain|rule|set> [xml|json]
> >
> > So far we're pretty much (except for shortcuts like default "add rule")
> > following the scheme "nft action object". "event" doesn't fit in this
> > scheme, so I'd propose to change this to "monitor". Also I guess object
> > specification could be optional so "all" wouldn't be needed, but that's
> > not too important.
> >
> 
> Ok, so I propose the syntax:
> 
>  % nft monitor [table|chain|rule|set] [new|delete] [xml|json]

Looks good to me.

> >> add rule ip6 filter input handle 4
> >
> > No expressions in the output? Why the set caching then?
> >
> 
> Well, in this first approach I was unable to achieve that :-(
> I needed some feedback (this RFC).
> 
> The intention is of course print out all the expressions.

Great. Let me know if you need some help with that.