libnftables preparation work

PATCH Superseded Eric Leblond 4 participants 21 messages Sep 3, 2017 – Sep 21, 2017 netfilter

lore

Share & tools

lore

Eric Leblond

author

2017-09-03 22:04 UTC #0 lore raw

cover libnftables preparation work

Hi,

Here is two patches relative to libnftables preparation work.

The first one it changing the way a nft_ctx is created to be able to skip the netlink init function call and also to have some freedom later.

The second one is getting the printf out. This is completely changed from what was proposed before. It is now really simple on user side.

BR,
--
Eric

Eric Leblond

author

2017-09-03 22:04 UTC #1 lore raw

PATCH 1/2 src: add flags fo nft_ctx_new

By adding flags to nft_ctx_new, we will have a minimum capabilities of changing the way the nft_ctx is created.

For now, this patch uses a simple value that allow the user to specify that he will handle netlink by himself.

Signed-off-by Eric Leblond <[email protected]>

---
 include/nftables.h |  4 ++++
 src/main.c         | 20 +++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

include/nftables.h +4 -0

index 5035567..7c4e93f 100644
@@ -49,8 +49,12 @@ struct nft_ctx {
 	struct output_ctx	output;
 	bool			check;
 	struct nft_cache	cache;
+	uint32_t		flags;
 };
 
+#define NFT_CTX_CUSTOM_NETLINK	(1<<0)
+#define NFT_CTX_DEFAULT		0
+
 enum nftables_exit_codes {
 	NFT_EXIT_SUCCESS	= 0,
 	NFT_EXIT_FAILURE	= 1,

src/main.c +11 -9

index fce9bfe..94f8a47 100644
@@ -281,7 +281,12 @@ static void nft_exit(void)
 	mark_table_exit();
 }
 
-static struct nft_ctx *nft_ctx_new(void)
+static void nft_ctx_netlink_init(struct nft_ctx *ctx)
+{
+	ctx->nf_sock = netlink_open_sock();
+}
+
+static struct nft_ctx *nft_ctx_new(uint32_t flags)
 {
 	struct nft_ctx *ctx;

@@ -292,6 +297,10 @@ static struct nft_ctx *nft_ctx_new(void)
 	ctx->num_include_paths	= 1;
 	ctx->parser_max_errors	= 10;
 	init_list_head(&ctx->cache.list);
+	ctx->flags = flags;
+
+	if (! (flags & NFT_CTX_CUSTOM_NETLINK))
+		nft_ctx_netlink_init(ctx);
 
 	return ctx;
 }

@@ -307,11 +316,6 @@ static void nft_ctx_free(const struct nft_ctx *ctx)
 	nft_exit();
 }
 
