* [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
@ 2019-05-16 20:45 Fernando Fernandez Mancera
2019-05-16 20:45 ` [PATCH nft v3 2/2] jump: Allow goto and jump to a variable using nft input files Fernando Fernandez Mancera
2019-05-21 9:28 ` [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Pablo Neira Ayuso
0 siblings, 2 replies; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2019-05-16 20:45 UTC (permalink / raw)
To: netfilter-devel; +Cc: Fernando Fernandez Mancera
Now we can introduce expressions as a chain in jump and goto statements. This
is going to be used to support variables as a chain in the following patches.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: Initial patch
v2: Use expr_cmp in verdict expr cmp callback
v3: Fix strange variable cases
---
include/expression.h | 4 ++--
src/datatype.c | 22 ++++++++++++++++++++--
src/evaluate.c | 4 ++++
src/expression.c | 12 ++++++------
src/netlink.c | 26 +++++++++++++++++++++-----
src/parser_bison.y | 17 +++++++++++++----
6 files changed, 66 insertions(+), 19 deletions(-)
diff --git a/include/expression.h b/include/expression.h
index 6416ac0..ef41255 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -240,7 +240,7 @@ struct expr {
struct {
/* EXPR_VERDICT */
int verdict;
- const char *chain;
+ struct expr *chain;
};
struct {
/* EXPR_VALUE */
@@ -403,7 +403,7 @@ extern void relational_expr_pctx_update(struct proto_ctx *ctx,
const struct expr *expr);
extern struct expr *verdict_expr_alloc(const struct location *loc,
- int verdict, const char *chain);
+ int verdict, struct expr *chain);
extern struct expr *symbol_expr_alloc(const struct location *loc,
enum symbol_types type, struct scope *scope,
diff --git a/src/datatype.c b/src/datatype.c
index ac9f2af..10f185b 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -254,6 +254,8 @@ const struct datatype invalid_type = {
static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
{
+ char chain[NFT_CHAIN_MAXNAMELEN];
+
switch (expr->verdict) {
case NFT_CONTINUE:
nft_print(octx, "continue");
@@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
nft_print(octx, "break");
break;
case NFT_JUMP:
- nft_print(octx, "jump %s", expr->chain);
+ if (expr->chain->etype == EXPR_VALUE) {
+ mpz_export_data(chain, expr->chain->value,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN);
+ nft_print(octx, "jump %s", chain);
+ } else {
+ nft_print(octx, "jump ");
+ expr_print(expr->chain, octx);
+ }
break;
case NFT_GOTO:
- nft_print(octx, "goto %s", expr->chain);
+ if (expr->chain->etype == EXPR_VALUE) {
+ mpz_export_data(chain, expr->chain->value,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN);
+ nft_print(octx, "goto %s", chain);
+ } else {
+ nft_print(octx, "goto ");
+ expr_print(expr->chain, octx);
+ }
break;
case NFT_RETURN:
nft_print(octx, "return");
diff --git a/src/evaluate.c b/src/evaluate.c
index 21d9e14..8394037 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1947,6 +1947,10 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
case EXPR_VERDICT:
if (stmt->expr->verdict != NFT_CONTINUE)
stmt->flags |= STMT_F_TERMINAL;
+ if (stmt->expr->chain != NULL) {
+ if (expr_evaluate(ctx, &stmt->expr->chain) < 0)
+ return -1;
+ }
break;
case EXPR_MAP:
break;
diff --git a/src/expression.c b/src/expression.c
index eece12e..a41e2da 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -207,22 +207,22 @@ static bool verdict_expr_cmp(const struct expr *e1, const struct expr *e2)
if ((e1->verdict == NFT_JUMP ||
e1->verdict == NFT_GOTO) &&
- strcmp(e1->chain, e2->chain))
- return false;
+ expr_cmp(e1->chain, e2->chain))
+ return true;
- return true;
+ return false;
}
static void verdict_expr_clone(struct expr *new, const struct expr *expr)
{
new->verdict = expr->verdict;
if (expr->chain != NULL)
- new->chain = xstrdup(expr->chain);
+ mpz_init_set(new->chain->value, expr->chain->value);
}
static void verdict_expr_destroy(struct expr *expr)
{
- xfree(expr->chain);
+ expr_free(expr->chain);
}
static const struct expr_ops verdict_expr_ops = {
@@ -236,7 +236,7 @@ static const struct expr_ops verdict_expr_ops = {
};
struct expr *verdict_expr_alloc(const struct location *loc,
- int verdict, const char *chain)
+ int verdict, struct expr *chain)
{
struct expr *expr;
diff --git a/src/netlink.c b/src/netlink.c
index c051ae6..ef12cb0 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -218,12 +218,17 @@ static void netlink_gen_constant_data(const struct expr *expr,
static void netlink_gen_verdict(const struct expr *expr,
struct nft_data_linearize *data)
{
+ char chain[NFT_CHAIN_MAXNAMELEN];
+
data->verdict = expr->verdict;
switch (expr->verdict) {
case NFT_JUMP:
case NFT_GOTO:
- snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", expr->chain);
+ mpz_export_data(chain, expr->chain->value,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN);
+ snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain);
data->chain[NFT_CHAIN_MAXNAMELEN-1] = '\0';
break;
}
@@ -253,12 +258,15 @@ struct expr *netlink_alloc_value(const struct location *loc,
static struct expr *netlink_alloc_verdict(const struct location *loc,
const struct nft_data_delinearize *nld)
{
- char *chain;
+ struct expr *chain;
switch (nld->verdict) {
case NFT_JUMP:
case NFT_GOTO:
- chain = xstrdup(nld->chain);
+ chain = constant_expr_alloc(loc, &string_type,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN *
+ BITS_PER_BYTE, nld->chain);
break;
default:
chain = NULL;
@@ -1153,14 +1161,22 @@ static void trace_print_expr(const struct nftnl_trace *nlt, unsigned int attr,
static void trace_print_verdict(const struct nftnl_trace *nlt,
struct output_ctx *octx)
{
+ struct expr *chain_expr = NULL;
const char *chain = NULL;
unsigned int verdict;
struct expr *expr;
verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_VERDICT);
- if (nftnl_trace_is_set(nlt, NFTNL_TRACE_JUMP_TARGET))
+ if (nftnl_trace_is_set(nlt, NFTNL_TRACE_JUMP_TARGET)) {
chain = xstrdup(nftnl_trace_get_str(nlt, NFTNL_TRACE_JUMP_TARGET));
- expr = verdict_expr_alloc(&netlink_location, verdict, chain);
+ chain_expr = constant_expr_alloc(&netlink_location,
+ &string_type,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN
+ * BITS_PER_BYTE,
+ chain);
+ }
+ expr = verdict_expr_alloc(&netlink_location, verdict, chain_expr);
nft_print(octx, "verdict ");
expr_print(expr, octx);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 9e632c0..b1e29a8 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -618,8 +618,8 @@ int nft_lex(void *, void *, void *);
%type <stmt> meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
%destructor { stmt_free($$); } meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
-%type <expr> symbol_expr verdict_expr integer_expr variable_expr
-%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr
+%type <expr> symbol_expr verdict_expr integer_expr variable_expr chain_expr
+%destructor { expr_free($$); } symbol_expr verdict_expr integer_expr variable_expr chain_expr
%type <expr> primary_expr shift_expr and_expr
%destructor { expr_free($$); } primary_expr shift_expr and_expr
%type <expr> exclusive_or_expr inclusive_or_expr
@@ -3827,11 +3827,11 @@ verdict_expr : ACCEPT
{
$$ = verdict_expr_alloc(&@$, NFT_CONTINUE, NULL);
}
- | JUMP identifier
+ | JUMP chain_expr
{
$$ = verdict_expr_alloc(&@$, NFT_JUMP, $2);
}
- | GOTO identifier
+ | GOTO chain_expr
{
$$ = verdict_expr_alloc(&@$, NFT_GOTO, $2);
}
@@ -3841,6 +3841,15 @@ verdict_expr : ACCEPT
}
;
+chain_expr : identifier
+ {
+ $$ = constant_expr_alloc(&@$, &string_type,
+ BYTEORDER_HOST_ENDIAN,
+ NFT_CHAIN_MAXNAMELEN *
+ BITS_PER_BYTE, $1);
+ }
+ ;
+
meta_expr : META meta_key
{
$$ = meta_expr_alloc(&@$, $2);
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nft v3 2/2] jump: Allow goto and jump to a variable using nft input files
2019-05-16 20:45 [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Fernando Fernandez Mancera
@ 2019-05-16 20:45 ` Fernando Fernandez Mancera
2019-05-21 9:28 ` [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Pablo Neira Ayuso
1 sibling, 0 replies; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2019-05-16 20:45 UTC (permalink / raw)
To: netfilter-devel; +Cc: Fernando Fernandez Mancera
This patch introduces the use of nft input files variables in 'jump' and 'goto'
statements, e.g.
define dest = ber
add table ip foo
add chain ip foo bar {type filter hook input priority 0;}
add chain ip foo ber
add rule ip foo ber counter
add rule ip foo bar jump $dest
table ip foo {
chain bar {
type filter hook input priority filter; policy accept;
jump ber
}
chain ber {
counter packets 71 bytes 6664
}
}
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: Initial patch
v2: Add shell tests
v3: Fix strange cases and more test cases
---
src/datatype.c | 11 ++++++++++
src/evaluate.c | 7 +++++++
src/parser_bison.y | 3 ++-
.../shell/testcases/nft-f/0018jump_variable_0 | 19 ++++++++++++++++++
.../shell/testcases/nft-f/0019jump_variable_1 | 20 +++++++++++++++++++
.../shell/testcases/nft-f/0020jump_variable_1 | 20 +++++++++++++++++++
.../nft-f/dumps/0018jump_variable_0.nft | 8 ++++++++
7 files changed, 87 insertions(+), 1 deletion(-)
create mode 100755 tests/shell/testcases/nft-f/0018jump_variable_0
create mode 100755 tests/shell/testcases/nft-f/0019jump_variable_1
create mode 100755 tests/shell/testcases/nft-f/0020jump_variable_1
create mode 100644 tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft
diff --git a/src/datatype.c b/src/datatype.c
index 10f185b..1d5ed6f 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -309,11 +309,22 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
}
}
+static struct error_record *verdict_type_parse(const struct expr *sym,
+ struct expr **res)
+{
+ *res = constant_expr_alloc(&sym->location, &string_type,
+ BYTEORDER_HOST_ENDIAN,
+ (strlen(sym->identifier) + 1) * BITS_PER_BYTE,
+ sym->identifier);
+ return NULL;
+}
+
const struct datatype verdict_type = {
.type = TYPE_VERDICT,
.name = "verdict",
.desc = "netfilter verdict",
.print = verdict_type_print,
+ .parse = verdict_type_parse,
};
static const struct symbol_table nfproto_tbl = {
diff --git a/src/evaluate.c b/src/evaluate.c
index 8394037..55fb3b6 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1950,6 +1950,13 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
if (stmt->expr->chain != NULL) {
if (expr_evaluate(ctx, &stmt->expr->chain) < 0)
return -1;
+ if ((stmt->expr->chain->etype != EXPR_SYMBOL &&
+ stmt->expr->chain->etype != EXPR_VALUE) ||
+ stmt->expr->chain->symtype != SYMBOL_VALUE) {
+ return stmt_error(ctx, stmt,
+ "invalid verdict chain expression %s\n",
+ expr_name(stmt->expr->chain));
+ }
}
break;
case EXPR_MAP:
diff --git a/src/parser_bison.y b/src/parser_bison.y
index b1e29a8..0fea3c6 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3841,7 +3841,8 @@ verdict_expr : ACCEPT
}
;
-chain_expr : identifier
+chain_expr : variable_expr
+ | identifier
{
$$ = constant_expr_alloc(&@$, &string_type,
BYTEORDER_HOST_ENDIAN,
diff --git a/tests/shell/testcases/nft-f/0018jump_variable_0 b/tests/shell/testcases/nft-f/0018jump_variable_0
new file mode 100755
index 0000000..003a1bd
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0018jump_variable_0
@@ -0,0 +1,19 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = ber
+
+table ip foo {
+ chain bar {
+ jump \$dest
+ }
+
+ chain ber {
+ }
+}"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/nft-f/0019jump_variable_1 b/tests/shell/testcases/nft-f/0019jump_variable_1
new file mode 100755
index 0000000..bda861c
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0019jump_variable_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = { 1024 }
+
+table ip foo {
+ chain bar {
+ jump \$dest
+ }
+
+ chain ber {
+ }
+}"
+
+$NFT -f - <<< "$RULESET" && exit 1
+exit 0
diff --git a/tests/shell/testcases/nft-f/0020jump_variable_1 b/tests/shell/testcases/nft-f/0020jump_variable_1
new file mode 100755
index 0000000..f753058
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0020jump_variable_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = *
+
+table ip foo {
+ chain bar {
+ jump \$dest
+ }
+
+ chain ber {
+ }
+}"
+
+$NFT -f - <<< "$RULESET" && exit 1
+exit 0
diff --git a/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft b/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft
new file mode 100644
index 0000000..0ddaf07
--- /dev/null
+++ b/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft
@@ -0,0 +1,8 @@
+table ip foo {
+ chain bar {
+ jump ber
+ }
+
+ chain ber {
+ }
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-16 20:45 [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Fernando Fernandez Mancera
2019-05-16 20:45 ` [PATCH nft v3 2/2] jump: Allow goto and jump to a variable using nft input files Fernando Fernandez Mancera
@ 2019-05-21 9:28 ` Pablo Neira Ayuso
2019-05-21 19:38 ` Fernando Fernandez Mancera
1 sibling, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-21 9:28 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel
On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
> Now we can introduce expressions as a chain in jump and goto statements. This
> is going to be used to support variables as a chain in the following patches.
Something is wrong with json:
json.c: In function ‘verdict_expr_json’:
json.c:683:11: warning: assignment from incompatible pointer type
[-Wincompatible-pointer-types]
chain = expr->chain;
^
parser_json.c: In function ‘json_parse_verdict_expr’:
parser_json.c:1086:8: warning: passing argument 3 of
‘verdict_expr_alloc’ from incompatible pointer type
[-Wincompatible-pointer-types]
chain ? xstrdup(chain) : NULL);
^~~~~
Most likely --enable-json missing there.
diff --git a/src/datatype.c b/src/datatype.c
> index ac9f2af..10f185b 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -254,6 +254,8 @@ const struct datatype invalid_type = {
>
> static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
> {
> + char chain[NFT_CHAIN_MAXNAMELEN];
> +
> switch (expr->verdict) {
> case NFT_CONTINUE:
> nft_print(octx, "continue");
> @@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
> nft_print(octx, "break");
> break;
> case NFT_JUMP:
> - nft_print(octx, "jump %s", expr->chain);
> + if (expr->chain->etype == EXPR_VALUE) {
> + mpz_export_data(chain, expr->chain->value,
> + BYTEORDER_HOST_ENDIAN,
> + NFT_CHAIN_MAXNAMELEN);
> + nft_print(octx, "jump %s", chain);
> + } else {
> + nft_print(octx, "jump ");
> + expr_print(expr->chain, octx);
> + }
I think this should be fine:
case NFT_JUMP:
nft_print(octx, "jump ");
expr_print(expr->chain, octx);
break;
Any reason to have the 'if (expr->chain->etype == EXPR_VALUE) {'
check?
> break;
> case NFT_GOTO:
> - nft_print(octx, "goto %s", expr->chain);
> + if (expr->chain->etype == EXPR_VALUE) {
> + mpz_export_data(chain, expr->chain->value,
> + BYTEORDER_HOST_ENDIAN,
> + NFT_CHAIN_MAXNAMELEN);
> + nft_print(octx, "goto %s", chain);
> + } else {
> + nft_print(octx, "goto ");
> + expr_print(expr->chain, octx);
Same thing here.
Apart from those nitpicks, this looks good :)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-21 9:28 ` [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Pablo Neira Ayuso
@ 2019-05-21 19:38 ` Fernando Fernandez Mancera
2019-05-21 19:42 ` Pablo Neira Ayuso
2019-05-24 7:17 ` Fernando Fernandez Mancera
0 siblings, 2 replies; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2019-05-21 19:38 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Hi Pablo,
On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
> On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
>> Now we can introduce expressions as a chain in jump and goto statements. This
>> is going to be used to support variables as a chain in the following patches.
>
> Something is wrong with json:
>
> json.c: In function ‘verdict_expr_json’:
> json.c:683:11: warning: assignment from incompatible pointer type
> [-Wincompatible-pointer-types]
> chain = expr->chain;
> ^
> parser_json.c: In function ‘json_parse_verdict_expr’:
> parser_json.c:1086:8: warning: passing argument 3 of
> ‘verdict_expr_alloc’ from incompatible pointer type
> [-Wincompatible-pointer-types]
> chain ? xstrdup(chain) : NULL);
> ^~~~~
>
> Most likely --enable-json missing there.
>
Sorry, I am going to fix that.
> diff --git a/src/datatype.c b/src/datatype.c
>> index ac9f2af..10f185b 100644
>> --- a/src/datatype.c
>> +++ b/src/datatype.c
>> @@ -254,6 +254,8 @@ const struct datatype invalid_type = {
>>
>> static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
>> {
>> + char chain[NFT_CHAIN_MAXNAMELEN];
>> +
>> switch (expr->verdict) {
>> case NFT_CONTINUE:
>> nft_print(octx, "continue");
>> @@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
>> nft_print(octx, "break");
>> break;
>> case NFT_JUMP:
>> - nft_print(octx, "jump %s", expr->chain);
>> + if (expr->chain->etype == EXPR_VALUE) {
>> + mpz_export_data(chain, expr->chain->value,
>> + BYTEORDER_HOST_ENDIAN,
>> + NFT_CHAIN_MAXNAMELEN);
>> + nft_print(octx, "jump %s", chain);
>> + } else {
>> + nft_print(octx, "jump ");
>> + expr_print(expr->chain, octx);
>> + }
>
> I think this should be fine:
>
> case NFT_JUMP:
> nft_print(octx, "jump ");
> expr_print(expr->chain, octx);
> break;
>
> Any reason to have the 'if (expr->chain->etype == EXPR_VALUE) {'
> check?
>
Yes, without this check the list ruleset is slightly different when
using variables.
table ip foo {
chain bar {
type filter hook input priority filter; policy accept;
jump "ber"
}
chain ber {
counter packets 45 bytes 3132
}
}
Please, note the quote marks in the jump statement. If we don't want to
check that, we need to change all the tests that involve jumps (about 12).
Thanks!
>> break;
>> case NFT_GOTO:
>> - nft_print(octx, "goto %s", expr->chain);
>> + if (expr->chain->etype == EXPR_VALUE) {
>> + mpz_export_data(chain, expr->chain->value,
>> + BYTEORDER_HOST_ENDIAN,
>> + NFT_CHAIN_MAXNAMELEN);
>> + nft_print(octx, "goto %s", chain);
>> + } else {
>> + nft_print(octx, "goto ");
>> + expr_print(expr->chain, octx);
>
> Same thing here.
>
> Apart from those nitpicks, this looks good :)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-21 19:38 ` Fernando Fernandez Mancera
@ 2019-05-21 19:42 ` Pablo Neira Ayuso
2019-05-24 7:17 ` Fernando Fernandez Mancera
1 sibling, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-21 19:42 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel
On Tue, May 21, 2019 at 09:38:16PM +0200, Fernando Fernandez Mancera wrote:
> Hi Pablo,
>
> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
> > On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
> >> Now we can introduce expressions as a chain in jump and goto statements. This
> >> is going to be used to support variables as a chain in the following patches.
> >
> > Something is wrong with json:
> >
> > json.c: In function ‘verdict_expr_json’:
> > json.c:683:11: warning: assignment from incompatible pointer type
> > [-Wincompatible-pointer-types]
> > chain = expr->chain;
> > ^
> > parser_json.c: In function ‘json_parse_verdict_expr’:
> > parser_json.c:1086:8: warning: passing argument 3 of
> > ‘verdict_expr_alloc’ from incompatible pointer type
> > [-Wincompatible-pointer-types]
> > chain ? xstrdup(chain) : NULL);
> > ^~~~~
> >
> > Most likely --enable-json missing there.
> >
>
> Sorry, I am going to fix that.
Thanks!
> > diff --git a/src/datatype.c b/src/datatype.c
> >> index ac9f2af..10f185b 100644
> >> --- a/src/datatype.c
> >> +++ b/src/datatype.c
> >> @@ -254,6 +254,8 @@ const struct datatype invalid_type = {
> >>
> >> static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
> >> {
> >> + char chain[NFT_CHAIN_MAXNAMELEN];
> >> +
> >> switch (expr->verdict) {
> >> case NFT_CONTINUE:
> >> nft_print(octx, "continue");
> >> @@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
> >> nft_print(octx, "break");
> >> break;
> >> case NFT_JUMP:
> >> - nft_print(octx, "jump %s", expr->chain);
> >> + if (expr->chain->etype == EXPR_VALUE) {
> >> + mpz_export_data(chain, expr->chain->value,
> >> + BYTEORDER_HOST_ENDIAN,
> >> + NFT_CHAIN_MAXNAMELEN);
> >> + nft_print(octx, "jump %s", chain);
> >> + } else {
> >> + nft_print(octx, "jump ");
> >> + expr_print(expr->chain, octx);
> >> + }
> >
> > I think this should be fine:
> >
> > case NFT_JUMP:
> > nft_print(octx, "jump ");
> > expr_print(expr->chain, octx);
> > break;
> >
> > Any reason to have the 'if (expr->chain->etype == EXPR_VALUE) {'
> > check?
> >
>
> Yes, without this check the list ruleset is slightly different when
> using variables.
>
> table ip foo {
> chain bar {
> type filter hook input priority filter; policy accept;
> jump "ber"
> }
>
> chain ber {
> counter packets 45 bytes 3132
> }
> }
>
> Please, note the quote marks in the jump statement. If we don't want to
> check that, we need to change all the tests that involve jumps (about 12).
Thanks for explaining.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-21 19:38 ` Fernando Fernandez Mancera
2019-05-21 19:42 ` Pablo Neira Ayuso
@ 2019-05-24 7:17 ` Fernando Fernandez Mancera
2019-05-24 7:29 ` Fernando Fernandez Mancera
1 sibling, 1 reply; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2019-05-24 7:17 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Hi Pablo,
On 5/21/19 9:38 PM, Fernando Fernandez Mancera wrote:
> Hi Pablo,
>
> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
>> On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
>>> Now we can introduce expressions as a chain in jump and goto statements. This
>>> is going to be used to support variables as a chain in the following patches.
>>
>> Something is wrong with json:
>>
>> json.c: In function ‘verdict_expr_json’:
>> json.c:683:11: warning: assignment from incompatible pointer type
>> [-Wincompatible-pointer-types]
>> chain = expr->chain;
>> ^
>> parser_json.c: In function ‘json_parse_verdict_expr’:
>> parser_json.c:1086:8: warning: passing argument 3 of
>> ‘verdict_expr_alloc’ from incompatible pointer type
>> [-Wincompatible-pointer-types]
>> chain ? xstrdup(chain) : NULL);
>> ^~~~~
>>
>> Most likely --enable-json missing there.
>>
>
> Sorry, I am going to fix that.
> [...]
I am compiling nftables with:
$ ./configure --enable-json
$ make
And I am not getting any error, am I missing something? Thanks! :-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-24 7:17 ` Fernando Fernandez Mancera
@ 2019-05-24 7:29 ` Fernando Fernandez Mancera
2019-05-24 9:21 ` Pablo Neira Ayuso
0 siblings, 1 reply; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2019-05-24 7:29 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 5/24/19 9:17 AM, Fernando Fernandez Mancera wrote:
> Hi Pablo,
>
> On 5/21/19 9:38 PM, Fernando Fernandez Mancera wrote:
>> Hi Pablo,
>>
>> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
>>> On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
>>>> Now we can introduce expressions as a chain in jump and goto statements. This
>>>> is going to be used to support variables as a chain in the following patches.
>>>
>>> Something is wrong with json:
>>>
>>> json.c: In function ‘verdict_expr_json’:
>>> json.c:683:11: warning: assignment from incompatible pointer type
>>> [-Wincompatible-pointer-types]
>>> chain = expr->chain;
>>> ^
>>> parser_json.c: In function ‘json_parse_verdict_expr’:
>>> parser_json.c:1086:8: warning: passing argument 3 of
>>> ‘verdict_expr_alloc’ from incompatible pointer type
>>> [-Wincompatible-pointer-types]
>>> chain ? xstrdup(chain) : NULL);
>>> ^~~~~
>>>
>>> Most likely --enable-json missing there.
>>>
>>
>> Sorry, I am going to fix that.
>> [...]
>
> I am compiling nftables with:
>
> $ ./configure --enable-json
> $ make
>
> And I am not getting any error, am I missing something? Thanks! :-)
>
Fixed, the option is --with-json. Why isn't it "--enable-json" as other
features?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-24 7:29 ` Fernando Fernandez Mancera
@ 2019-05-24 9:21 ` Pablo Neira Ayuso
2019-05-24 10:29 ` Phil Sutter
0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-24 9:21 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel, phil
On Fri, May 24, 2019 at 09:29:34AM +0200, Fernando Fernandez Mancera wrote:
> On 5/24/19 9:17 AM, Fernando Fernandez Mancera wrote:
> > Hi Pablo,
> >
> > On 5/21/19 9:38 PM, Fernando Fernandez Mancera wrote:
> >> Hi Pablo,
> >>
> >> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
> >>> On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
> >>>> Now we can introduce expressions as a chain in jump and goto statements. This
> >>>> is going to be used to support variables as a chain in the following patches.
> >>>
> >>> Something is wrong with json:
> >>>
> >>> json.c: In function ‘verdict_expr_json’:
> >>> json.c:683:11: warning: assignment from incompatible pointer type
> >>> [-Wincompatible-pointer-types]
> >>> chain = expr->chain;
> >>> ^
> >>> parser_json.c: In function ‘json_parse_verdict_expr’:
> >>> parser_json.c:1086:8: warning: passing argument 3 of
> >>> ‘verdict_expr_alloc’ from incompatible pointer type
> >>> [-Wincompatible-pointer-types]
> >>> chain ? xstrdup(chain) : NULL);
> >>> ^~~~~
> >>>
> >>> Most likely --enable-json missing there.
> >>>
> >>
> >> Sorry, I am going to fix that.
> >> [...]
> >
> > I am compiling nftables with:
> >
> > $ ./configure --enable-json
> > $ make
> >
> > And I am not getting any error, am I missing something? Thanks! :-)
> >
>
> Fixed, the option is --with-json. Why isn't it "--enable-json" as other
> features?
Cc'ing Phil.
We can just update this to accept both, either --with-json or
--enable-json.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements
2019-05-24 9:21 ` Pablo Neira Ayuso
@ 2019-05-24 10:29 ` Phil Sutter
0 siblings, 0 replies; 9+ messages in thread
From: Phil Sutter @ 2019-05-24 10:29 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Fernando Fernandez Mancera, netfilter-devel
Hi,
On Fri, May 24, 2019 at 11:21:48AM +0200, Pablo Neira Ayuso wrote:
> On Fri, May 24, 2019 at 09:29:34AM +0200, Fernando Fernandez Mancera wrote:
> > On 5/24/19 9:17 AM, Fernando Fernandez Mancera wrote:
> > > Hi Pablo,
> > >
> > > On 5/21/19 9:38 PM, Fernando Fernandez Mancera wrote:
> > >> Hi Pablo,
> > >>
> > >> On 5/21/19 11:28 AM, Pablo Neira Ayuso wrote:
> > >>> On Thu, May 16, 2019 at 10:45:58PM +0200, Fernando Fernandez Mancera wrote:
> > >>>> Now we can introduce expressions as a chain in jump and goto statements. This
> > >>>> is going to be used to support variables as a chain in the following patches.
> > >>>
> > >>> Something is wrong with json:
> > >>>
> > >>> json.c: In function ‘verdict_expr_json’:
> > >>> json.c:683:11: warning: assignment from incompatible pointer type
> > >>> [-Wincompatible-pointer-types]
> > >>> chain = expr->chain;
> > >>> ^
> > >>> parser_json.c: In function ‘json_parse_verdict_expr’:
> > >>> parser_json.c:1086:8: warning: passing argument 3 of
> > >>> ‘verdict_expr_alloc’ from incompatible pointer type
> > >>> [-Wincompatible-pointer-types]
> > >>> chain ? xstrdup(chain) : NULL);
> > >>> ^~~~~
> > >>>
> > >>> Most likely --enable-json missing there.
> > >>>
> > >>
> > >> Sorry, I am going to fix that.
> > >> [...]
> > >
> > > I am compiling nftables with:
> > >
> > > $ ./configure --enable-json
> > > $ make
> > >
> > > And I am not getting any error, am I missing something? Thanks! :-)
> > >
> >
> > Fixed, the option is --with-json. Why isn't it "--enable-json" as other
> > features?
It is actually not that uniform. While we have:
--enable-debug
--enable-man-doc
--enable-python
we also have:
--with-mini-gmp
--with-cli
--with-xtables
--with-json
and all of them just enable/disable something, unlike --with-python-bin.
> We can just update this to accept both, either --with-json or
> --enable-json.
For consistency, we should turn all of the above --with flags into
--enable ones, but there's of course the compatibility problem.
What do you think, is it feasible to change all the above and introduce
--with aliases for them?
Cheers, Phil
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-24 10:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 20:45 [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Fernando Fernandez Mancera
2019-05-16 20:45 ` [PATCH nft v3 2/2] jump: Allow goto and jump to a variable using nft input files Fernando Fernandez Mancera
2019-05-21 9:28 ` [PATCH nft v3 1/2] jump: Introduce chain_expr in jump and goto statements Pablo Neira Ayuso
2019-05-21 19:38 ` Fernando Fernandez Mancera
2019-05-21 19:42 ` Pablo Neira Ayuso
2019-05-24 7:17 ` Fernando Fernandez Mancera
2019-05-24 7:29 ` Fernando Fernandez Mancera
2019-05-24 9:21 ` Pablo Neira Ayuso
2019-05-24 10:29 ` Phil Sutter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).