| Message ID | 20240322154855.13857-3-phil@nwl.cc |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for table's persist flag | expand |
On 2024-03-22 16:48, Phil Sutter wrote: > Bison parser lacked support for passing multiple flags, JSON parser > did not support table flags at all. > > Document also 'owner' flag (and describe their relationship in nft.8. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > doc/libnftables-json.adoc | 11 +++- > doc/nft.txt | 9 +++ > include/rule.h | 3 +- > src/parser_bison.y | 42 +++++++++---- > src/parser_json.c | 68 ++++++++++++++++++++- > src/rule.c | 1 + > tests/shell/features/table_flag_persist.nft | 3 + > tests/shell/testcases/owner/0002-persist | 36 +++++++++++ > 8 files changed, 156 insertions(+), 17 deletions(-) > create mode 100644 tests/shell/features/table_flag_persist.nft > create mode 100755 tests/shell/testcases/owner/0002-persist > > diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc > index 3948a0bad47c1..a4adcde2a66a9 100644 > --- a/doc/libnftables-json.adoc > +++ b/doc/libnftables-json.adoc > @@ -202,12 +202,19 @@ Rename a chain. The new name is expected in a dedicated property named > > === TABLE > [verse] > +____ > *{ "table": { > "family":* 'STRING'*, > "name":* 'STRING'*, > - "handle":* 'NUMBER' > + "handle":* 'NUMBER'*, > + "flags":* 'TABLE_FLAGS' > *}}* > > +'TABLE_FLAGS' := 'TABLE_FLAG' | *[* 'TABLE_FLAG_LIST' *]* > +'TABLE_FLAG_LIST' := 'TABLE_FLAG' [*,* 'TABLE_FLAG_LIST' ] > +'TABLE_FLAG' := *"dormant"* | *"owner"* | *"persist"* > +____ > + > This object describes a table. > > *family*:: > @@ -217,6 +224,8 @@ This object describes a table. > *handle*:: > The table's handle. In input, it is used only in *delete* command as > alternative to *name*. > +*flags*:: > + The table's flags. > > === CHAIN > [verse] > diff --git a/doc/nft.txt b/doc/nft.txt > index 248b29af369ad..2080c07350f6d 100644 > --- a/doc/nft.txt > +++ b/doc/nft.txt > @@ -343,8 +343,17 @@ return an error. > |Flag | Description > |dormant | > table is not evaluated any more (base chains are unregistered). > +|owner | > +table is owned by the creating process. > +|persist | > +table shall outlive the owning process. > |================= > > +Creating a table with flag *owner* excludes other processes from manipulating > +it or its contents. By default, it will be removed when the process exits. > +Setting flag *persist* will prevent this and the resulting orphaned table will > +accept a new owner, e.g. a restarting daemon maintaining the table. > + > .*Add, change, delete a table* > --------------------------------------- > # start nft in interactive mode > diff --git a/include/rule.h b/include/rule.h > index 3a833cf3a4588..2f8292ee9dc32 100644 > --- a/include/rule.h > +++ b/include/rule.h > @@ -130,8 +130,9 @@ struct symbol *symbol_get(const struct scope *scope, const char *identifier); > enum table_flags { > TABLE_F_DORMANT = (1 << 0), > TABLE_F_OWNER = (1 << 1), > + TABLE_F_PERSIST = (1 << 2), > }; > -#define TABLE_FLAGS_MAX 2 > +#define TABLE_FLAGS_MAX 3 > > const char *table_flag_name(uint32_t flag); > > diff --git a/src/parser_bison.y b/src/parser_bison.y > index bdb73911759c8..1ade7417f8d6a 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -742,6 +742,8 @@ int nft_lex(void *, void *, void *); > %type <rule> rule rule_alloc > %destructor { rule_free($$); } rule > > +%type <val> table_flags table_flag > + > %type <val> set_flag_list set_flag > > %type <val> set_policy_spec > @@ -1905,20 +1907,9 @@ table_block_alloc : /* empty */ > } > ; > > -table_options : FLAGS STRING > +table_options : FLAGS table_flags > { > - if (strcmp($2, "dormant") == 0) { > - $<table>0->flags |= TABLE_F_DORMANT; > - free_const($2); > - } else if (strcmp($2, "owner") == 0) { > - $<table>0->flags |= TABLE_F_OWNER; > - free_const($2); > - } else { > - erec_queue(error(&@2, "unknown table option %s", $2), > - state->msgs); > - free_const($2); > - YYERROR; > - } > + $<table>0->flags |= $2; > } > | comment_spec > { > @@ -1930,6 +1921,31 @@ table_options : FLAGS STRING > } > ; > > +table_flags : table_flag > + | table_flags COMMA table_flag > + { > + $$ = $1 | $3; > + } > + ; > +table_flag : STRING > + { > + if (strcmp($1, "dormant") == 0) { > + $$ = TABLE_F_DORMANT; > + free_const($1); > + } else if (strcmp($1, "owner") == 0) { > + $$ = TABLE_F_OWNER; Don't you need to free_const($1) here too? > + } else if (strcmp($1, "persist") == 0) { > + $$ = TABLE_F_PERSIST; > + free_const($1); > + } else { > + erec_queue(error(&@1, "unknown table option %s", $1), > + state->msgs); > + free_const($1); > + YYERROR; > + } > + } > + ; > + > table_block : /* empty */ { $$ = $<table>-1; } > | table_block common_block > | table_block stmt_separator > diff --git a/src/parser_json.c b/src/parser_json.c > index 4fc0479cf4972..04255688ca04c 100644 > --- a/src/parser_json.c > +++ b/src/parser_json.c > @@ -2954,6 +2954,64 @@ static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root) > return NULL; > } > > +static int string_to_table_flag(const char *str) > +{ > + const struct { > + enum table_flags val; > + const char *name; > + } flag_tbl[] = { > + { TABLE_F_DORMANT, "dormant" }, > + { TABLE_F_OWNER, "owner" }, > + { TABLE_F_PERSIST, "persist" }, > + }; > + unsigned int i; > + > + for (i = 0; i < array_size(flag_tbl); i++) { > + if (!strcmp(str, flag_tbl[i].name)) > + return flag_tbl[i].val; > + } > + return 0; > +} > + > +static int json_parse_table_flags(struct json_ctx *ctx, json_t *root, > + enum table_flags *flags) > +{ > + json_t *tmp, *tmp2; > + size_t index; > + int flag; > + > + if (json_unpack(root, "{s:o}", "flags", &tmp)) > + return 0; > + > + if (json_is_string(tmp)) { > + flag = string_to_table_flag(json_string_value(tmp)); > + if (flag) { > + *flags = flag; > + return 0; > + } > + json_error(ctx, "Invalid table flag '%s'.", > + json_string_value(tmp)); > + return 1; > + } > + if (!json_is_array(tmp)) { > + json_error(ctx, "Unexpected table flags value."); > + return 1; > + } > + json_array_foreach(tmp, index, tmp2) { > + if (json_is_string(tmp2)) { > + flag = string_to_table_flag(json_string_value(tmp2)); > + > + if (flag) { > + *flags |= flag; > + continue; > + } > + } > + json_error(ctx, "Invalid table flag at index %zu.", index); > + return 1; > + } > + return 0; > +} > + > static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, > enum cmd_ops op, enum cmd_obj obj) > { > @@ -2962,6 +3020,7 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, > .table.location = *int_loc, > }; > struct table *table = NULL; > + enum table_flags flags = 0; > > if (json_unpack_err(ctx, root, "{s:s}", > "family", &family)) > @@ -2972,6 +3031,9 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, > return NULL; > > json_unpack(root, "{s:s}", "comment", &comment); > + if (json_parse_table_flags(ctx, root, &flags)) > + return NULL; > + > } else if (op == CMD_DELETE && > json_unpack(root, "{s:s}", "name", &h.table.name) && > json_unpack(root, "{s:I}", "handle", &h.handle.id)) { > @@ -2985,10 +3047,12 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, > if (h.table.name) > h.table.name = xstrdup(h.table.name); > > - if (comment) { > + if (comment || flags) { > table = table_alloc(); > handle_merge(&table->handle, &h); > - table->comment = xstrdup(comment); > + if (comment) > + table->comment = xstrdup(comment); > + table->flags = flags; > } > > if (op == CMD_ADD) > diff --git a/src/rule.c b/src/rule.c > index 45289cc01dce8..6e56a129c81d1 100644 > --- a/src/rule.c > +++ b/src/rule.c > @@ -1215,6 +1215,7 @@ struct table *table_lookup_fuzzy(const struct handle *h, > static const char *table_flags_name[TABLE_FLAGS_MAX] = { > "dormant", > "owner", > + "persist", > }; > > const char *table_flag_name(uint32_t flag) > diff --git a/tests/shell/features/table_flag_persist.nft b/tests/shell/features/table_flag_persist.nft > new file mode 100644 > index 0000000000000..0da3e6d4f03ff > --- /dev/null > +++ b/tests/shell/features/table_flag_persist.nft > @@ -0,0 +1,3 @@ > +table t { > + flags persist; > +} > diff --git a/tests/shell/testcases/owner/0002-persist b/tests/shell/testcases/owner/0002-persist > new file mode 100755 > index 0000000000000..cf4b8f1327ec1 > --- /dev/null > +++ b/tests/shell/testcases/owner/0002-persist > @@ -0,0 +1,36 @@ > +#!/bin/bash > + > +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_table_flag_owner) > +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_table_flag_persist) > + > +die() { > + echo "$@" > + exit 1 > +} > + > +$NFT -f - <<EOF > +table ip t { > + flags owner, persist > +} > +EOF > +[[ $? -eq 0 ]] || { > + die "table add failed" > +} > + > +$NFT list ruleset | grep -q 'table ip t' || { > + die "table does not persist" > +} > +$NFT list ruleset | grep -q 'flags persist$' || { > + die "unexpected flags in orphaned table" > +} > + > +$NFT -f - <<EOF > +table ip t { > + flags owner, persist > +} > +EOF > +[[ $? -eq 0 ]] || { > + die "retake ownership failed" > +} > + > +exit 0
On Mon, Mar 25, 2024 at 02:33:48PM +0100, Quentin Deslandes wrote: > On 2024-03-22 16:48, Phil Sutter wrote: [...] > > diff --git a/src/parser_bison.y b/src/parser_bison.y > > index bdb73911759c8..1ade7417f8d6a 100644 > > --- a/src/parser_bison.y > > +++ b/src/parser_bison.y [...] > > @@ -1930,6 +1921,31 @@ table_options : FLAGS STRING > > } > > ; > > > > +table_flags : table_flag > > + | table_flags COMMA table_flag > > + { > > + $$ = $1 | $3; > > + } > > + ; > > +table_flag : STRING > > + { > > + if (strcmp($1, "dormant") == 0) { > > + $$ = TABLE_F_DORMANT; > > + free_const($1); > > + } else if (strcmp($1, "owner") == 0) { > > + $$ = TABLE_F_OWNER; > > Don't you need to free_const($1) here too? Oh yes, thanks for spotting it! Guess I messed up some c'n'p here, the code is really obvious. Thanks, Phil
diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc index 3948a0bad47c1..a4adcde2a66a9 100644 --- a/doc/libnftables-json.adoc +++ b/doc/libnftables-json.adoc @@ -202,12 +202,19 @@ Rename a chain. The new name is expected in a dedicated property named === TABLE [verse] +____ *{ "table": { "family":* 'STRING'*, "name":* 'STRING'*, - "handle":* 'NUMBER' + "handle":* 'NUMBER'*, + "flags":* 'TABLE_FLAGS' *}}* +'TABLE_FLAGS' := 'TABLE_FLAG' | *[* 'TABLE_FLAG_LIST' *]* +'TABLE_FLAG_LIST' := 'TABLE_FLAG' [*,* 'TABLE_FLAG_LIST' ] +'TABLE_FLAG' := *"dormant"* | *"owner"* | *"persist"* +____ + This object describes a table. *family*:: @@ -217,6 +224,8 @@ This object describes a table. *handle*:: The table's handle. In input, it is used only in *delete* command as alternative to *name*. +*flags*:: + The table's flags. === CHAIN [verse] diff --git a/doc/nft.txt b/doc/nft.txt index 248b29af369ad..2080c07350f6d 100644 --- a/doc/nft.txt +++ b/doc/nft.txt @@ -343,8 +343,17 @@ return an error. |Flag | Description |dormant | table is not evaluated any more (base chains are unregistered). +|owner | +table is owned by the creating process. +|persist | +table shall outlive the owning process. |================= +Creating a table with flag *owner* excludes other processes from manipulating +it or its contents. By default, it will be removed when the process exits. +Setting flag *persist* will prevent this and the resulting orphaned table will +accept a new owner, e.g. a restarting daemon maintaining the table. + .*Add, change, delete a table* --------------------------------------- # start nft in interactive mode diff --git a/include/rule.h b/include/rule.h index 3a833cf3a4588..2f8292ee9dc32 100644 --- a/include/rule.h +++ b/include/rule.h @@ -130,8 +130,9 @@ struct symbol *symbol_get(const struct scope *scope, const char *identifier); enum table_flags { TABLE_F_DORMANT = (1 << 0), TABLE_F_OWNER = (1 << 1), + TABLE_F_PERSIST = (1 << 2), }; -#define TABLE_FLAGS_MAX 2 +#define TABLE_FLAGS_MAX 3 const char *table_flag_name(uint32_t flag); diff --git a/src/parser_bison.y b/src/parser_bison.y index bdb73911759c8..1ade7417f8d6a 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -742,6 +742,8 @@ int nft_lex(void *, void *, void *); %type <rule> rule rule_alloc %destructor { rule_free($$); } rule +%type <val> table_flags table_flag + %type <val> set_flag_list set_flag %type <val> set_policy_spec @@ -1905,20 +1907,9 @@ table_block_alloc : /* empty */ } ; -table_options : FLAGS STRING +table_options : FLAGS table_flags { - if (strcmp($2, "dormant") == 0) { - $<table>0->flags |= TABLE_F_DORMANT; - free_const($2); - } else if (strcmp($2, "owner") == 0) { - $<table>0->flags |= TABLE_F_OWNER; - free_const($2); - } else { - erec_queue(error(&@2, "unknown table option %s", $2), - state->msgs); - free_const($2); - YYERROR; - } + $<table>0->flags |= $2; } | comment_spec { @@ -1930,6 +1921,31 @@ table_options : FLAGS STRING } ; +table_flags : table_flag + | table_flags COMMA table_flag + { + $$ = $1 | $3; + } + ; +table_flag : STRING + { + if (strcmp($1, "dormant") == 0) { + $$ = TABLE_F_DORMANT; + free_const($1); + } else if (strcmp($1, "owner") == 0) { + $$ = TABLE_F_OWNER; + } else if (strcmp($1, "persist") == 0) { + $$ = TABLE_F_PERSIST; + free_const($1); + } else { + erec_queue(error(&@1, "unknown table option %s", $1), + state->msgs); + free_const($1); + YYERROR; + } + } + ; + table_block : /* empty */ { $$ = $<table>-1; } | table_block common_block | table_block stmt_separator diff --git a/src/parser_json.c b/src/parser_json.c index 4fc0479cf4972..04255688ca04c 100644 --- a/src/parser_json.c +++ b/src/parser_json.c @@ -2954,6 +2954,64 @@ static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root) return NULL; } +static int string_to_table_flag(const char *str) +{ + const struct { + enum table_flags val; + const char *name; + } flag_tbl[] = { + { TABLE_F_DORMANT, "dormant" }, + { TABLE_F_OWNER, "owner" }, + { TABLE_F_PERSIST, "persist" }, + }; + unsigned int i; + + for (i = 0; i < array_size(flag_tbl); i++) { + if (!strcmp(str, flag_tbl[i].name)) + return flag_tbl[i].val; + } + return 0; +} + +static int json_parse_table_flags(struct json_ctx *ctx, json_t *root, + enum table_flags *flags) +{ + json_t *tmp, *tmp2; + size_t index; + int flag; + + if (json_unpack(root, "{s:o}", "flags", &tmp)) + return 0; + + if (json_is_string(tmp)) { + flag = string_to_table_flag(json_string_value(tmp)); + if (flag) { + *flags = flag; + return 0; + } + json_error(ctx, "Invalid table flag '%s'.", + json_string_value(tmp)); + return 1; + } + if (!json_is_array(tmp)) { + json_error(ctx, "Unexpected table flags value."); + return 1; + } + json_array_foreach(tmp, index, tmp2) { + if (json_is_string(tmp2)) { + flag = string_to_table_flag(json_string_value(tmp2)); + + if (flag) { + *flags |= flag; + continue; + } + } + json_error(ctx, "Invalid table flag at index %zu.", index); + return 1; + } + return 0; +} + static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, enum cmd_ops op, enum cmd_obj obj) { @@ -2962,6 +3020,7 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, .table.location = *int_loc, }; struct table *table = NULL; + enum table_flags flags = 0; if (json_unpack_err(ctx, root, "{s:s}", "family", &family)) @@ -2972,6 +3031,9 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, return NULL; json_unpack(root, "{s:s}", "comment", &comment); + if (json_parse_table_flags(ctx, root, &flags)) + return NULL; + } else if (op == CMD_DELETE && json_unpack(root, "{s:s}", "name", &h.table.name) && json_unpack(root, "{s:I}", "handle", &h.handle.id)) { @@ -2985,10 +3047,12 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root, if (h.table.name) h.table.name = xstrdup(h.table.name); - if (comment) { + if (comment || flags) { table = table_alloc(); handle_merge(&table->handle, &h); - table->comment = xstrdup(comment); + if (comment) + table->comment = xstrdup(comment); + table->flags = flags; } if (op == CMD_ADD) diff --git a/src/rule.c b/src/rule.c index 45289cc01dce8..6e56a129c81d1 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1215,6 +1215,7 @@ struct table *table_lookup_fuzzy(const struct handle *h, static const char *table_flags_name[TABLE_FLAGS_MAX] = { "dormant", "owner", + "persist", }; const char *table_flag_name(uint32_t flag) diff --git a/tests/shell/features/table_flag_persist.nft b/tests/shell/features/table_flag_persist.nft new file mode 100644 index 0000000000000..0da3e6d4f03ff --- /dev/null +++ b/tests/shell/features/table_flag_persist.nft @@ -0,0 +1,3 @@ +table t { + flags persist; +} diff --git a/tests/shell/testcases/owner/0002-persist b/tests/shell/testcases/owner/0002-persist new file mode 100755 index 0000000000000..cf4b8f1327ec1 --- /dev/null +++ b/tests/shell/testcases/owner/0002-persist @@ -0,0 +1,36 @@ +#!/bin/bash + +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_table_flag_owner) +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_table_flag_persist) + +die() { + echo "$@" + exit 1 +} + +$NFT -f - <<EOF +table ip t { + flags owner, persist +} +EOF +[[ $? -eq 0 ]] || { + die "table add failed" +} + +$NFT list ruleset | grep -q 'table ip t' || { + die "table does not persist" +} +$NFT list ruleset | grep -q 'flags persist$' || { + die "unexpected flags in orphaned table" +} + +$NFT -f - <<EOF +table ip t { + flags owner, persist +} +EOF +[[ $? -eq 0 ]] || { + die "retake ownership failed" +} + +exit 0
Bison parser lacked support for passing multiple flags, JSON parser did not support table flags at all. Document also 'owner' flag (and describe their relationship in nft.8. Signed-off-by: Phil Sutter <phil@nwl.cc> --- doc/libnftables-json.adoc | 11 +++- doc/nft.txt | 9 +++ include/rule.h | 3 +- src/parser_bison.y | 42 +++++++++---- src/parser_json.c | 68 ++++++++++++++++++++- src/rule.c | 1 + tests/shell/features/table_flag_persist.nft | 3 + tests/shell/testcases/owner/0002-persist | 36 +++++++++++ 8 files changed, 156 insertions(+), 17 deletions(-) create mode 100644 tests/shell/features/table_flag_persist.nft create mode 100755 tests/shell/testcases/owner/0002-persist