-static void nft_ctx_netlink_init(struct nft_ctx *ctx)
-{
-	ctx->nf_sock = netlink_open_sock();
-}
-
 static int nft_run_cmd_from_buffer(struct nft_ctx *nft,
 				   char *buf, size_t buflen)
 {

@@ -367,9 +371,7 @@ int main(int argc, char * const *argv)
 	struct parser_state state;
 	int i, val, rc;
 
-	nft = nft_ctx_new();
-
-	nft_ctx_netlink_init(nft);
+	nft = nft_ctx_new(NFT_CTX_DEFAULT);
 
 	while (1) {
 		val = getopt_long(argc, argv, OPTSTRING, options, NULL);

Signature · 1 line

2.14.1

Pablo Neira Ayuso

maintainer

2017-09-03 22:33 UTC #2 lore raw

Re: [nft PATCH 1/2] src: add flags fo nft_ctx_new

Quoted 17 lines · Eric Leblond

By adding flags to nft_ctx_new, we will have a minimum capabilities of changing the way the nft_ctx is created.
For now, this patch uses a simple value that allow the user to specify that he will handle netlink by himself.
Signed-off-by Eric Leblond <[email protected]>
---
 include/nftables.h |  4 ++++
 src/main.c         | 20 +++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)
include/nftables.h +0 -0
index 5035567..7c4e93f 100644

include/nftables.h struct nft_ctx { · line 56

Quoted 8 lines

include/nftables.h +3 -0

@@ -49,8 +49,12 @@ struct nft_ctx {
 	struct output_ctx	output;
 	bool			check;
 	struct nft_cache	cache;
+	uint32_t		flags;
 };
 
+#define NFT_CTX_CUSTOM_NETLINK	(1<<0)
+#define NFT_CTX_DEFAULT		0

Better than flag, let's make this a type, ie. NFT_CTX_NETLINK is just 1.

OK?

Pablo Neira Ayuso

maintainer

2017-09-03 22:46 UTC #3 lore raw

Re: [nft PATCH 1/2] src: add flags fo nft_ctx_new

Quoted 24 lines · Pablo Neira Ayuso

Quoted 23 lines · Eric Leblond
By adding flags to nft_ctx_new, we will have a minimum capabilities of changing the way the nft_ctx is created.
For now, this patch uses a simple value that allow the user to specify that he will handle netlink by himself.
Signed-off-by Eric Leblond <[email protected]>
---
 include/nftables.h |  4 ++++
 src/main.c         | 20 +++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)
include/nftables.h +1 -0
index 5035567..7c4e93f 100644
@@ -49,8 +49,12 @@ struct nft_ctx {
 	struct output_ctx	output;
 	bool			check;
 	struct nft_cache	cache;
+	uint32_t		flags;
 };
 

Quoted 6 lines

include/nftables.h struct nft_ctx { · line 56
Quoted 2 lines
include/nftables.h +2 -0
+#define NFT_CTX_CUSTOM_NETLINK	(1<<0)
+#define NFT_CTX_DEFAULT		0
Better than flag, let's make this a type, ie. NFT_CTX_NETLINK is just 1.
OK?

Actually, we keep this back, I mean, we just have NFT_CTX_DEFAULT, so we don't introduce NFT_CTX_NETLINK until we have the advanced API in place.

Eric Leblond

author

2017-09-04 07:21 UTC #4 lore raw

Re: [nft PATCH 1/2] src: add flags fo nft_ctx_new

Hi,

Quoted 33 lines · Pablo Neira Ayuso

Quoted 32 lines · Pablo Neira Ayuso
Quoted 25 lines · Eric Leblond
By adding flags to nft_ctx_new, we will have a minimum capabilities
of changing the way the nft_ctx is created.
For now, this patch uses a simple value that allow the user to specify
that he will handle netlink by himself.
Signed-off-by Eric Leblond <[email protected]>
---
include/nftables.h | 4 ++++
src/main.c | 20 +++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)
include/nftables.h +1 -0
index 5035567..7c4e93f 100644
@@ -49,8 +49,12 @@ struct nft_ctx {
  	struct output_ctx	output;
  	bool			check;
  	struct nft_cache	cache;
+	uint32_t		flags;
  };
include/nftables.h struct nft_ctx { · line 56
Quoted 2 lines
include/nftables.h +2 -0
+#define NFT_CTX_CUSTOM_NETLINK	(1<<0)
+#define NFT_CTX_DEFAULT		0
Better than flag, let's make this a type, ie. NFT_CTX_NETLINK is just 1.

Quoted 5 lines

Quoted 1 line
OK?
Actually, we keep this back, I mean, we just have NFT_CTX_DEFAULT, so we don't introduce NFT_CTX_NETLINK until we have the advanced API in place.

Yes, make sense, it was a bit artificial. I will cook updated patch today.

BR,

Signature · 1 line

Eric Leblond <[email protected]>

Eric Leblond

author

2017-09-03 22:04 UTC #5 lore raw

PATCH 2/2 src: get rid of printf

In reply to Eric Leblond

This patch introduces the nft_print_to_output_ctx function that has to be used instead of printf to output information that where previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output() function.

This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.

Signed-off-by Eric Leblond <[email protected]>

---
 include/datatype.h   |   5 +-
 include/expression.h |   2 +-
 include/nftables.h   |   5 ++
 src/cli.c            |   1 +
 src/ct.c             |  21 ++---
 src/datatype.c       |  66 ++++++++-------
 src/expression.c     |  79 ++++++++++--------
 src/exthdr.c         |  16 ++--
 src/fib.c            |  23 +++---
 src/hash.c           |  10 +--
 src/main.c           |  45 ++++++++++
 src/meta.c           |  32 +++++---
 src/numgen.c         |   8 +-
 src/payload.c        |   9 +-
 src/rule.c           | 228 +++++++++++++++++++++++++++++----------------------
 src/statement.c      | 145 +++++++++++++++++---------------
 16 files changed, 408 insertions(+), 287 deletions(-)

include/datatype.h +3 -2

index 2e34591..e9f6079 100644
@@ -209,7 +209,8 @@ extern void symbolic_constant_print(const struct symbol_table *tbl,
 				    struct output_ctx *octx);
 extern void symbol_table_print(const struct symbol_table *tbl,
 			       const struct datatype *dtype,
-			       enum byteorder byteorder);
+			       enum byteorder byteorder,
+			       struct output_ctx *octx);
 
 extern struct symbol_table *rt_symbol_table_init(const char *filename);
 extern void rt_symbol_table_free(struct symbol_table *tbl);

@@ -261,7 +262,7 @@ extern const struct datatype *
 set_datatype_alloc(const struct datatype *orig_dtype, unsigned int byteorder);
 extern void set_datatype_destroy(const struct datatype *dtype);
 
-extern void time_print(uint64_t seconds);
+extern void time_print(uint64_t seconds, struct output_ctx *octx);
 extern struct error_record *time_parse(const struct location *loc,
 				       const char *c, uint64_t *res);

include/expression.h +1 -1

index 32d4423..ce6b702 100644
@@ -334,7 +334,7 @@ extern struct expr *expr_get(struct expr *expr);
 extern void expr_free(struct expr *expr);
 extern void expr_print(const struct expr *expr, struct output_ctx *octx);
 extern bool expr_cmp(const struct expr *e1, const struct expr *e2);
-extern void expr_describe(const struct expr *expr);
+extern void expr_describe(const struct expr *expr, struct output_ctx *octx);
 
 extern const struct datatype *expr_basetype(const struct expr *expr);
 extern void expr_set_type(struct expr *expr, const struct datatype *dtype,

include/nftables.h +5 -0

index 7c4e93f..f4d5ce1 100644
@@ -30,6 +30,8 @@ struct output_ctx {
 	unsigned int ip2name;
 	unsigned int handle;
 	unsigned int echo;
+	char *output_buf;
+	size_t output_buf_len;
 };
 
 struct nft_cache {

@@ -149,4 +151,7 @@ void realm_table_meta_exit(void);
 void devgroup_table_exit(void);
 void realm_table_rt_exit(void);
 
+int nft_print_to_output_ctx(struct output_ctx *octx, const char *fmt, ...);
+char *nft_ctx_get_output(struct nft_ctx *ctx);
+
 #endif /* NFTABLES_NFTABLES_H */

src/cli.c +1 -0

index d923ff7..ca4418c 100644
@@ -138,6 +138,7 @@ static void cli_complete(char *line)
 		    cli_nft->debug_mask);
 	scanner_push_buffer(scanner, &indesc_cli, line);
 	nft_run(cli_nft, cli_nf_sock, scanner, state, &msgs);
+	printf("%s", nft_ctx_get_output(cli_nft));
 	erec_print_list(stdout, &msgs, cli_nft->debug_mask);
 	xfree(line);
 	cache_release(&cli_nft->cache);

src/ct.c +11 ~~-10~~

index d64f467..f19608a 100644
@@ -141,11 +141,12 @@ static void ct_label_type_print(const struct expr *expr,
 	for (s = ct_label_tbl->symbols; s->identifier != NULL; s++) {
 		if (bit != s->value)
 			continue;
-		printf("\"%s\"", s->identifier);
+		nft_print_to_output_ctx(octx, "\"%s\"", s->identifier);
 		return;
 	}
 	/* can happen when connlabel.conf is altered after rules were added */
-	printf("%ld\n", (long)mpz_scan1(expr->value, 0));
+	nft_print_to_output_ctx(octx, "%ld\n",
+				(long)mpz_scan1(expr->value, 0));
 }
 
 static struct error_record *ct_label_type_parse(const struct expr *sym,

@@ -269,27 +270,27 @@ static const struct ct_template ct_templates[] = {
 					      BYTEORDER_HOST_ENDIAN, 32),
 };
 
-static void ct_print(enum nft_ct_keys key, int8_t dir)
+static void ct_print(enum nft_ct_keys key, int8_t dir, struct output_ctx *octx)
 {
 	const struct symbolic_constant *s;
 
-	printf("ct ");
+	nft_print_to_output_ctx(octx, "ct ");
 	if (dir < 0)
 		goto done;
 
 	for (s = ct_dir_tbl.symbols; s->identifier != NULL; s++) {
 		if (dir == (int)s->value) {
-			printf("%s ", s->identifier);
+			nft_print_to_output_ctx(octx, "%s ", s->identifier);
 			break;
 		}
 	}
  done:
-	printf("%s", ct_templates[key].token);
+	nft_print_to_output_ctx(octx, "%s", ct_templates[key].token);
 }
 
 static void ct_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	ct_print(expr->ct.key, expr->ct.direction);
+	ct_print(expr->ct.key, expr->ct.direction, octx);
 }
 
 static bool ct_expr_cmp(const struct expr *e1, const struct expr *e2)

@@ -445,8 +446,8 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 
 static void ct_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
-	ct_print(stmt->ct.key, stmt->ct.direction);
-	printf(" set ");
+	ct_print(stmt->ct.key, stmt->ct.direction, octx);
+	nft_print_to_output_ctx(octx, " set ");
 	expr_print(stmt->ct.expr, octx);
 }

@@ -472,7 +473,7 @@ struct stmt *ct_stmt_alloc(const struct location *loc, enum nft_ct_keys key,
 
 static void notrack_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
-	printf("notrack");
+	nft_print_to_output_ctx(octx, "notrack");
 }
 
 static const struct stmt_ops notrack_stmt_ops = {

src/datatype.c +37 ~~-29~~

180 lines not on this page · raw message

src/expression.c +43 ~~-36~~

192 lines not on this page · raw message

src/exthdr.c +10 -6

index 4add3da..c8a33f8 100644
@@ -34,20 +34,24 @@ static void exthdr_expr_print(const struct expr *expr, struct output_ctx *octx)
 		char buf[9] = {0};
 
 		if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT) {
-			printf("tcp option %s", expr->exthdr.desc->name);
+			nft_print_to_output_ctx(octx, "tcp option %s",
+						expr->exthdr.desc->name);
 			return;
 		}
 
 		if (offset)
 			snprintf(buf, sizeof buf, "%d", offset);
-		printf("tcp option %s%s %s", expr->exthdr.desc->name, buf,
-					     expr->exthdr.tmpl->token);
+		nft_print_to_output_ctx(octx, "tcp option %s%s %s",
+					expr->exthdr.desc->name, buf,
+					expr->exthdr.tmpl->token);
 	} else {
 		if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT)
-			printf("exthdr %s", expr->exthdr.desc->name);
+			nft_print_to_output_ctx(octx, "exthdr %s",
+						expr->exthdr.desc->name);
 		else {
-			printf("%s %s", expr->exthdr.desc ? expr->exthdr.desc->name : "unknown-exthdr",
-					expr->exthdr.tmpl->token);
+			nft_print_to_output_ctx(octx, "%s %s",
+						expr->exthdr.desc ? expr->exthdr.desc->name : "unknown-exthdr",
+						expr->exthdr.tmpl->token);
 		}
 	}
 }

src/fib.c +12 ~~-11~~

index b3488af..df39e35 100644
@@ -60,32 +60,33 @@ static const char *fib_result_str(enum nft_fib_result result)
 	return "unknown";
 }
 
