Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets") Signed-off-by: Phil Sutter --- tools/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/.gitignore diff --git a/tools/.gitignore b/tools/.gitignore new file mode 100644 index 0000000000000..2d06c49835b15 --- /dev/null +++ b/tools/.gitignore @@ -0,0 +1 @@ +nftables.service -- 2.51.0 Fixed commit missed the fact that there are two routines printing chain declarations. Fixes: eb30f236d91a8 ("rule: print chain and flowtable devices in quotes") Signed-off-by: Phil Sutter --- src/rule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rule.c b/src/rule.c index ceb56488d37f6..d0a62a3ee002d 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1131,7 +1131,7 @@ void chain_print_plain(const struct chain *chain, struct output_ctx *octx) nft_print(octx, "devices = { "); for (i = 0; i < chain->dev_array_len; i++) { - nft_print(octx, "%s", chain->dev_array[i]); + nft_print(octx, "\"%s\"", chain->dev_array[i]); if (i + 1 != chain->dev_array_len) nft_print(octx, ", "); } -- 2.51.0 Complete commit a66b5ad9540dd ("src: allow for updating devices on existing netdev chain") in supporting inet family ingress hook chains as well. The kernel does already but nft has to add a proper hooknum attribute to pass the checks. The hook.num field has to be initialized from hook.name using str2hooknum(), which is part of chain evaluation. Calling chain_evaluate() just for that purpose is a bit over the top, but the hook name lookup may fail and performing chain evaluation for delete command as well fits more into the code layout than duplicating parts of it in mnl_nft_chain_del() or elsewhere. Just avoid the chain_cache_find() call as its assert() triggers when deleting by handle and also don't add to be deleted chains to cache. Signed-off-by: Phil Sutter --- src/evaluate.c | 6 ++++-- src/mnl.c | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/evaluate.c b/src/evaluate.c index b7e4f71fdfbc9..db4ac18f1dc9f 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -5758,7 +5758,9 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain) return table_not_found(ctx); if (chain == NULL) { - if (!chain_cache_find(table, ctx->cmd->handle.chain.name)) { + if (ctx->cmd->op != CMD_DELETE && + ctx->cmd->op != CMD_DESTROY && + !chain_cache_find(table, ctx->cmd->handle.chain.name)) { chain = chain_alloc(); handle_merge(&chain->handle, &ctx->cmd->handle); chain_cache_add(chain, table); @@ -6070,7 +6072,7 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd) return 0; case CMD_OBJ_CHAIN: chain_del_cache(ctx, cmd); - return 0; + return chain_evaluate(ctx, cmd->chain); case CMD_OBJ_TABLE: table_del_cache(ctx, cmd); return 0; diff --git a/src/mnl.c b/src/mnl.c index 984dcac27b1cf..d1402c0fcb9f4 100644 --- a/src/mnl.c +++ b/src/mnl.c @@ -994,6 +994,8 @@ int mnl_nft_chain_del(struct netlink_ctx *ctx, struct cmd *cmd) struct nlattr *nest; nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK); + mnl_attr_put_u32(nlh, NFTA_HOOK_HOOKNUM, + htonl(cmd->chain->hook.num)); mnl_nft_chain_devs_build(nlh, cmd); mnl_attr_nest_end(nlh, nest); } -- 2.51.0 Since kernel commit a1050dd07168 ("netfilter: nf_tables: Reintroduce shortened deletion notifications"), type-specific data is no longer dumped when notifying for a deleted object. JSON output was not aware of this and tried to print bogus data. Signed-off-by: Phil Sutter --- include/json.h | 5 +++-- src/json.c | 18 ++++++++++++------ src/monitor.c | 2 +- tests/monitor/testcases/object.t | 10 +++++----- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/include/json.h b/include/json.h index 42e1c8616c975..3b8d045f87bbc 100644 --- a/include/json.h +++ b/include/json.h @@ -113,7 +113,7 @@ void monitor_print_set_json(struct netlink_mon_handler *monh, void monitor_print_element_json(struct netlink_mon_handler *monh, const char *cmd, struct set *s); void monitor_print_obj_json(struct netlink_mon_handler *monh, - const char *cmd, struct obj *o); + const char *cmd, struct obj *o, bool delete); void monitor_print_flowtable_json(struct netlink_mon_handler *monh, const char *cmd, struct flowtable *ft); void monitor_print_rule_json(struct netlink_mon_handler *monh, @@ -252,7 +252,8 @@ static inline void monitor_print_element_json(struct netlink_mon_handler *monh, } static inline void monitor_print_obj_json(struct netlink_mon_handler *monh, - const char *cmd, struct obj *o) + const char *cmd, struct obj *o, + bool delete) { /* empty */ } diff --git a/src/json.c b/src/json.c index d06fd04027140..0afce5415f541 100644 --- a/src/json.c +++ b/src/json.c @@ -397,7 +397,8 @@ static json_t *tunnel_erspan_print_json(const struct obj *obj) return tunnel; } -static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj) +static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj, + bool delete) { const char *rate_unit = NULL, *burst_unit = NULL; const char *type = obj_type_name(obj->type); @@ -410,6 +411,9 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj) "table", obj->handle.table.name, "handle", obj->handle.handle.id); + if (delete) + goto out; + if (obj->comment) { tmp = nft_json_pack("{s:s}", "comment", obj->comment); json_object_update(root, tmp); @@ -570,6 +574,7 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj) break; } +out: return nft_json_pack("{s:o}", type, root); } @@ -1815,7 +1820,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx, json_array_append_new(root, tmp); } list_for_each_entry(obj, &table->obj_cache.list, cache.list) { - tmp = obj_print_json(&ctx->nft->output, obj); + tmp = obj_print_json(&ctx->nft->output, obj, false); json_array_append_new(root, tmp); } list_for_each_entry(set, &table->set_cache.list, cache.list) { @@ -1971,7 +1976,7 @@ static json_t *do_list_sets_json(struct netlink_ctx *ctx, struct cmd *cmd) static json_t *do_list_obj_json(struct netlink_ctx *ctx, struct cmd *cmd, uint32_t type) { - json_t *root = json_array(); + json_t *root = json_array(), *tmp; struct table *table; struct obj *obj; @@ -1990,7 +1995,8 @@ static json_t *do_list_obj_json(struct netlink_ctx *ctx, strcmp(cmd->handle.obj.name, obj->handle.obj.name))) continue; - json_array_append_new(root, obj_print_json(&ctx->nft->output, obj)); + tmp = obj_print_json(&ctx->nft->output, obj, false); + json_array_append_new(root, tmp); } } @@ -2207,11 +2213,11 @@ void monitor_print_element_json(struct netlink_mon_handler *monh, } void monitor_print_obj_json(struct netlink_mon_handler *monh, - const char *cmd, struct obj *o) + const char *cmd, struct obj *o, bool delete) { struct output_ctx *octx = &monh->ctx->nft->output; - monitor_print_json(monh, cmd, obj_print_json(octx, o)); + monitor_print_json(monh, cmd, obj_print_json(octx, o, delete)); } void monitor_print_flowtable_json(struct netlink_mon_handler *monh, diff --git a/src/monitor.c b/src/monitor.c index e58f62252ca2d..fafeeebe914b8 100644 --- a/src/monitor.c +++ b/src/monitor.c @@ -549,7 +549,7 @@ static int netlink_events_obj_cb(const struct nlmsghdr *nlh, int type, nft_mon_print(monh, "\n"); break; case NFTNL_OUTPUT_JSON: - monitor_print_obj_json(monh, cmd, obj); + monitor_print_obj_json(monh, cmd, obj, type == NFT_MSG_DELOBJ); if (!nft_output_echo(&monh->ctx->nft->output)) nft_mon_print(monh, "\n"); break; diff --git a/tests/monitor/testcases/object.t b/tests/monitor/testcases/object.t index 53a9f8c59836b..b60dc9899ab2a 100644 --- a/tests/monitor/testcases/object.t +++ b/tests/monitor/testcases/object.t @@ -9,7 +9,7 @@ J {"add": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0, " I delete counter ip t c O - -J {"delete": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0, "packets": 0, "bytes": 0}}} +J {"delete": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0}}} # FIXME: input/output shouldn't be asynchronous here I add quota ip t q 25 mbytes @@ -18,7 +18,7 @@ J {"add": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0, "by I delete quota ip t q O - -J {"delete": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0, "bytes": 26214400, "used": 0, "inv": false}}} +J {"delete": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0}}} # FIXME: input/output shouldn't be asynchronous here I add limit ip t l rate 1/second @@ -27,7 +27,7 @@ J {"add": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0, "ra I delete limit ip t l O - -J {"delete": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0, "rate": 1, "per": "second", "burst": 5}}} +J {"delete": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0}}} I add ct helper ip t cth { type "sip" protocol tcp; l3proto ip; } O - @@ -35,7 +35,7 @@ J {"add": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle": I delete ct helper ip t cth O - -J {"delete": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle": 0, "type": "sip", "protocol": "tcp", "l3proto": "ip"}}} +J {"delete": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle": 0}}} I add ct timeout ip t ctt { protocol udp; l3proto ip; policy = { unreplied : 15s, replied : 12s }; } O - @@ -43,4 +43,4 @@ J {"add": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle": I delete ct timeout ip t ctt O - -J {"delete": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle": 0, "protocol": "udp", "l3proto": "ip", "policy": {"unreplied": 15, "replied": 12}}}} +J {"delete": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle": 0}}} -- 2.51.0 Try to cover for reduced table and chain deletion notifications by creating them with data which is omitted by the kernel during deletion. Also try to expose the difference in reported flowtable hook deletion vs. flowtable deletion. Signed-off-by: Phil Sutter --- tests/monitor/testcases/chain.t | 41 ++++++++++++++++++++++ tests/monitor/testcases/flowtable-simple.t | 12 +++++++ tests/monitor/testcases/table.t | 15 ++++++++ 3 files changed, 68 insertions(+) create mode 100644 tests/monitor/testcases/chain.t create mode 100644 tests/monitor/testcases/table.t diff --git a/tests/monitor/testcases/chain.t b/tests/monitor/testcases/chain.t new file mode 100644 index 0000000000000..975ccf1d33919 --- /dev/null +++ b/tests/monitor/testcases/chain.t @@ -0,0 +1,41 @@ +I add table inet t +O - +J {"add": {"table": {"family": "inet", "name": "t", "handle": 0}}} + +I add chain inet t c +O - +J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}} + +I delete chain inet t c +O - +J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}} + +I add chain inet t c { type filter hook input priority filter; } +O add chain inet t c { type filter hook input priority 0; policy accept; } +J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}} + +I delete chain inet t c +O - +J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}} + +I add chain inet t c { type filter hook ingress priority filter; devices = { "lo" }; } +O add chain inet t c { type filter hook ingress devices = { "lo" } priority 0; policy accept; } +J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": "lo", "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}} + +I delete chain inet t c +O - +J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}} + +I add chain inet t c { type filter hook ingress priority filter; devices = { "eth1", "lo" }; } +O add chain inet t c { type filter hook ingress devices = { "eth1", "lo" } priority 0; policy accept; } +J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": ["eth1", "lo"], "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}} + +I delete chain inet t c { type filter hook ingress priority filter; devices = { "eth1" }; } +O delete chain inet t c { type filter hook ingress devices = { "eth1" } priority 0; policy accept; } +J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": "eth1", "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}} + +I delete chain inet t c +O - +J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}} + + diff --git a/tests/monitor/testcases/flowtable-simple.t b/tests/monitor/testcases/flowtable-simple.t index 11254c51fcab7..e1889ae5d0076 100644 --- a/tests/monitor/testcases/flowtable-simple.t +++ b/tests/monitor/testcases/flowtable-simple.t @@ -8,3 +8,15 @@ J {"add": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0 I delete flowtable ip t ft O - J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0}}} + +I add flowtable ip t ft { hook ingress priority 0; devices = { "eth1", "lo" }; } +O - +J {"add": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0, "hook": "ingress", "prio": 0, "dev": ["eth1", "lo"]}}} + +I delete flowtable ip t ft { hook ingress priority 0; devices = { "eth1" }; } +O - +J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0, "hook": "ingress", "prio": 0, "dev": "eth1"}}} + +I delete flowtable ip t ft +O - +J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0}}} diff --git a/tests/monitor/testcases/table.t b/tests/monitor/testcases/table.t new file mode 100644 index 0000000000000..35a0f510436dc --- /dev/null +++ b/tests/monitor/testcases/table.t @@ -0,0 +1,15 @@ +I add table ip t +O - +J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}} + +I delete table ip t +O - +J {"delete": {"table": {"family": "ip", "name": "t", "handle": 0}}} + +I add table ip t { comment "foo bar"; flags dormant; } +O add table ip t { flags dormant; } +J {"add": {"table": {"family": "ip", "name": "t", "handle": 0, "flags": ["dormant"], "comment": "foo bar"}}} + +I delete table ip t +O - +J {"delete": {"table": {"family": "ip", "name": "t", "handle": 0}}} -- 2.51.0