src: allow for updating devices on existing netdev chain

This patch allows you to add/remove devices to an existing chain:

 # cat ruleset.nft
 table netdev x {
	chain y {
		type filter hook ingress devices = { eth0 } priority 0; policy accept;
	}
 }
 # nft -f ruleset.nft
 # nft add chain netdev x y '{ devices = { eth1 };  }'
 # nft list ruleset
 table netdev x {
	chain y {
		type filter hook ingress devices = { eth0, eth1 } priority 0; policy accept;
	}
 }
 # nft delete chain netdev x y '{ devices = { eth0 }; }'
 # nft list ruleset
 table netdev x {
	chain y {
		type filter hook ingress devices = { eth1 } priority 0; policy accept;
	}
 }

This feature allows for creating an empty netdev chain, with no devices.
In such case, no packets are seen until a device is registered.

This patch includes extended netlink error reporting:

 # nft add chain netdev x y '{ devices = { x } ; }'
 Error: Could not process rule: No such file or directory
 add chain netdev x y { devices = { x } ; }
                                    ^

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/evaluate.c b/src/evaluate.c
index fe15d7ace5ddea7af94e61618ed100dd0f3a94eb..35910b03ba7c3d8f20f5294154ccedd1230647a0 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4964,15 +4964,17 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
                                return chain_error(ctx, chain, "invalid policy expression %s",
                                                   expr_name(chain->policy));
                }
+       }
+
+       if (chain->dev_expr) {
+               if (!(chain->flags & CHAIN_F_BASECHAIN))
+                       chain->flags |= CHAIN_F_BASECHAIN;
 
                if (chain->handle.family == NFPROTO_NETDEV ||
                    (chain->handle.family == NFPROTO_INET &&
                     chain->hook.num == NF_INET_INGRESS)) {
-                       if (!chain->dev_expr)
-                               return __stmt_binary_error(ctx, &chain->loc, NULL,
-                                                          "Missing `device' in this chain definition");
-
-                       if (!evaluate_device_expr(ctx, &chain->dev_expr))
+                       if (chain->dev_expr &&
+                           !evaluate_device_expr(ctx, &chain->dev_expr))
                                return -1;
                } else if (chain->dev_expr) {
                        return __stmt_binary_error(ctx, &chain->dev_expr->location, NULL,

diff --git a/src/mnl.c b/src/mnl.c
index 4c16facaccab5972a3aac4ecffd4897b495d339b..c590c9554e0cf4b2fbcf66050617fb9f8c4dde5c 100644 (file)
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -763,18 +763,34 @@ static void nft_dev_array_free(const struct nft_dev *dev_array)
        xfree(dev_array);
 }
 
+static void mnl_nft_chain_devs_build(struct nlmsghdr *nlh, struct cmd *cmd)
+{
+       const struct expr *dev_expr = cmd->chain->dev_expr;
+       const struct nft_dev *dev_array;
+       struct nlattr *nest_dev;
+       int i, num_devs = 0;
+
+       dev_array = nft_dev_array(dev_expr, &num_devs);
+       if (num_devs == 1) {
+               mnl_attr_put_strz(nlh, NFTA_HOOK_DEV, dev_array[0].ifname);
+       } else {
+               nest_dev = mnl_attr_nest_start(nlh, NFTA_HOOK_DEVS);
+               for (i = 0; i < num_devs; i++) {
+                       cmd_add_loc(cmd, nlh->nlmsg_len, dev_array[i].location);
+                       mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev_array[i].ifname);
+                       mnl_attr_nest_end(nlh, nest_dev);
+               }
+       }
+       nft_dev_array_free(dev_array);
+}
+
 int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                      unsigned int flags)
 {
        struct nftnl_udata_buf *udbuf;
-       int priority, policy, i = 0;
        struct nftnl_chain *nlc;
-       unsigned int ifname_len;
-       const char **dev_array;
-       char ifname[IFNAMSIZ];
        struct nlmsghdr *nlh;
-       struct expr *expr;
-       int dev_array_len;
+       int priority, policy;
 
        nlc = nftnl_chain_alloc();
        if (nlc == NULL)
@@ -787,46 +803,6 @@ int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                        nftnl_chain_set_u32(nlc, NFTNL_CHAIN_FLAGS,
                                            CHAIN_F_HW_OFFLOAD);
                }