-static void __fib_expr_print_f(unsigned int *flags, unsigned int f, const char *s)
+static void __fib_expr_print_f(unsigned int *flags, unsigned int f,
+			       const char *s, struct output_ctx *octx)
 {
 	if ((*flags & f) == 0)
 		return;
 
-	printf("%s", s);
+	nft_print_to_output_ctx(octx, "%s", s);
 	*flags &= ~f;
 	if (*flags)
-		printf(" . ");
+		nft_print_to_output_ctx(octx, " . ");
 }
 
 static void fib_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	unsigned int flags = expr->fib.flags & ~NFTA_FIB_F_PRESENT;
 
-	printf("fib ");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_SADDR, "saddr");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_DADDR, "daddr");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_MARK, "mark");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_IIF, "iif");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_OIF, "oif");
+	nft_print_to_output_ctx(octx, "fib ");
+	__fib_expr_print_f(&flags, NFTA_FIB_F_SADDR, "saddr", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_DADDR, "daddr", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_MARK, "mark", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_IIF, "iif", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_OIF, "oif", octx);
 
 	if (flags)
-		printf("0x%x", flags);
+		nft_print_to_output_ctx(octx, "0x%x", flags);
 
-	printf(" %s", fib_result_str(expr->fib.result));
+	nft_print_to_output_ctx(octx, " %s", fib_result_str(expr->fib.result));
 }
 
 static bool fib_expr_cmp(const struct expr *e1, const struct expr *e2)

src/hash.c +5 -5

index 1a4bfb3..7820a81 100644
@@ -19,19 +19,19 @@ static void hash_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	switch (expr->hash.type) {
 	case NFT_HASH_SYM:
-		printf("symhash");
+		nft_print_to_output_ctx(octx, "symhash");
 	break;
 	case NFT_HASH_JENKINS:
 	default:
-		printf("jhash ");
+		nft_print_to_output_ctx(octx, "jhash ");
 		expr_print(expr->hash.expr, octx);
 	}
 
-	printf(" mod %u", expr->hash.mod);
+	nft_print_to_output_ctx(octx, " mod %u", expr->hash.mod);
 	if (expr->hash.seed_set)
-		printf(" seed 0x%x", expr->hash.seed);
+		nft_print_to_output_ctx(octx, " seed 0x%x", expr->hash.seed);
 	if (expr->hash.offset)
-		printf(" offset %u", expr->hash.offset);
+		nft_print_to_output_ctx(octx, " offset %u", expr->hash.offset);
 }
 
 static bool hash_expr_cmp(const struct expr *e1, const struct expr *e2)

src/main.c +45 -0

91 lines not on this page · raw message

src/meta.c +19 ~~-13~~

index 9c80893..8d271a8 100644
@@ -54,13 +54,15 @@ static void tchandle_type_print(const struct expr *expr,
 
 	switch(handle) {
 	case TC_H_ROOT:
-		printf("root");
+		nft_print_to_output_ctx(octx, "root");
 		break;
 	case TC_H_UNSPEC:
-		printf("none");
+		nft_print_to_output_ctx(octx, "none");
 		break;
 	default:
-		printf("%0x:%0x", TC_H_MAJ(handle) >> 16, TC_H_MIN(handle));
+		nft_print_to_output_ctx(octx, "%0x:%0x",
+					TC_H_MAJ(handle) >> 16,
+					TC_H_MIN(handle));
 		break;
 	}
 }

@@ -134,9 +136,9 @@ static void ifindex_type_print(const struct expr *expr, struct output_ctx *octx)
 
 	ifindex = mpz_get_uint32(expr->value);
 	if (nft_if_indextoname(ifindex, name))
-		printf("\"%s\"", name);
+		nft_print_to_output_ctx(octx, "\"%s\"", name);
 	else
-		printf("%d", ifindex);
+		nft_print_to_output_ctx(octx, "%d", ifindex);
 }
 
 static struct error_record *ifindex_type_parse(const struct expr *sym,

@@ -209,9 +211,9 @@ static void uid_type_print(const struct expr *expr, struct output_ctx *octx)
 
 		pw = getpwuid(uid);
 		if (pw != NULL)
-			printf("\"%s\"", pw->pw_name);
+			nft_print_to_output_ctx(octx, "\"%s\"", pw->pw_name);
 		else
-			printf("%d", uid);
+			nft_print_to_output_ctx(octx, "%d", uid);
 		return;
 	}
 	expr_basetype(expr)->print(expr, octx);

@@ -261,9 +263,9 @@ static void gid_type_print(const struct expr *expr, struct output_ctx *octx)
 
 		gr = getgrgid(gid);
 		if (gr != NULL)
-			printf("\"%s\"", gr->gr_name);
+			nft_print_to_output_ctx(octx, "\"%s\"", gr->gr_name);
 		else
-			printf("%u", gid);
+			nft_print_to_output_ctx(octx, "%u", gid);
 		return;
 	}
 	expr_basetype(expr)->print(expr, octx);

@@ -446,9 +448,11 @@ static bool meta_key_is_qualified(enum nft_meta_keys key)
 static void meta_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	if (meta_key_is_qualified(expr->meta.key))
-		printf("meta %s", meta_templates[expr->meta.key].token);
+		nft_print_to_output_ctx(octx, "meta %s",
+				        meta_templates[expr->meta.key].token);
 	else
-		printf("%s", meta_templates[expr->meta.key].token);
+		nft_print_to_output_ctx(octx, "%s",
+				        meta_templates[expr->meta.key].token);
 }
 
 static bool meta_expr_cmp(const struct expr *e1, const struct expr *e2)

@@ -573,9 +577,11 @@ struct expr *meta_expr_alloc(const struct location *loc, enum nft_meta_keys key)
 static void meta_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
 	if (meta_key_is_qualified(stmt->meta.key))
-		printf("meta %s set ", meta_templates[stmt->meta.key].token);
+		nft_print_to_output_ctx(octx, "meta %s set ",
+					meta_templates[stmt->meta.key].token);
 	else
-		printf("%s set ", meta_templates[stmt->meta.key].token);
+		nft_print_to_output_ctx(octx, "%s set ",
+					meta_templates[stmt->meta.key].token);
 
 	expr_print(stmt->meta.expr, octx);
 }

src/numgen.c +5 -3

index 19a4a9c..af844fb 100644
@@ -30,10 +30,12 @@ static const char *numgen_type_str(enum nft_ng_types type)
 
 static void numgen_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	printf("numgen %s mod %u", numgen_type_str(expr->numgen.type),
-	       expr->numgen.mod);
+	nft_print_to_output_ctx(octx, "numgen %s mod %u",
+				numgen_type_str(expr->numgen.type),
+				expr->numgen.mod);
 	if (expr->numgen.offset)
-		printf(" offset %u", expr->numgen.offset);
+		nft_print_to_output_ctx(octx, " offset %u",
+				        expr->numgen.offset);
 }
 
 static bool numgen_expr_cmp(const struct expr *e1, const struct expr *e2)

src/payload.c +5 -4

index 7f94ff7..be05e22 100644
@@ -46,11 +46,12 @@ static void payload_expr_print(const struct expr *expr, struct output_ctx *octx)
 	desc = expr->payload.desc;
 	tmpl = expr->payload.tmpl;
 	if (payload_is_known(expr))
-		printf("%s %s", desc->name, tmpl->token);
+		nft_print_to_output_ctx(octx, "%s %s", desc->name,
+				        tmpl->token);
 	else
-		printf("payload @%s,%u,%u",
-		       proto_base_tokens[expr->payload.base],
-		       expr->payload.offset, expr->len);
+		nft_print_to_output_ctx(octx, "payload @%s,%u,%u",
+				        proto_base_tokens[expr->payload.base],
+				        expr->payload.offset, expr->len);
 }
 
 static bool payload_expr_cmp(const struct expr *e1, const struct expr *e2)

src/rule.c +128 ~~-100~~

458 lines not on this page · raw message

src/statement.c +78 ~~-67~~

353 lines not on this page · raw message

Signature · 1 line

2.14.1

Pablo Neira Ayuso

maintainer

2017-09-03 22:35 UTC #6 lore raw

Re: [nft PATCH 2/2] src: get rid of printf

Quoted 104 lines · Eric Leblond

This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.

Signed-off-by Eric Leblond <[email protected]>

---
 include/datatype.h   |   5 +-
 include/expression.h |   2 +-
 include/nftables.h   |   5 ++
 src/cli.c            |   1 +
 src/ct.c             |  21 ++---
 src/datatype.c       |  66 ++++++++-------
 src/expression.c     |  79 ++++++++++--------
 src/exthdr.c         |  16 ++--
 src/fib.c            |  23 +++---
 src/hash.c           |  10 +--
 src/main.c           |  45 ++++++++++
 src/meta.c           |  32 +++++---
 src/numgen.c         |   8 +-
 src/payload.c        |   9 +-
 src/rule.c           | 228 +++++++++++++++++++++++++++++----------------------
 src/statement.c      | 145 +++++++++++++++++---------------
 16 files changed, 408 insertions(+), 287 deletions(-)

include/datatype.h +3 -2

index 2e34591..e9f6079 100644
@@ -209,7 +209,8 @@ extern void symbolic_constant_print(const struct symbol_table *tbl,
 				    struct output_ctx *octx);
 extern void symbol_table_print(const struct symbol_table *tbl,
 			       const struct datatype *dtype,
-			       enum byteorder byteorder);
+			       enum byteorder byteorder,
+			       struct output_ctx *octx);
 
 extern struct symbol_table *rt_symbol_table_init(const char *filename);
 extern void rt_symbol_table_free(struct symbol_table *tbl);

@@ -261,7 +262,7 @@ extern const struct datatype *
 set_datatype_alloc(const struct datatype *orig_dtype, unsigned int byteorder);
 extern void set_datatype_destroy(const struct datatype *dtype);
 
-extern void time_print(uint64_t seconds);
+extern void time_print(uint64_t seconds, struct output_ctx *octx);
 extern struct error_record *time_parse(const struct location *loc,
 				       const char *c, uint64_t *res);

include/expression.h +1 -1

index 32d4423..ce6b702 100644
@@ -334,7 +334,7 @@ extern struct expr *expr_get(struct expr *expr);
 extern void expr_free(struct expr *expr);
 extern void expr_print(const struct expr *expr, struct output_ctx *octx);
 extern bool expr_cmp(const struct expr *e1, const struct expr *e2);