-               if (cmd->chain->flags & CHAIN_F_BASECHAIN) {
-                       nftnl_chain_set_u32(nlc, NFTNL_CHAIN_HOOKNUM,
-                                           cmd->chain->hook.num);
-                       mpz_export_data(&priority,
-                                       cmd->chain->priority.expr->value,
-                                       BYTEORDER_HOST_ENDIAN, sizeof(int));
-                       nftnl_chain_set_s32(nlc, NFTNL_CHAIN_PRIO, priority);
-                       nftnl_chain_set_str(nlc, NFTNL_CHAIN_TYPE,
-                                           cmd->chain->type.str);
-               }
-               if (cmd->chain->dev_expr) {
-                       dev_array = xmalloc(sizeof(char *) * 8);
-                       dev_array_len = 8;
-                       list_for_each_entry(expr, &cmd->chain->dev_expr->expressions, list) {
-                               ifname_len = div_round_up(expr->len, BITS_PER_BYTE);
-                               memset(ifname, 0, sizeof(ifname));
-                               mpz_export_data(ifname, expr->value,
-                                               BYTEORDER_HOST_ENDIAN,
-                                               ifname_len);
-                               dev_array[i++] = xstrdup(ifname);
-                               if (i == dev_array_len) {
-                                       dev_array_len *= 2;
-                                       dev_array = xrealloc(dev_array,
-                                                            dev_array_len * sizeof(char *));
-                               }
-                       }
-
-                       dev_array[i] = NULL;
-                       if (i == 1)
-                               nftnl_chain_set_str(nlc, NFTNL_CHAIN_DEV, dev_array[0]);
-                       else if (i > 1)
-                               nftnl_chain_set_data(nlc, NFTNL_CHAIN_DEVICES, dev_array,
-                                                    sizeof(char *) * dev_array_len);
-
-                       i = 0;
-                       while (dev_array[i] != NULL)
-                               xfree(dev_array[i++]);
-
-                       xfree(dev_array);
-               }
                if (cmd->chain->comment) {
                        udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
                        if (!udbuf)
@@ -861,12 +837,6 @@ int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                        nftnl_chain_set_u32(nlc, NFTNL_CHAIN_FLAGS, cmd->chain->flags);
        }
 
-       if (cmd->chain && cmd->chain->flags & CHAIN_F_BASECHAIN) {
-               nftnl_chain_unset(nlc, NFTNL_CHAIN_TYPE);
-               cmd_add_loc(cmd, nlh->nlmsg_len, &cmd->chain->type.loc);
-               mnl_attr_put_strz(nlh, NFTA_CHAIN_TYPE, cmd->chain->type.str);
-       }
-
        if (cmd->chain && cmd->chain->policy) {
                mpz_export_data(&policy, cmd->chain->policy->value,
                                BYTEORDER_HOST_ENDIAN, sizeof(int));
@@ -874,7 +844,33 @@ int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                mnl_attr_put_u32(nlh, NFTA_CHAIN_POLICY, htonl(policy));
        }
 
+       nftnl_chain_unset(nlc, NFTNL_CHAIN_TYPE);
+
        nftnl_chain_nlmsg_build_payload(nlh, nlc);
+
+       if (cmd->chain && cmd->chain->flags & CHAIN_F_BASECHAIN) {
+               struct nlattr *nest;
+
+               if (cmd->chain->type.str) {
+                       cmd_add_loc(cmd, nlh->nlmsg_len, &cmd->chain->type.loc);
+                       mnl_attr_put_strz(nlh, NFTA_CHAIN_TYPE, cmd->chain->type.str);
+               }
+
+               nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK);
+
+               if (cmd->chain->type.str) {
+                       mnl_attr_put_u32(nlh, NFTA_HOOK_HOOKNUM, htonl(cmd->chain->hook.num));
+                       mpz_export_data(&priority, cmd->chain->priority.expr->value,
+                                       BYTEORDER_HOST_ENDIAN, sizeof(int));
+                       mnl_attr_put_u32(nlh, NFTA_HOOK_PRIORITY, htonl(priority));
+               }
+
+               if (cmd->chain && cmd->chain->dev_expr)
+                       mnl_nft_chain_devs_build(nlh, cmd);
+
+               mnl_attr_nest_end(nlh, nest);
+       }
+
        nftnl_chain_free(nlc);
 
        mnl_nft_batch_continue(ctx->batch);
@@ -943,6 +939,15 @@ int mnl_nft_chain_del(struct netlink_ctx *ctx, struct cmd *cmd)
                                 htobe64(cmd->handle.handle.id));
        }
 
+       if (cmd->op == CMD_DELETE &&
+           cmd->chain && cmd->chain->dev_expr) {
+               struct nlattr *nest;
+
+               nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK);
+               mnl_nft_chain_devs_build(nlh, cmd);
+               mnl_attr_nest_end(nlh, nest);
+       }
+
        nftnl_chain_nlmsg_build_payload(nlh, nlc);
        nftnl_chain_free(nlc);
 

diff --git a/src/monitor.c b/src/monitor.c
index 9692b859e6eb0e5077479f21fd3596a61a065de8..3a1896917923f6c0c9b1b46adc08c975356b0d7d 100644 (file)
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -272,10 +272,13 @@ static int netlink_events_chain_cb(const struct nlmsghdr *nlh, int type,
                        chain_print_plain(c, &monh->ctx->nft->output);
                        break;
                case NFT_MSG_DELCHAIN:
-                       nft_mon_print(monh, "chain %s %s %s",
-                                     family2str(c->handle.family),
-                                     c->handle.table.name,
-                                     c->handle.chain.name);
+                       if (c->dev_array_len > 0)
+                               chain_print_plain(c, &monh->ctx->nft->output);
+                       else
+                               nft_mon_print(monh, "chain %s %s %s",
+                                             family2str(c->handle.family),
+                                             c->handle.table.name,
+                                             c->handle.chain.name);
                        break;
                }
                nft_mon_print(monh, "\n");