-extern void expr_describe(const struct expr *expr);
+extern void expr_describe(const struct expr *expr, struct output_ctx *octx);
 
 extern const struct datatype *expr_basetype(const struct expr *expr);
 extern void expr_set_type(struct expr *expr, const struct datatype *dtype,

include/nftables.h +5 -0

index 7c4e93f..f4d5ce1 100644
@@ -30,6 +30,8 @@ struct output_ctx {
 	unsigned int ip2name;
 	unsigned int handle;
 	unsigned int echo;
+	char *output_buf;
+	size_t output_buf_len;
 };
 
 struct nft_cache {

@@ -149,4 +151,7 @@ void realm_table_meta_exit(void);
 void devgroup_table_exit(void);
 void realm_table_rt_exit(void);
 
+int nft_print_to_output_ctx(struct output_ctx *octx, const char *fmt, ...);
+char *nft_ctx_get_output(struct nft_ctx *ctx);
+
 #endif /* NFTABLES_NFTABLES_H */

src/cli.c +1 -0

index d923ff7..ca4418c 100644
@@ -138,6 +138,7 @@ static void cli_complete(char *line)
 		    cli_nft->debug_mask);
 	scanner_push_buffer(scanner, &indesc_cli, line);
 	nft_run(cli_nft, cli_nf_sock, scanner, state, &msgs);
+	printf("%s", nft_ctx_get_output(cli_nft));
 	erec_print_list(stdout, &msgs, cli_nft->debug_mask);
 	xfree(line);
 	cache_release(&cli_nft->cache);

src/ct.c +0 -0

index d64f467..f19608a 100644

src/ct.c static void ct_label_type_print(const struct expr *expr, · line 148

Quoted 10 lines

src/ct.c +2 -2

@@ -141,11 +141,12 @@ static void ct_label_type_print(const struct expr *expr,
 	for (s = ct_label_tbl->symbols; s->identifier != NULL; s++) {
 		if (bit != s->value)
 			continue;
-		printf("\"%s\"", s->identifier);
+		nft_print_to_output_ctx(octx, "\"%s\"", s->identifier);
 		return;
 	}
 	/* can happen when connlabel.conf is altered after rules were added */
-	printf("%ld\n", (long)mpz_scan1(expr->value, 0));
+	nft_print_to_output_ctx(octx, "%ld\n",

Let's call this nft_print(), ok?

I can make the rename here before applying.

I have a bad record of making functions unnecessarily long, so I'm trying to fix it this time. Anyway, this will be internal, so no problem.

Eric Leblond

author

2017-09-04 07:45 UTC #7 lore raw

Re: [nft PATCH 2/2] src: get rid of printf

Hi,

Quoted 126 lines · Pablo Neira Ayuso

Quoted 125 lines · Eric Leblond
This patch introduces the nft_print_to_output_ctx function that has to be used instead of printf to output information that where previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output()
function.
This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.
Signed-off-by Eric Leblond <[email protected]>
---
include/datatype.h   |   5 +-
include/expression.h |   2 +-
include/nftables.h   |   5 ++
src/cli.c            |   1 +
src/ct.c             |  21 ++---
src/datatype.c       |  66 ++++++++-------
src/expression.c     |  79 ++++++++++--------
src/exthdr.c         |  16 ++--
src/fib.c            |  23 +++---
 src/hash.c           |  10 +--
 src/main.c           |  45 ++++++++++
 src/meta.c           |  32 +++++---
src/numgen.c         |   8 +-
src/payload.c        |   9 +-
src/rule.c           | 228 +++++++++++++++++++++++++++++--------
--------------
src/statement.c      | 145 +++++++++++++++++---------------
 16 files changed, 408 insertions(+), 287 deletions(-)
include/datatype.h +2 -1
index 2e34591..e9f6079 100644
@@ -209,7 +209,8 @@ extern void symbolic_constant_print(const
 struct symbol_table *tbl,
  				    struct output_ctx *octx);
  extern void symbol_table_print(const struct symbol_table *tbl,
  			       const struct datatype *dtype,
-			       enum byteorder byteorder);
+			       enum byteorder byteorder,
+			       struct output_ctx *octx);
extern struct symbol_table *rt_symbol_table_init(const char *filename);
extern void rt_symbol_table_free(struct symbol_table *tbl);
include/datatype.h +0 -0
@@ -261,7 +262,7 @@ extern const struct datatype *
set_datatype_alloc(const struct datatype *orig_dtype, unsigned int byteorder);
extern void set_datatype_destroy(const struct datatype *dtype);
include/datatype.h +1 -1
-extern void time_print(uint64_t seconds);
+extern void time_print(uint64_t seconds, struct output_ctx *octx);
extern struct error_record *time_parse(const struct location *loc, const char *c, uint64_t *res);
include/expression.h +0 -0
index 32d4423..ce6b702 100644
@@ -334,7 +334,7 @@ extern struct expr *expr_get(struct expr
 *expr);
  extern void expr_free(struct expr *expr);
extern void expr_print(const struct expr *expr, struct output_ctx *octx);
extern bool expr_cmp(const struct expr *e1, const struct expr *e2);
include/expression.h +1 -1
-extern void expr_describe(const struct expr *expr);
+extern void expr_describe(const struct expr *expr, struct
output_ctx *octx);

extern const struct datatype *expr_basetype(const struct expr *expr);
extern void expr_set_type(struct expr *expr, const struct datatype *dtype,
include/nftables.h +2 -0
index 7c4e93f..f4d5ce1 100644
@@ -30,6 +30,8 @@ struct output_ctx {
  	unsigned int ip2name;
  	unsigned int handle;
  	unsigned int echo;
+	char *output_buf;
+	size_t output_buf_len;
  };
struct nft_cache {
include/nftables.h +0 -0
@@ -149,4 +151,7 @@ void realm_table_meta_exit(void);
  void devgroup_table_exit(void);
  void realm_table_rt_exit(void);
+int nft_print_to_output_ctx(struct output_ctx *octx, const char *fmt, ...);
include/nftables.h +2 -0
+char *nft_ctx_get_output(struct nft_ctx *ctx);
+
#endif /* NFTABLES_NFTABLES_H */
src/cli.c +1 -0
index d923ff7..ca4418c 100644
@@ -138,6 +138,7 @@ static void cli_complete(char *line)
  		    cli_nft->debug_mask);
  	scanner_push_buffer(scanner, &indesc_cli, line);
  	nft_run(cli_nft, cli_nf_sock, scanner, state, &msgs);
+	printf("%s", nft_ctx_get_output(cli_nft));
  	erec_print_list(stdout, &msgs, cli_nft->debug_mask);
  	xfree(line);
  	cache_release(&cli_nft->cache);
src/ct.c +1 -1
index d64f467..f19608a 100644
@@ -141,11 +141,12 @@ static void ct_label_type_print(const struct
 expr *expr,
  	for (s = ct_label_tbl->symbols; s->identifier != NULL;
 s++) {
  		if (bit != s->value)
  			continue;
-		printf("\"%s\"", s->identifier);
+		nft_print_to_output_ctx(octx, "\"%s\"", s-
 >identifier);

Quoted 8 lines

src/ct.c static void ct_label_type_print(const struct expr *expr, · line 148
Quoted 6 lines
src/ct.c +0 -0
  		return;
  	}
  	/* can happen when connlabel.conf is altered after rules
were added */
src/ct.c +1 -1
-	printf("%ld\n", (long)mpz_scan1(expr->value, 0));
+	nft_print_to_output_ctx(octx, "%ld\n",
Let's call this nft_print(), ok?

Yes, that's really better.

Quoted 2 lines

I can make the rename here before applying.

No need, I'm gonna resubmit the patchset.

BR,

Signature · 1 line

Eric Leblond <[email protected]>

Eric Leblond

author

2017-09-04 07:56 UTC #8 lore raw

PATCH v2 1/1 libnftables preparation work

In reply to Pablo Neira Ayuso

Hi,

This patchset update previous one following Pablo's suggestion:
* remove unused custom netlink flag
* rename ouput function to nft_print
* change default output buffer size to 4k instead of 128 that was used for debug (and causing a lot of realloc)

BR,
--
Eric

Eric Leblond

author

2017-09-04 07:56 UTC #9 lore raw

PATCH v2 1/2 src: add flags fo nft_ctx_new

By adding flags to nft_ctx_new, we will have a minimum capabilities of changing the way the nft_ctx is created.

For now, this patch uses a simple value that allow the user to specify that he will handle netlink by himself.

Signed-off-by Eric Leblond <[email protected]>

---
 include/nftables.h |  3 +++
 src/main.c         | 20 +++++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

include/nftables.h +3 -0

index 5035567..3429e4c 100644
@@ -49,8 +49,11 @@ struct nft_ctx {
 	struct output_ctx	output;
 	bool			check;
 	struct nft_cache	cache;
+	uint32_t		flags;
 };
 
+#define NFT_CTX_DEFAULT		0
+
 enum nftables_exit_codes {
 	NFT_EXIT_SUCCESS	= 0,
 	NFT_EXIT_FAILURE	= 1,

src/main.c +11 -9

index fce9bfe..702ef30 100644
@@ -281,7 +281,12 @@ static void nft_exit(void)
 	mark_table_exit();
 }
 
-static struct nft_ctx *nft_ctx_new(void)
+static void nft_ctx_netlink_init(struct nft_ctx *ctx)
+{
+	ctx->nf_sock = netlink_open_sock();
+}
+
+static struct nft_ctx *nft_ctx_new(uint32_t flags)
 {
 	struct nft_ctx *ctx;

@@ -292,6 +297,10 @@ static struct nft_ctx *nft_ctx_new(void)
 	ctx->num_include_paths	= 1;
 	ctx->parser_max_errors	= 10;
 	init_list_head(&ctx->cache.list);
+	ctx->flags = flags;
+
+	if (flags == NFT_CTX_DEFAULT)
+		nft_ctx_netlink_init(ctx);
 
 	return ctx;
 }

@@ -307,11 +316,6 @@ static void nft_ctx_free(const struct nft_ctx *ctx)
 	nft_exit();
 }
 
-static void nft_ctx_netlink_init(struct nft_ctx *ctx)
-{
-	ctx->nf_sock = netlink_open_sock();
-}
-
 static int nft_run_cmd_from_buffer(struct nft_ctx *nft,
 				   char *buf, size_t buflen)
 {

@@ -367,9 +371,7 @@ int main(int argc, char * const *argv)
 	struct parser_state state;
 	int i, val, rc;
 
-	nft = nft_ctx_new();
-
-	nft_ctx_netlink_init(nft);
+	nft = nft_ctx_new(NFT_CTX_DEFAULT);
 
 	while (1) {
 		val = getopt_long(argc, argv, OPTSTRING, options, NULL);

Signature · 1 line

2.14.1

Pablo Neira Ayuso

maintainer Applied

2017-09-04 20:43 UTC #10 lore raw

Re: [nft PATCH v2 1/2] src: add flags fo nft_ctx_new

Quoted 5 lines · Eric Leblond

By adding flags to nft_ctx_new, we will have a minimum capabilities of changing the way the nft_ctx is created.
For now, this patch uses a simple value that allow the user to specify that he will handle netlink by himself.

Applied, thanks Eric.

Eric Leblond

author

2017-09-04 07:56 UTC #11 lore raw

PATCH v2 2/2 src: get rid of printf

In reply to Eric Leblond

This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output() function.

This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.

Signed-off-by Eric Leblond <[email protected]>

---
 include/datatype.h   |   5 +-
 include/expression.h |   2 +-
 include/nftables.h   |   5 ++
 src/cli.c            |   1 +
 src/ct.c             |  20 ++---
 src/datatype.c       |  66 ++++++++-------
 src/expression.c     |  75 ++++++++---------
 src/exthdr.c         |  16 ++--
 src/fib.c            |  23 +++---
 src/hash.c           |  10 +--
 src/main.c           |  45 +++++++++++
 src/meta.c           |  32 +++++---
 src/numgen.c         |   8 +-
 src/payload.c        |   8 +-
 src/rule.c           | 222 ++++++++++++++++++++++++++++-----------------------
 src/statement.c      | 139 ++++++++++++++++----------------
 16 files changed, 390 insertions(+), 287 deletions(-)

include/datatype.h +3 -2

index 2e34591..e9f6079 100644
@@ -209,7 +209,8 @@ extern void symbolic_constant_print(const struct symbol_table *tbl,
 				    struct output_ctx *octx);
 extern void symbol_table_print(const struct symbol_table *tbl,
 			       const struct datatype *dtype,
-			       enum byteorder byteorder);
+			       enum byteorder byteorder,
+			       struct output_ctx *octx);
 
 extern struct symbol_table *rt_symbol_table_init(const char *filename);
 extern void rt_symbol_table_free(struct symbol_table *tbl);

@@ -261,7 +262,7 @@ extern const struct datatype *
 set_datatype_alloc(const struct datatype *orig_dtype, unsigned int byteorder);
 extern void set_datatype_destroy(const struct datatype *dtype);
 
-extern void time_print(uint64_t seconds);
+extern void time_print(uint64_t seconds, struct output_ctx *octx);
 extern struct error_record *time_parse(const struct location *loc,
 				       const char *c, uint64_t *res);

include/expression.h +1 -1

index 32d4423..ce6b702 100644
@@ -334,7 +334,7 @@ extern struct expr *expr_get(struct expr *expr);
 extern void expr_free(struct expr *expr);
 extern void expr_print(const struct expr *expr, struct output_ctx *octx);
 extern bool expr_cmp(const struct expr *e1, const struct expr *e2);
-extern void expr_describe(const struct expr *expr);
+extern void expr_describe(const struct expr *expr, struct output_ctx *octx);
 
 extern const struct datatype *expr_basetype(const struct expr *expr);
 extern void expr_set_type(struct expr *expr, const struct datatype *dtype,

include/nftables.h +5 -0

index 3429e4c..0ddfd3d 100644
@@ -30,6 +30,8 @@ struct output_ctx {
 	unsigned int ip2name;
 	unsigned int handle;
 	unsigned int echo;
+	char *output_buf;
+	size_t output_buf_len;
 };
 
 struct nft_cache {

@@ -148,4 +150,7 @@ void realm_table_meta_exit(void);
 void devgroup_table_exit(void);
 void realm_table_rt_exit(void);
 
+int nft_print(struct output_ctx *octx, const char *fmt, ...);
+char *nft_ctx_get_output(struct nft_ctx *ctx);
+
 #endif /* NFTABLES_NFTABLES_H */

src/cli.c +1 -0

index d923ff7..ca4418c 100644
@@ -138,6 +138,7 @@ static void cli_complete(char *line)
 		    cli_nft->debug_mask);
 	scanner_push_buffer(scanner, &indesc_cli, line);
 	nft_run(cli_nft, cli_nf_sock, scanner, state, &msgs);
+	printf("%s", nft_ctx_get_output(cli_nft));
 	erec_print_list(stdout, &msgs, cli_nft->debug_mask);
 	xfree(line);
 	cache_release(&cli_nft->cache);

src/ct.c +10 ~~-10~~

index d64f467..482fbe0 100644
@@ -141,11 +141,11 @@ static void ct_label_type_print(const struct expr *expr,
 	for (s = ct_label_tbl->symbols; s->identifier != NULL; s++) {
 		if (bit != s->value)
 			continue;
-		printf("\"%s\"", s->identifier);
+		nft_print(octx, "\"%s\"", s->identifier);
 		return;
 	}
 	/* can happen when connlabel.conf is altered after rules were added */
-	printf("%ld\n", (long)mpz_scan1(expr->value, 0));
+	nft_print(octx, "%ld\n", (long)mpz_scan1(expr->value, 0));
 }
 
 static struct error_record *ct_label_type_parse(const struct expr *sym,

@@ -269,27 +269,27 @@ static const struct ct_template ct_templates[] = {
 					      BYTEORDER_HOST_ENDIAN, 32),
 };
 
-static void ct_print(enum nft_ct_keys key, int8_t dir)
+static void ct_print(enum nft_ct_keys key, int8_t dir, struct output_ctx *octx)
 {
 	const struct symbolic_constant *s;
 
-	printf("ct ");
+	nft_print(octx, "ct ");
 	if (dir < 0)
 		goto done;
 
 	for (s = ct_dir_tbl.symbols; s->identifier != NULL; s++) {
 		if (dir == (int)s->value) {
-			printf("%s ", s->identifier);
+			nft_print(octx, "%s ", s->identifier);
 			break;
 		}
 	}
  done:
-	printf("%s", ct_templates[key].token);
+	nft_print(octx, "%s", ct_templates[key].token);
 }
 
 static void ct_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	ct_print(expr->ct.key, expr->ct.direction);
+	ct_print(expr->ct.key, expr->ct.direction, octx);
 }
 
 static bool ct_expr_cmp(const struct expr *e1, const struct expr *e2)

@@ -445,8 +445,8 @@ void ct_expr_update_type(struct proto_ctx *ctx, struct expr *expr)
 
 static void ct_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
-	ct_print(stmt->ct.key, stmt->ct.direction);
-	printf(" set ");
+	ct_print(stmt->ct.key, stmt->ct.direction, octx);
+	nft_print(octx, " set ");
 	expr_print(stmt->ct.expr, octx);
 }

@@ -472,7 +472,7 @@ struct stmt *ct_stmt_alloc(const struct location *loc, enum nft_ct_keys key,
 
 static void notrack_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
-	printf("notrack");
+	nft_print(octx, "notrack");
 }
 
 static const struct stmt_ops notrack_stmt_ops = {

src/datatype.c +37 ~~-29~~

180 lines not on this page · raw message

src/expression.c +39 ~~-36~~

188 lines not on this page · raw message

src/exthdr.c +10 -6

index 4add3da..7e59bbf 100644
@@ -34,20 +34,24 @@ static void exthdr_expr_print(const struct expr *expr, struct output_ctx *octx)
 		char buf[9] = {0};
 
 		if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT) {
-			printf("tcp option %s", expr->exthdr.desc->name);
+			nft_print(octx, "tcp option %s",
+				  expr->exthdr.desc->name);
 			return;
 		}
 
 		if (offset)
 			snprintf(buf, sizeof buf, "%d", offset);
-		printf("tcp option %s%s %s", expr->exthdr.desc->name, buf,
-					     expr->exthdr.tmpl->token);
+		nft_print(octx, "tcp option %s%s %s",
+			  expr->exthdr.desc->name, buf,
+			  expr->exthdr.tmpl->token);
 	} else {
 		if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT)
-			printf("exthdr %s", expr->exthdr.desc->name);
+			nft_print(octx, "exthdr %s",
+				  expr->exthdr.desc->name);
 		else {
-			printf("%s %s", expr->exthdr.desc ? expr->exthdr.desc->name : "unknown-exthdr",
-					expr->exthdr.tmpl->token);
+			nft_print(octx, "%s %s",
+				  expr->exthdr.desc ? expr->exthdr.desc->name : "unknown-exthdr",
+				  expr->exthdr.tmpl->token);
 		}
 	}
 }

src/fib.c +12 ~~-11~~

index b3488af..21bc69a 100644
@@ -60,32 +60,33 @@ static const char *fib_result_str(enum nft_fib_result result)
 	return "unknown";
 }
 
-static void __fib_expr_print_f(unsigned int *flags, unsigned int f, const char *s)
+static void __fib_expr_print_f(unsigned int *flags, unsigned int f,
+			       const char *s, struct output_ctx *octx)
 {
 	if ((*flags & f) == 0)
 		return;
 
-	printf("%s", s);
+	nft_print(octx, "%s", s);
 	*flags &= ~f;
 	if (*flags)
-		printf(" . ");
+		nft_print(octx, " . ");
 }
 
 static void fib_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	unsigned int flags = expr->fib.flags & ~NFTA_FIB_F_PRESENT;
 
-	printf("fib ");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_SADDR, "saddr");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_DADDR, "daddr");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_MARK, "mark");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_IIF, "iif");
-	__fib_expr_print_f(&flags, NFTA_FIB_F_OIF, "oif");
+	nft_print(octx, "fib ");
+	__fib_expr_print_f(&flags, NFTA_FIB_F_SADDR, "saddr", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_DADDR, "daddr", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_MARK, "mark", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_IIF, "iif", octx);
+	__fib_expr_print_f(&flags, NFTA_FIB_F_OIF, "oif", octx);
 
 	if (flags)
-		printf("0x%x", flags);
+		nft_print(octx, "0x%x", flags);
 
-	printf(" %s", fib_result_str(expr->fib.result));
+	nft_print(octx, " %s", fib_result_str(expr->fib.result));
 }
 
 static bool fib_expr_cmp(const struct expr *e1, const struct expr *e2)

src/hash.c +5 -5

index 1a4bfb3..9cd3c8c 100644
@@ -19,19 +19,19 @@ static void hash_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	switch (expr->hash.type) {
 	case NFT_HASH_SYM:
-		printf("symhash");
+		nft_print(octx, "symhash");
 	break;
 	case NFT_HASH_JENKINS:
 	default:
-		printf("jhash ");
+		nft_print(octx, "jhash ");
 		expr_print(expr->hash.expr, octx);
 	}
 
-	printf(" mod %u", expr->hash.mod);
+	nft_print(octx, " mod %u", expr->hash.mod);
 	if (expr->hash.seed_set)
-		printf(" seed 0x%x", expr->hash.seed);
+		nft_print(octx, " seed 0x%x", expr->hash.seed);
 	if (expr->hash.offset)
-		printf(" offset %u", expr->hash.offset);
+		nft_print(octx, " offset %u", expr->hash.offset);
 }
 
 static bool hash_expr_cmp(const struct expr *e1, const struct expr *e2)

src/main.c +45 -0