diff --git a/src/parser_bison.y b/src/parser_bison.y
index e4f21ca1a722358b653586ec3c1f7eed3e6e262f..90a2b9c3d681c1fb3bc205af910f63d4c62aa971 100644 (file)
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1358,6 +1358,13 @@ delete_cmd               :       TABLE           table_or_id_spec
                        {
                                $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_CHAIN, &$2, &@$, NULL);
                        }
+                       |       CHAIN           chain_spec      chain_block_alloc
+                                               '{'     chain_block     '}'
+                       {
+                               $5->location = @5;
+                               handle_merge(&$3->handle, &$2);
+                               $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_CHAIN, &$2, &@$, $5);
+                       }
                        |       RULE            ruleid_spec
                        {
                                $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_RULE, &$2, &@$, NULL);
@@ -2004,6 +2011,15 @@ chain_block              :       /* empty */     { $$ = $<chain>-1; }
                                list_add_tail(&$2->list, &$1->rules);
                                $$ = $1;
                        }
+                       |       chain_block     DEVICES         '='     flowtable_expr  stmt_separator
+                       {
+                               if ($$->dev_expr) {
+                                       list_splice_init(&$4->expressions, &$$->dev_expr->expressions);
+                                       expr_free($4);
+                                       break;
+                               }
+                               $$->dev_expr = $4;
+                       }
                        |       chain_block     comment_spec    stmt_separator
                        {
                                if (already_set($1->comment, &@2, state)) {

diff --git a/src/rule.c b/src/rule.c
index 06042239c84374ce709e9ac7a5a70ed70e567f90..c075d027f9ba5472bf4d4906359613cf55f23881 100644 (file)
--- a/src/rule.c
+++ b/src/rule.c
@@ -1091,8 +1091,21 @@ void chain_print_plain(const struct chain *chain, struct output_ctx *octx)
        if (chain->flags & CHAIN_F_BASECHAIN) {
                mpz_export_data(&policy, chain->policy->value,
                                BYTEORDER_HOST_ENDIAN, sizeof(int));
-               nft_print(octx, " { type %s hook %s priority %s; policy %s; }",
-                         chain->type.str, chain->hook.name,
+               nft_print(octx, " { type %s hook %s ",
+                         chain->type.str, chain->hook.name);
+
+               if (chain->dev_array_len > 0) {
+                       int i;
+
+                       nft_print(octx, "devices = { ");
+                       for (i = 0; i < chain->dev_array_len; i++) {
+                               nft_print(octx, "%s", chain->dev_array[i]);
+                               if (i + 1 != chain->dev_array_len)
+                                       nft_print(octx, ", ");
+                       }
+                       nft_print(octx, " } ");
+               }
+               nft_print(octx, "priority %s; policy %s; }",
                          prio2str(octx, priobuf, sizeof(priobuf),
                                   chain->handle.family, chain->hook.num,
                                   chain->priority.expr),

diff --git a/tests/shell/testcases/chains/dumps/netdev_chain_0.nft b/tests/shell/testcases/chains/dumps/netdev_chain_0.nft
new file mode 100644 (file)
index 0000000..bc02dc1
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/netdev_chain_0.nft
@@ -0,0 +1,5 @@
+table netdev x {
+       chain y {
+               type filter hook ingress devices = { d0, d1 } priority filter; policy accept;
+       }
+}

diff --git a/tests/shell/testcases/chains/netdev_chain_0 b/tests/shell/testcases/chains/netdev_chain_0
new file mode 100755 (executable)
index 0000000..67cd715
--- /dev/null
+++ b/tests/shell/testcases/chains/netdev_chain_0
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+ip link add d0 type dummy || {
+        echo "Skipping, no dummy interface available"
+        exit 0
+}
+trap "ip link del d0" EXIT
+
+ip link add d1 type dummy || {
+        echo "Skipping, no dummy interface available"
+        exit 0
+}
+trap "ip link del d1" EXIT
+
+ip link add d2 type dummy || {
+        echo "Skipping, no dummy interface available"
+        exit 0
+}
+trap "ip link del d2" EXIT
+
+set -e
+
+RULESET="table netdev x {
+       chain y {
+               type filter hook ingress priority 0; policy accept;
+       }
+}"
+
+$NFT -f - <<< "$RULESET"
+
+$NFT add chain netdev x y '{ devices = { d0 }; }'
+$NFT add chain netdev x y '{ devices = { d1, d2, lo }; }'
+$NFT delete chain netdev x y '{ devices = { lo }; }'