91 lines not on this page · raw message

src/meta.c +19 ~~-13~~

index 9c80893..56b9e29 100644
@@ -54,13 +54,15 @@ static void tchandle_type_print(const struct expr *expr,
 
 	switch(handle) {
 	case TC_H_ROOT:
-		printf("root");
+		nft_print(octx, "root");
 		break;
 	case TC_H_UNSPEC:
-		printf("none");
+		nft_print(octx, "none");
 		break;
 	default:
-		printf("%0x:%0x", TC_H_MAJ(handle) >> 16, TC_H_MIN(handle));
+		nft_print(octx, "%0x:%0x",
+			  TC_H_MAJ(handle) >> 16,
+			  TC_H_MIN(handle));
 		break;
 	}
 }

@@ -134,9 +136,9 @@ static void ifindex_type_print(const struct expr *expr, struct output_ctx *octx)
 
 	ifindex = mpz_get_uint32(expr->value);
 	if (nft_if_indextoname(ifindex, name))
-		printf("\"%s\"", name);
+		nft_print(octx, "\"%s\"", name);
 	else
-		printf("%d", ifindex);
+		nft_print(octx, "%d", ifindex);
 }
 
 static struct error_record *ifindex_type_parse(const struct expr *sym,

@@ -209,9 +211,9 @@ static void uid_type_print(const struct expr *expr, struct output_ctx *octx)
 
 		pw = getpwuid(uid);
 		if (pw != NULL)
-			printf("\"%s\"", pw->pw_name);
+			nft_print(octx, "\"%s\"", pw->pw_name);
 		else
-			printf("%d", uid);
+			nft_print(octx, "%d", uid);
 		return;
 	}
 	expr_basetype(expr)->print(expr, octx);

@@ -261,9 +263,9 @@ static void gid_type_print(const struct expr *expr, struct output_ctx *octx)
 
 		gr = getgrgid(gid);
 		if (gr != NULL)
-			printf("\"%s\"", gr->gr_name);
+			nft_print(octx, "\"%s\"", gr->gr_name);
 		else
-			printf("%u", gid);
+			nft_print(octx, "%u", gid);
 		return;
 	}
 	expr_basetype(expr)->print(expr, octx);

@@ -446,9 +448,11 @@ static bool meta_key_is_qualified(enum nft_meta_keys key)
 static void meta_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
 	if (meta_key_is_qualified(expr->meta.key))
-		printf("meta %s", meta_templates[expr->meta.key].token);
+		nft_print(octx, "meta %s",
+			  meta_templates[expr->meta.key].token);
 	else
-		printf("%s", meta_templates[expr->meta.key].token);
+		nft_print(octx, "%s",
+			  meta_templates[expr->meta.key].token);
 }
 
 static bool meta_expr_cmp(const struct expr *e1, const struct expr *e2)

@@ -573,9 +577,11 @@ struct expr *meta_expr_alloc(const struct location *loc, enum nft_meta_keys key)
 static void meta_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 {
 	if (meta_key_is_qualified(stmt->meta.key))
-		printf("meta %s set ", meta_templates[stmt->meta.key].token);
+		nft_print(octx, "meta %s set ",
+			  meta_templates[stmt->meta.key].token);
 	else
-		printf("%s set ", meta_templates[stmt->meta.key].token);
+		nft_print(octx, "%s set ",
+			  meta_templates[stmt->meta.key].token);
 
 	expr_print(stmt->meta.expr, octx);
 }

src/numgen.c +5 -3

index 19a4a9c..3d542fd 100644
@@ -30,10 +30,12 @@ static const char *numgen_type_str(enum nft_ng_types type)
 
 static void numgen_expr_print(const struct expr *expr, struct output_ctx *octx)
 {
-	printf("numgen %s mod %u", numgen_type_str(expr->numgen.type),
-	       expr->numgen.mod);
+	nft_print(octx, "numgen %s mod %u",
+		  numgen_type_str(expr->numgen.type),
+		  expr->numgen.mod);
 	if (expr->numgen.offset)
-		printf(" offset %u", expr->numgen.offset);
+		nft_print(octx, " offset %u",
+			  expr->numgen.offset);
 }
 
 static bool numgen_expr_cmp(const struct expr *e1, const struct expr *e2)

src/payload.c +4 -4

index 7f94ff7..832104d 100644
@@ -46,11 +46,11 @@ static void payload_expr_print(const struct expr *expr, struct output_ctx *octx)
 	desc = expr->payload.desc;
 	tmpl = expr->payload.tmpl;
 	if (payload_is_known(expr))
-		printf("%s %s", desc->name, tmpl->token);
+		nft_print(octx, "%s %s", desc->name, tmpl->token);
 	else
-		printf("payload @%s,%u,%u",
-		       proto_base_tokens[expr->payload.base],
-		       expr->payload.offset, expr->len);
+		nft_print(octx, "payload @%s,%u,%u",
+			  proto_base_tokens[expr->payload.base],
+			  expr->payload.offset, expr->len);
 }
 
 static bool payload_expr_cmp(const struct expr *e1, const struct expr *e2)

src/rule.c +122 ~~-100~~

452 lines not on this page · raw message

src/statement.c +72 ~~-67~~

347 lines not on this page · raw message

Signature · 1 line

2.14.1

Pablo Neira Ayuso

maintainer

2017-09-04 20:44 UTC #12 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 8 lines · Eric Leblond

This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output() function.
This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.

tests/shell/./run-tests

is reporting problems with this :-(

Pablo Neira Ayuso

maintainer

2017-09-04 20:53 UTC #13 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 6 lines · Pablo Neira Ayuso

Quoted 5 lines · Eric Leblond
This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output() function.

Quoted 7 lines

Quoted 3 lines
This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.
tests/shell/./run-tests
is reporting problems with this :-(

Eric, I can also see there are missing printf() to nft_print() conversion on the monitor side.

I would expect the simple API works like this: if you pass the "monitor" command, it justs prints events to whatever destination you have specified.

This is the most simple integration that makes sense to me.

Therefore, I think it would be good to convert those too to use nft_print().

Let me know, thanks.

Eric Leblond

author

2017-09-04 21:23 UTC #14 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Hi,

Quoted 21 lines · Pablo Neira Ayuso

Quoted 13 lines · Pablo Neira Ayuso
Quoted 11 lines · Eric Leblond
This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can
be fetched by the user with the nft_ctx_get_output() function.
This modification will allow the libnftables library to provide an
easy way to the users to get the output data and display them like
they want.
Quoted 3 lines
tests/shell/./run-tests
is reporting problems with this :-(
Eric, I can also see there are missing printf() to nft_print() conversion on the monitor side.

Quoted 8 lines

I would expect the simple API works like this: if you pass the "monitor" command, it justs prints events to whatever destination you have specified.
This is the most simple integration that makes sense to me.
Therefore, I think it would be good to convert those too to use nft_print().

Yes indeed, that make sense. For each message, the user get the human formatted version of the event.

Do you want me to cook that based on the previously attached patch ?

BR,

Signature · 1 line

Eric Leblond <[email protected]>

Pablo Neira Ayuso

maintainer

2017-09-05 17:34 UTC #15 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 28 lines · Eric Leblond

Hi,
Quoted 25 lines · Pablo Neira Ayuso
Quoted 13 lines · Pablo Neira Ayuso
Quoted 11 lines · Eric Leblond
This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can
be fetched by the user with the nft_ctx_get_output() function.
This modification will allow the libnftables library to provide an
easy way to the users to get the output data and display them like
they want.
Quoted 3 lines
tests/shell/./run-tests
is reporting problems with this :-(
Eric, I can also see there are missing printf() to nft_print() conversion on the monitor side.
I would expect the simple API works like this: if you pass the "monitor" command, it justs prints events to whatever destination you have specified.

Quoted 7 lines

Quoted 4 lines
This is the most simple integration that makes sense to me.
Therefore, I think it would be good to convert those too to use nft_print().
Yes indeed, that make sense. For each message, the user get the human formatted version of the event.

Yes. Once we have json support for this - in the high level representation - we could add a function to specify the print mode. But that is something we can of course later on, just thinking aloud.

Quoted 1 line

Do you want me to cook that based on the previously attached patch ?

Please do. Thanks!

Phil Sutter

reviewer

2017-09-21 15:37 UTC #16 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

In reply to Pablo Neira Ayuso

Hi,

Quoted 22 lines · Pablo Neira Ayuso

Quoted 6 lines · Pablo Neira Ayuso
Quoted 5 lines · Eric Leblond
This patch introduces the nft_print function that has to be used instead of printf to output information that were previously send to stdout. This function accumulate the output in a buffer that can be fetched by the user with the nft_ctx_get_output() function.
Quoted 7 lines
Quoted 3 lines
This modification will allow the libnftables library to provide an easy way to the users to get the output data and display them like they want.
tests/shell/./run-tests
is reporting problems with this :-(
Eric, I can also see there are missing printf() to nft_print() conversion on the monitor side.
I would expect the simple API works like this: if you pass the "monitor" command, it justs prints events to whatever destination you have specified.

Quoted 6 lines

This is the most simple integration that makes sense to me.
Therefore, I think it would be good to convert those too to use nft_print().
Let me know, thanks.

With the proposed implementation of nft_print(), this will be
problematic: nft_run_cmd_from_buffer() waits for the command to finish before printing all the output at once. This obviously breaks monitor which runs endlessly.

Maybe we should reference nft_print via a function pointer in output_ctx? This would allow to configure a different nft_print implementation which flushes the buffer to stdout immediately.

What do you think?

Cheers, Phil

Florian Westphal

maintainer

2017-09-21 15:46 UTC #17 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 10 lines · Phil Sutter

With the proposed implementation of nft_print(), this will be
problematic: nft_run_cmd_from_buffer() waits for the command to finish before printing all the output at once. This obviously breaks monitor which runs endlessly.
Maybe we should reference nft_print via a function pointer in output_ctx? This would allow to configure a different nft_print implementation which flushes the buffer to stdout immediately.
What do you think?

I wondered the same thing.

I have following issue: When using new typeof keyword then listing a set definition should output something like

set foo { type typeof(meta iifname) }

Because the kernel has no notion of 'meta iifname', we have to store this information in the kernel so we can read it back during delinearization.

One way to do this would be to store the 'meta iifname' string in the sets userdata.

For that, expr_print() would have to be able to print to a buffer (or we would need an expr_snprintf or something similar) to extract the convert struct *expr back to its original name.

The other solution would be to stash this in the expression during parsing but that seems weird as we need to be able to do such conversion anyway when printing the ruleset, so we merely need to make this accessible outside of plain printf() to stdout.

Pablo Neira Ayuso

maintainer

2017-09-21 15:51 UTC #18 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 26 lines · Florian Westphal

Quoted 10 lines · Phil Sutter
With the proposed implementation of nft_print(), this will be
problematic: nft_run_cmd_from_buffer() waits for the command to finish before printing all the output at once. This obviously breaks monitor which runs endlessly.
Maybe we should reference nft_print via a function pointer in output_ctx? This would allow to configure a different nft_print implementation which flushes the buffer to stdout immediately.
What do you think?
I wondered the same thing.
I have following issue: When using new typeof keyword then listing a set definition should output something like
set foo { type typeof(meta iifname) }
Because the kernel has no notion of 'meta iifname', we have to store this information in the kernel so we can read it back during delinearization.
One way to do this would be to store the 'meta iifname' string in the sets userdata.

Quoted 8 lines

For that, expr_print() would have to be able to print to a buffer (or we would need an expr_snprintf or something similar) to extract the convert struct *expr back to its original name.
The other solution would be to stash this in the expression during parsing but that seems weird as we need to be able to do such conversion anyway when printing the ruleset, so we merely need to make this accessible outside of plain printf() to stdout.

Probably the underlying problem is that the monitor code is that not delinearing, ie. transforming from netlink to abstract syntax tree (ast) before printing?

Phil Sutter

reviewer

2017-09-21 16:21 UTC #19 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Hi,

Quoted 31 lines · Pablo Neira Ayuso

Quoted 30 lines · Florian Westphal
Quoted 10 lines · Phil Sutter
With the proposed implementation of nft_print(), this will be
problematic: nft_run_cmd_from_buffer() waits for the command to finish before printing all the output at once. This obviously breaks monitor which runs endlessly.
Maybe we should reference nft_print via a function pointer in output_ctx? This would allow to configure a different nft_print implementation which flushes the buffer to stdout immediately.
What do you think?
I wondered the same thing.
I have following issue: When using new typeof keyword then listing a set definition should output something like
set foo { type typeof(meta iifname) }
Because the kernel has no notion of 'meta iifname', we have to store this information in the kernel so we can read it back during delinearization.
One way to do this would be to store the 'meta iifname' string in the sets userdata.
For that, expr_print() would have to be able to print to a buffer (or we would need an expr_snprintf or something similar) to extract the convert struct *expr back to its original name.

Quoted 8 lines

Quoted 4 lines
The other solution would be to stash this in the expression during parsing but that seems weird as we need to be able to do such conversion anyway when printing the ruleset, so we merely need to make this accessible outside of plain printf() to stdout.
Probably the underlying problem is that the monitor code is that not delinearing, ie. transforming from netlink to abstract syntax tree (ast) before printing?

No, we are talking about different issues (but with a potential common solution):

Florian searches for a way to "convert" an expression from it's binary form (a struct expr) into a human readable string (here: "meta iifname"). We have this "converter" already in very limited form, namely expr_print(). Eric's patch changes that function to make use of nft_print(), which effectively allows to define a custom buffer to print into.

Florian, did I get this right?

Cheers, Phil

Florian Westphal

maintainer

2017-09-21 17:09 UTC #20 lore raw

Re: [nft PATCH v2 2/2] src: get rid of printf

Quoted 44 lines · Phil Sutter

Quoted 31 lines · Pablo Neira Ayuso
Quoted 30 lines · Florian Westphal
Quoted 10 lines · Phil Sutter
With the proposed implementation of nft_print(), this will be
problematic: nft_run_cmd_from_buffer() waits for the command to finish before printing all the output at once. This obviously breaks monitor which runs endlessly.
Maybe we should reference nft_print via a function pointer in output_ctx? This would allow to configure a different nft_print implementation which flushes the buffer to stdout immediately.
What do you think?
I wondered the same thing.
I have following issue: When using new typeof keyword then listing a set definition should output something like
set foo { type typeof(meta iifname) }
Because the kernel has no notion of 'meta iifname', we have to store this information in the kernel so we can read it back during delinearization.
One way to do this would be to store the 'meta iifname' string in the sets userdata.
For that, expr_print() would have to be able to print to a buffer (or we would need an expr_snprintf or something similar) to extract the convert struct *expr back to its original name.
Quoted 8 lines
Quoted 4 lines
The other solution would be to stash this in the expression during parsing but that seems weird as we need to be able to do such conversion anyway when printing the ruleset, so we merely need to make this accessible outside of plain printf() to stdout.
Probably the underlying problem is that the monitor code is that not delinearing, ie. transforming from netlink to abstract syntax tree (ast) before printing?
No, we are talking about different issues (but with a potential common solution):

Quoted 8 lines

Florian searches for a way to "convert" an expression from it's binary form (a struct expr) into a human readable string (here: "meta iifname"). We have this "converter" already in very limited form, namely expr_print(). Eric's patch changes that function to make use of nft_print(), which effectively allows to define a custom buffer to print into.
Florian, did I get this right?

Yes, thats right.

(Pablo, I do not NEED this function, but, as far is I understood
you want nft to list

set name { type typeof(ip saddr) }

back exactly like this, keeping the 'typeof'. And AFAICS the best way to do this is to store the human readable "ip saddr" as-is in the userdata area and then just print that back.

If I misunderstood what you were saying, please clarify.

Ultimately something is needed to annotate in any case because in case of 'meta iifname . ipv4_addr' we don't have anything to go on when deliarizing as the string type lacks any length information and we don't have any rule context from set definition to infer the needed key length information of the interface name.