Re: [PATCH nf-next 2/6] netfilter: conntrack: get rid of conntrack timer

[Eric Dumazet]

On Fri, 2016-08-19 at 18:04 +0200, Florian Westphal wrote:
> Eric Dumazet  wrote:
> > On Fri, 2016-08-19 at 17:16 +0200, Florian Westphal wrote:
> > 
> > > Hmm, nf_conntrack_find caller needs to hold rcu_read_lock,
> > > in case object is free'd SLAB_DESTROY_BY_RCU should delay actual release
> > > of the page.
> > 
> > Well, point is that SLAB_DESTROY_BY_RCU means that we have no grace
> > period, and object can be immediately reused and recycled.
> > 
> > @next pointer can definitely be overwritten.
> 
> I see.  Isn't that detected by the nulls magic (to restart
> lookup if entry was moved to other chain due to overwritten next pointer)?

Well, you did not add the nulls magic in your code ;)

It might be fine, since it should be a rare event, and garbage
collection is best effort, so you might add a comment in gc_worker() why
it is probably overkill to restart the loop in this unlikely event.

BTW, maybe nf_conntrack_tuple_taken() should get the nulls magic check,
as it is currently missing.




--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf] netfilter: nft_reject: restrict to INPUT/FORWARD/OUTPUT

[Liping Zhang]

From: Liping Zhang 

After I add the nft rule "nft add rule filter prerouting reject
with tcp reset", kernel panic happened on my system:
  NULL pointer dereference at ...
  IP: [] nf_send_reset+0xaf/0x400
  Call Trace:
  [] ? nf_reject_ip_tcphdr_get+0x160/0x160
  [] nft_reject_ipv4_eval+0x61/0xb0 [nft_reject_ipv4]
  [] nft_do_chain+0x1fa/0x890 [nf_tables]
  [] ? __nft_trace_packet+0x170/0x170 [nf_tables]
  [] ? nf_ct_invert_tuple+0xb0/0xc0 [nf_conntrack]
  [] ? nf_nat_setup_info+0x5d4/0x650 [nf_nat]
  [...]

Because in the PREROUTING chain, routing information is not exist,
then we will dereference the NULL pointer and oops happen.

So we restrict reject expression to INPUT, FORWARD and OUTPUT chain.
This is consistent with iptables REJECT target.

Signed-off-by: Liping Zhang 
---
 include/net/netfilter/nft_reject.h   |  4 
 net/ipv4/netfilter/nft_reject_ipv4.c |  1 +
 net/ipv6/netfilter/nft_reject_ipv6.c |  1 +
 net/netfilter/nft_reject.c   | 16 
 net/netfilter/nft_reject_inet.c  |  7 ++-
 5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/net/netfilter/nft_reject.h 
b/include/net/netfilter/nft_reject.h
index 60fa153..02e28c5 100644
--- a/include/net/netfilter/nft_reject.h
+++ b/include/net/netfilter/nft_reject.h
@@ -8,6 +8,10 @@ struct nft_reject {
 
 extern const struct nla_policy nft_reject_policy[];
 
+int nft_reject_validate(const struct nft_ctx *ctx,
+   const struct nft_expr *expr,
+   const struct nft_data **data);
+
 int nft_reject_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[]);
diff --git a/net/ipv4/netfilter/nft_reject_ipv4.c 
b/net/ipv4/netfilter/nft_reject_ipv4.c
index c24f41c..2c2553b 100644
--- a/net/ipv4/netfilter/nft_reject_ipv4.c
+++ b/net/ipv4/netfilter/nft_reject_ipv4.c
@@ -46,6 +46,7 @@ static const struct nft_expr_ops nft_reject_ipv4_ops = {
.eval   = nft_reject_ipv4_eval,
.init   = nft_reject_init,
.dump   = nft_reject_dump,
+   .validate   = nft_reject_validate,
 };
 
 static struct nft_expr_type nft_reject_ipv4_type __read_mostly = {
diff --git a/net/ipv6/netfilter/nft_reject_ipv6.c 
b/net/ipv6/netfilter/nft_reject_ipv6.c
index 533cd57..92bda99 100644
--- a/net/ipv6/netfilter/nft_reject_ipv6.c
+++ b/net/ipv6/netfilter/nft_reject_ipv6.c
@@ -47,6 +47,7 @@ static const struct nft_expr_ops nft_reject_ipv6_ops = {
.eval   = nft_reject_ipv6_eval,
.init   = nft_reject_init,
.dump   = nft_reject_dump,
+   .validate   = nft_reject_validate,
 };
 
 static struct nft_expr_type nft_reject_ipv6_type __read_mostly = {
diff --git a/net/netfilter/nft_reject.c b/net/netfilter/nft_reject.c
index 0522fc9..c64de3f7 100644
--- a/net/netfilter/nft_reject.c
+++ b/net/netfilter/nft_reject.c
@@ -26,11 +26,27 @@ const struct nla_policy nft_reject_policy[NFTA_REJECT_MAX + 
1] = {
 };
 EXPORT_SYMBOL_GPL(nft_reject_policy);
 
+int nft_reject_validate(const struct nft_ctx *ctx,
+   const struct nft_expr *expr,
+   const struct nft_data **data)
+{
+   return nft_chain_validate_hooks(ctx->chain,
+   (1 << NF_INET_LOCAL_IN) |
+   (1 << NF_INET_FORWARD) |
+   (1 << NF_INET_LOCAL_OUT));
+}
+EXPORT_SYMBOL_GPL(nft_reject_validate);
+
 int nft_reject_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
 {
struct nft_reject *priv = nft_expr_priv(expr);
+   int err;
+
+   err = nft_reject_validate(ctx, expr, NULL);
+   if (err < 0)
+   return err;
 
if (tb[NFTA_REJECT_TYPE] == NULL)
return -EINVAL;
diff --git a/net/netfilter/nft_reject_inet.c b/net/netfilter/nft_reject_inet.c
index 759ca52..e79d9ca 100644
--- a/net/netfilter/nft_reject_inet.c
+++ b/net/netfilter/nft_reject_inet.c
@@ -66,7 +66,11 @@ static int nft_reject_inet_init(const struct nft_ctx *ctx,
const struct nlattr * const tb[])
 {
struct nft_reject *priv = nft_expr_priv(expr);
-   int icmp_code;
+   int icmp_code, err;
+
+   err = nft_reject_validate(ctx, expr, NULL);
+   if (err < 0)
+   return err;
 
if (tb[NFTA_REJECT_TYPE] == NULL)
return -EINVAL;
@@ -124,6 +128,7 @@ static const struct nft_expr_ops nft_reject_inet_ops = {
.eval   = nft_reject_inet_eval,
.init   = nft_reject_inet_init,
.dump   = nft_reject_inet_dump,
+   .validate   = nft_reject_validate,
 };
 
 static struct nft_expr_type nft_reject_inet_type __read_mostly = {
-- 
2.5.5


--
To unsubscribe from this list: 

Re: [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible

[Arturo Borrero Gonzalez]

On 21 August 2016 at 20:10, Pablo M. Bermudo Garay  wrote:
> This patch adds a verification of the compatibility between the nft
> ruleset and iptables. If the nft ruleset is not compatible with
> iptables, the execution stops and an error message is displayed to the
> user.
>
> This checking is triggered by xtables-compat -L and xtables-compat-save
> commands.
>
> Signed-off-by: Pablo M. Bermudo Garay 
> ---
>  iptables/nft.c  | 166 
> 
>  iptables/nft.h  |   2 +
>  iptables/xtables-save.c |   5 ++
>  iptables/xtables.c  |   5 ++
>  4 files changed, 178 insertions(+)
>
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 247a60a..7389689 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -2698,3 +2698,169 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t 
> flag)
>
> return NFT_CMP_EQ;
>  }
> +
> +static int nft_is_rule_compatible(struct nftnl_rule *rule)
> +{
> +   struct nftnl_expr_iter *iter;
> +   struct nftnl_expr *expr;
> +
> +   iter = nftnl_expr_iter_create(rule);
> +   if (iter == NULL)
> +   return -1;
> +
> +   expr = nftnl_expr_iter_next(iter);
> +   while (expr != NULL) {
> +   const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
> +
> +   if (strcmp(name, "counter")   &&
> +   strcmp(name, "match") &&
> +   strcmp(name, "target")&&
> +   strcmp(name, "payload")   &&
> +   strcmp(name, "meta")  &&
> +   strcmp(name, "bitwise")   &&
> +   strcmp(name, "cmp")   &&
> +   strcmp(name, "immediate") &&
> +   strcmp(name, "match") &&
> +   strcmp(name, "target")) {
> +   nftnl_expr_iter_destroy(iter);
> +   return 1;
> +   }
> +
> +   expr = nftnl_expr_iter_next(iter);
> +   }
> +
> +   nftnl_expr_iter_destroy(iter);
> +   return 0;
> +}
> +

I don't fully understand this logic. It seems there are expression
names which are repeated. Is that intended?

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4 V6 nft] Implement deleting rule by description

[Carlos Falgueras García]

This patch introduces deletion in a similar fashion as in iptables, thus,
we can delete the first rule that matches our description, for example:

$ nft list -a ruleset
table ip t {
chain c {
ip saddr 1.1.1.1 counter packets 0 bytes 0 # handle 1
ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 2
ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 3
ip saddr 1.1.1.4 counter packets 0 bytes 0 # handle 4
}
}
$ nft delete rule table chain ip saddr 1.1.1.2 counter
$ nft list -a ruleset
table ip t {
chain c {
ip saddr 1.1.1.1 counter packets 0 bytes 0 # handle 1
ip saddr 1.1.1.2 counter packets 0 bytes 0 # handle 3
ip saddr 1.1.1.4 counter packets 0 bytes 0 # handle 4
}
}

The parser rule 'ruleid_spec' is now of the type 'struct rule' in order to
hold a rule description. When rule is identified with its handle a dummy
'struct rule' is allocated to hold the specified handle.

Signed-off-by: Carlos Falgueras García 
---
 src/evaluate.c |  6 ++
 src/parser_bison.y | 24 
 src/rule.c | 50 --
 3 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 2f94ac6..f7b349b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2661,7 +2661,13 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, 
struct cmd *cmd)
 
return setelem_evaluate(ctx, >expr);
case CMD_OBJ_SET:
+   return 0;
case CMD_OBJ_RULE:
+   /* CMD_LIST force caching all ruleset */
+   ret = cache_update(CMD_LIST, ctx->msgs);
+   if (ret < 0)
+   return ret;
+   return rule_evaluate(ctx, cmd->rule);
case CMD_OBJ_CHAIN:
case CMD_OBJ_TABLE:
return 0;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 2ca7eea..beea38b 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -425,8 +425,8 @@ static void location_update(struct location *loc, struct 
location *rhs, int n)
 %type base_cmd add_cmd replace_cmd create_cmd 
insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd 
describe_cmd
 %destructor { cmd_free($$); }  base_cmd add_cmd replace_cmd create_cmd 
insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd 
describe_cmd
 
-%type  table_spec chain_spec chain_identifier 
ruleid_spec handle_spec position_spec rule_position ruleset_spec
-%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier 
ruleid_spec handle_spec position_spec rule_position ruleset_spec
+%type  table_spec chain_spec chain_identifier 
handle_spec position_spec rule_position ruleset_spec
+%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier 
handle_spec position_spec rule_position ruleset_spec
 %type  set_spec set_identifier
 %destructor { handle_free(&$$); } set_spec set_identifier
 %type family_spec family_spec_explicit chain_policy 
prio_spec
@@ -438,7 +438,7 @@ static void location_update(struct location *loc, struct 
location *rhs, int n)
 %destructor { close_scope(state); table_free($$); }table_block_alloc
 %type   chain_block_alloc chain_block
 %destructor { close_scope(state); chain_free($$); }chain_block_alloc
-%typerule rule_alloc
+%typerule ruleid_spec rule_alloc
 %destructor { rule_free($$); } rule
 
 %type set_flag_list   set_flag
@@ -745,9 +745,10 @@ add_cmd:   TABLE   
table_spec
}
;
 
-replace_cmd:   RULEruleid_spec rule
+replace_cmd:   RULEchain_spec  handle_spec 
rule
{
-   $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, 
&@$, $3);
+   handle_merge(&$2, &$3);
+   $$ = cmd_alloc(CMD_REPLACE, CMD_OBJ_RULE, &$2, 
&@$, $4);
}
;
 
@@ -792,7 +793,7 @@ delete_cmd  :   TABLE   table_spec
}
|   RULEruleid_spec
{
-   $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_RULE, &$2, 
&@$, NULL);
+   $$ = cmd_alloc(CMD_DELETE, CMD_OBJ_RULE, 
&$2->handle, &@$, $2);
}
|   SET set_spec
{
@@ -1276,8 +1277,15 @@ rule_position

[PATCH 4/4 V6 nft] parser: Improve syntax errors

[Carlos Falgueras García]

Shows a more informative message when user commits a syntax error:

$ nft add rule t c handle 3 ...
:1:14-19: Error: Did you mean `position'?
add rule t c handle 3 ...
 ^^
$ nft delete rule t c position 3 ...
:1:17-24: Error: Did you mean `handle' or insert a rule 
description?
delete rule t c position 3 ...


Adds function 'erec_del_last' that deletes last error from the error queue.
This is needed to do not show two error messages.

Signed-off-by: Carlos Falgueras García 
---
 include/erec.h |  5 +
 src/parser_bison.y | 17 +
 2 files changed, 22 insertions(+)

diff --git a/include/erec.h b/include/erec.h
index 36e0efa..95ed24a 100644
--- a/include/erec.h
+++ b/include/erec.h
@@ -58,6 +58,11 @@ static inline void erec_queue(struct error_record *erec,
list_add_tail(>list, queue);
 }
 
+static inline void erec_del_last(struct list_head *queue)
+{
+   list_del(queue->prev);
+}
+
 extern void erec_print(FILE *f, const struct error_record *erec);
 extern void erec_print_list(FILE *f, struct list_head *list);
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index beea38b..9739cb1 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1273,6 +1273,14 @@ rule_position:   chain_spec
$$ = $1;
handle_merge(&$$, &$2);
}
+   |   chain_spec HANDLE error
+   {
+   erec_del_last(state->msgs);
+   erec_queue(error(&@2, "Did you mean 
`position'?"),
+  state->msgs);
+   $$ = $1;
+   YYERROR;
+   }
;
 
 ruleid_spec:   chain_spec handle_spec
@@ -1287,6 +1295,15 @@ ruleid_spec  :   chain_spec handle_spec
$$ = $2;
handle_merge(&$$->handle, &$1);
}
+   |   chain_spec POSITION error
+   {
+   erec_del_last(state->msgs);
+   erec_queue(error(&@2, "Did you mean `handle' or 
insert a rule description?"),
+  state->msgs);
+   $$ = rule_alloc(&@$, NULL);
+   handle_merge(&$$->handle, &$1);
+   YYERROR;
+   }
;
 
 comment_spec   :   COMMENT string
-- 
2.8.3

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4 V6 nft] Simplify parser rule_spec tree

[Carlos Falgueras García]

This patch separates the rule identification from the rule localization, so
the logic moves from the evaluator to the parser. This allows to revert the
patch "evaluate: improve rule managment checks"
(4176c7d30c2ff1b3f52468fc9c08b8df83f979a8) and saves a lot of code.

Signed-off-by: Carlos Falgueras García 
---
 src/evaluate.c | 68 +-
 src/parser_bison.y | 45 +---
 2 files changed, 22 insertions(+), 91 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 87f5a6d..2f94ac6 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -44,12 +44,6 @@ static const char *byteorder_names[] = {
__stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
 #define cmd_error(ctx, fmt, args...) \
__stmt_binary_error(ctx, &(ctx->cmd)->location, NULL, fmt, ## args)
-#define handle_error(ctx, fmt, args...) \
-   __stmt_binary_error(ctx, >cmd->handle.handle.location, NULL, fmt, 
## args)
-#define position_error(ctx, fmt, args...) \
-   __stmt_binary_error(ctx, >cmd->handle.position.location, NULL, 
fmt, ## args)
-#define handle_position_error(ctx, fmt, args...) \
-   __stmt_binary_error(ctx, >cmd->handle.handle.location, 
>cmd->handle.position.location, fmt, ## args)
 
 static int __fmtstring(3, 4) set_error(struct eval_ctx *ctx,
   const struct set *set,
@@ -2481,68 +2475,11 @@ static int set_evaluate(struct eval_ctx *ctx, struct 
set *set)
return 0;
 }
 
-static int rule_evaluate_cmd(struct eval_ctx *ctx)
-{
-   struct handle *handle = >cmd->handle;
-
-   /* allowed:
-* - insert [position] (no handle)
-* - add [position] (no handle)
-* - replace  (no position)
-* - delete  (no position)
-*/
-
-   switch (ctx->cmd->op) {
-   case CMD_INSERT:
-   if (handle->handle.id && handle->position.id)
-   return handle_position_error(ctx, "use only `position'"
-" instead");
-
-   if (handle->handle.id)
-   return handle_error(ctx, "use `position' instead");
-   break;
-   case CMD_ADD:
-   if (handle->handle.id && handle->position.id)
-   return handle_position_error(ctx, "use only `position'"
-" instead");
-
-   if (handle->handle.id)
-   return handle_error(ctx, "use `position' instead");
-
-   break;
-   case CMD_REPLACE:
-   if (handle->handle.id && handle->position.id)
-   return handle_position_error(ctx, "use only `handle' "
-"instead");
-   if (handle->position.id)
-   return position_error(ctx, "use `handle' instead");
-   if (!handle->handle.id)
-   return cmd_error(ctx, "missing `handle'");
-   break;
-   case CMD_DELETE:
-   if (handle->handle.id && handle->position.id)
-   return handle_position_error(ctx, "use only `handle' "
-"instead");
-   if (handle->position.id)
-   return position_error(ctx, "use `handle' instead");
-   if (!handle->handle.id)
-   return cmd_error(ctx, "missing `handle'");
-   break;
-   default:
-   BUG("unkown command type %u\n", ctx->cmd->op);
-   }
-
-   return 0;
-}
-
 static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
 {
struct stmt *stmt, *tstmt = NULL;
struct error_record *erec;
 
-   if (rule_evaluate_cmd(ctx) < 0)
-   return -1;
-
proto_ctx_init(>pctx, rule->handle.family);
memset(>ectx, 0, sizeof(ctx->ectx));
 
@@ -2723,11 +2660,8 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, 
struct cmd *cmd)
return ret;
 
return setelem_evaluate(ctx, >expr);
-   case CMD_OBJ_RULE:
-   if (rule_evaluate_cmd(ctx) < 0)
-   return -1;
-   /* fall through */
case CMD_OBJ_SET:
+   case CMD_OBJ_RULE:
case CMD_OBJ_CHAIN:
case CMD_OBJ_TABLE:
return 0;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index e16b8a3..2ca7eea 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -425,15 +425,12 @@ static void location_update(struct location *loc, struct 
location *rhs, int n)
 %type base_cmd add_cmd replace_cmd create_cmd 
insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd 
describe_cmd
 %destructor { cmd_free($$); }  base_cmd add_cmd replace_cmd create_cmd 
insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd 

[PATCH 3/4 V6 nft] test: shell: Add tests for deleting rule by description

[Carlos Falgueras García]

They checks if commands like "nft delete rule   "
works as is expected.

First one checks if command deletes only one of the matched rules.
Second one checks if command fails when rule did not found.

Signed-off-by: Carlos Falgueras García 
---
 .../testcases/rule_management/0010delete-by-desc_0 | 39 ++
 .../testcases/rule_management/0011delete-by-desc_1 | 20 +++
 2 files changed, 59 insertions(+)
 create mode 100755 tests/shell/testcases/rule_management/0010delete-by-desc_0
 create mode 100755 tests/shell/testcases/rule_management/0011delete-by-desc_1

diff --git a/tests/shell/testcases/rule_management/0010delete-by-desc_0 
b/tests/shell/testcases/rule_management/0010delete-by-desc_0
new file mode 100755
index 000..6afdec1
--- /dev/null
+++ b/tests/shell/testcases/rule_management/0010delete-by-desc_0
@@ -0,0 +1,39 @@
+#!/bin/bash
+
+# positive tests for rule deletion by description:
+#  $ nft delete rule   
+
+RULE2DEL="ip saddr 1.1.1.1 counter"
+
+set -e
+$NFT add table t
+$NFT add chain t c
+$NFT add rule t c ip saddr 1.1.1.1
+$NFT add rule t c $RULE2DEL
+$NFT add rule t c ip saddr 1.1.1.1 accept
+$NFT add rule t c $RULE2DEL
+
+$NFT delete rule t c $RULE2DEL
+if [ $? -ne 0 ]; then
+   echo "E: unable to delete rule \"$RULE2DEL\"" >&2
+   exit 1
+fi
+
+set +e; # Next commands can return 0
+REMAINS_RULE2DEL=$($NFT list -a ruleset | grep -c "$RULE2DEL")
+REMAINS_RULES=$(( $($NFT list -a ruleset | wc -l) - 4 ))
+set -e
+
+if   [ $REMAINS_RULE2DEL -eq 2 ]; then
+   echo "E: First rule \"$RULE2DEL\" should have been deleted" >&2
+   exit 1
+elif [ $REMAINS_RULE2DEL -eq 0 ]; then
+   echo "E: Second rule \"$RULE2DEL\" should not have been deleted" >&2
+   exit 1
+fi
+
+if [ $REMAINS_RULES -ne 3 ]; then
+   echo "E: Rest of rules should not have been deleted" >&2
+   $NFT list -a ruleset
+   exit 1
+fi
diff --git a/tests/shell/testcases/rule_management/0011delete-by-desc_1 
b/tests/shell/testcases/rule_management/0011delete-by-desc_1
new file mode 100755
index 000..3ddb5ef
--- /dev/null
+++ b/tests/shell/testcases/rule_management/0011delete-by-desc_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# negative tests for rule deletion by description:
+#  $ nft delete rule   
+
+set -e
+$NFT add table t
+$NFT add chain t c
+$NFT add rule t c ip saddr 1.1.1.1
+$NFT add rule t c ip saddr 1.1.1.1 accept
+
+set +e; # Next command must fail
+$NFT delete rule t c ip saddr 2.2.2.2
+RET=$?
+if [ $RET -ne 1 ]; then
+   echo "E: Try to delete a nonexistent rule should throw an error" >&2
+   exit $RET
+fi
+
+exit $RET
-- 
2.8.3

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH iptables 1/3] xtables-compat: remove useless functions

[Pablo M. Bermudo Garay]

The static function nft_rule_list_get was exposed outside nft.c through
the nft_rule_list_create function, but this was never used out there.

A similar situation occurs with nftnl_rule_list_free and
nft_rule_list_destroy.

This patch removes nft_rule_list_create and nft_rule_list_destroy for
the sake of simplicity.

Signed-off-by: Pablo M. Bermudo Garay 
---
 iptables/nft.c | 38 ++
 iptables/nft.h |  3 ---
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 05ba57a..247a60a 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1766,16 +1766,6 @@ static int __nft_rule_del(struct nft_handle *h, struct 
nftnl_rule_list *list,
return 1;
 }
 
-struct nftnl_rule_list *nft_rule_list_create(struct nft_handle *h)
-{
-   return nft_rule_list_get(h);
-}
-
-void nft_rule_list_destroy(struct nftnl_rule_list *list)
-{
-   nftnl_rule_list_free(list);
-}
-
 static struct nftnl_rule *
 nft_rule_find(struct nft_handle *h, struct nftnl_rule_list *list,
  const char *chain, const char *table, void *data, int rulenum)
@@ -1831,7 +1821,7 @@ int nft_rule_check(struct nft_handle *h, const char 
*chain,
 
nft_fn = nft_rule_check;
 
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
return 0;
 
@@ -1839,7 +1829,7 @@ int nft_rule_check(struct nft_handle *h, const char 
*chain,
if (ret == 0)
errno = ENOENT;
 
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
 
return ret;
 }
@@ -1853,7 +1843,7 @@ int nft_rule_delete(struct nft_handle *h, const char 
*chain,
 
nft_fn = nft_rule_delete;
 
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
return 0;
 
@@ -1865,7 +1855,7 @@ int nft_rule_delete(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
 
return ret;
 }
@@ -1906,7 +1896,7 @@ int nft_rule_insert(struct nft_handle *h, const char 
*chain,
nft_fn = nft_rule_insert;
 
if (rulenum > 0) {
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
goto err;
 
@@ -1918,7 +1908,7 @@ int nft_rule_insert(struct nft_handle *h, const char 
*chain,
r = nft_rule_find(h, list, chain, table, data,
  rulenum - 1);
if (r != NULL) {
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
return nft_rule_append(h, chain, table, data,
   0, verbose);
}
@@ -1930,12 +1920,12 @@ int nft_rule_insert(struct nft_handle *h, const char 
*chain,
handle = nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE);
DEBUGP("adding after rule handle %"PRIu64"\n", handle);
 
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
}
 
return nft_rule_add(h, chain, table, data, handle, verbose);
 err:
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
return 0;
 }
 
@@ -1948,7 +1938,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char 
*chain,
 
nft_fn = nft_rule_delete_num;
 
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
return 0;
 
@@ -1963,7 +1953,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
 
return ret;
 }
@@ -1977,7 +1967,7 @@ int nft_rule_replace(struct nft_handle *h, const char 
*chain,
 
nft_fn = nft_rule_replace;
 
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
return 0;
 
@@ -1993,7 +1983,7 @@ int nft_rule_replace(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
 
return ret;
 }
@@ -2256,7 +2246,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const 
char *chain,
 
nft_fn = nft_rule_delete;
 
-   list = nft_rule_list_create(h);
+   list = nft_rule_list_get(h);
if (list == NULL)
return 0;
 
@@ -2276,7 +2266,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const 
char *chain,
   false);
 
 error:
-   nft_rule_list_destroy(list);
+   nftnl_rule_list_free(list);
 
return ret;
 }
diff --git a/iptables/nft.h 

[PATCH iptables 3/3] xtables-compat: add rule cache

[Pablo M. Bermudo Garay]

This patch adds a cache of rules within the nft handle. This feature is
more useful after the new checks of ruleset compatibility, since the
rule list is loaded twice consecutively.

Now all the operations causing changes in the ruleset must invalidate
the cache, a function called flush_rule_cache has been introduced for
this purpose.

Signed-off-by: Pablo M. Bermudo Garay 
---
 iptables/nft.c | 35 +++
 iptables/nft.h |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 7389689..71bf6f7 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -780,8 +780,17 @@ int nft_init(struct nft_handle *h, struct builtin_table *t)
return 0;
 }
 
+static void flush_rule_cache(struct nft_handle *h)
+{
+   if (h->rule_cache) {
+   nftnl_rule_list_free(h->rule_cache);
+   h->rule_cache = NULL;
+   }
+}
+
 void nft_fini(struct nft_handle *h)
 {
+   flush_rule_cache(h);
mnl_socket_close(h->nl);
free(mnl_nlmsg_batch_head(h->batch));
mnl_nlmsg_batch_stop(h->batch);
@@ -1121,6 +1130,7 @@ nft_rule_append(struct nft_handle *h, const char *chain, 
const char *table,
if (batch_rule_add(h, type, r) < 0)
nftnl_rule_free(r);
 
+   flush_rule_cache(h);
return 1;
 }
 
@@ -1284,6 +1294,9 @@ static struct nftnl_rule_list *nft_rule_list_get(struct 
nft_handle *h)
struct nftnl_rule_list *list;
int ret;
 
+   if (h->rule_cache)
+   return h->rule_cache;
+
list = nftnl_rule_list_alloc();
if (list == NULL)
return 0;
@@ -1297,6 +1310,7 @@ static struct nftnl_rule_list *nft_rule_list_get(struct 
nft_handle *h)
return NULL;
}
 
+   h->rule_cache = list;
return list;
 }
 
@@ -1333,7 +1347,6 @@ next:
}
 
nftnl_rule_list_iter_destroy(iter);
-   nftnl_rule_list_free(list);
 
/* the core expects 1 for success and 0 for error */
return 1;
@@ -1396,6 +1409,7 @@ next:
}
 
nftnl_chain_list_iter_destroy(iter);
+   flush_rule_cache(h);
 err:
nftnl_chain_list_free(list);
 
@@ -1829,8 +1843,6 @@ int nft_rule_check(struct nft_handle *h, const char 
*chain,
if (ret == 0)
errno = ENOENT;
 
-   nftnl_rule_list_free(list);
-
return ret;
 }
 
@@ -1855,7 +1867,7 @@ int nft_rule_delete(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
 
return ret;
 }
@@ -1879,6 +1891,7 @@ nft_rule_add(struct nft_handle *h, const char *chain,
return 0;
}
 
+   flush_rule_cache(h);
return 1;
 }
 
@@ -1908,7 +1921,7 @@ int nft_rule_insert(struct nft_handle *h, const char 
*chain,
r = nft_rule_find(h, list, chain, table, data,
  rulenum - 1);
if (r != NULL) {
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
return nft_rule_append(h, chain, table, data,
   0, verbose);
}
@@ -1920,12 +1933,12 @@ int nft_rule_insert(struct nft_handle *h, const char 
*chain,
handle = nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE);
DEBUGP("adding after rule handle %"PRIu64"\n", handle);
 
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
}
 
return nft_rule_add(h, chain, table, data, handle, verbose);
 err:
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
return 0;
 }
 
@@ -1953,7 +1966,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
 
return ret;
 }
@@ -1983,7 +1996,7 @@ int nft_rule_replace(struct nft_handle *h, const char 
*chain,
} else
errno = ENOENT;
 
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
 
return ret;
 }
@@ -2037,8 +2050,6 @@ next:
 
nftnl_rule_list_iter_destroy(iter);
 err:
-   nftnl_rule_list_free(list);
-
if (ret == 0)
errno = ENOENT;
 
@@ -2266,7 +2277,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const 
char *chain,
   false);
 
 error:
-   nftnl_rule_list_free(list);
+   flush_rule_cache(h);
 
return ret;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index f5449db..4126593 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -35,6 +35,7 @@ struct nft_handle {
struct mnl_nlmsg_batch  *batch;
struct nft_family_ops   *ops;
struct builtin_table*tables;
+   struct nftnl_rule_list  

[PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible

[Pablo M. Bermudo Garay]

This patch adds a verification of the compatibility between the nft
ruleset and iptables. If the nft ruleset is not compatible with
iptables, the execution stops and an error message is displayed to the
user.

This checking is triggered by xtables-compat -L and xtables-compat-save
commands.

Signed-off-by: Pablo M. Bermudo Garay 
---
 iptables/nft.c  | 166 
 iptables/nft.h  |   2 +
 iptables/xtables-save.c |   5 ++
 iptables/xtables.c  |   5 ++
 4 files changed, 178 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 247a60a..7389689 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2698,3 +2698,169 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t 
flag)
 
return NFT_CMP_EQ;
 }
+
+static int nft_is_rule_compatible(struct nftnl_rule *rule)
+{
+   struct nftnl_expr_iter *iter;
+   struct nftnl_expr *expr;
+
+   iter = nftnl_expr_iter_create(rule);
+   if (iter == NULL)
+   return -1;
+
+   expr = nftnl_expr_iter_next(iter);
+   while (expr != NULL) {
+   const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
+
+   if (strcmp(name, "counter")   &&
+   strcmp(name, "match") &&
+   strcmp(name, "target")&&
+   strcmp(name, "payload")   &&
+   strcmp(name, "meta")  &&
+   strcmp(name, "bitwise")   &&
+   strcmp(name, "cmp")   &&
+   strcmp(name, "immediate") &&
+   strcmp(name, "match") &&
+   strcmp(name, "target")) {
+   nftnl_expr_iter_destroy(iter);
+   return 1;
+   }
+
+   expr = nftnl_expr_iter_next(iter);
+   }
+
+   nftnl_expr_iter_destroy(iter);
+   return 0;
+}
+
+static int check_builtin_chain(const char *table, const char *chain)
+{
+   const char *cur_table;
+   struct builtin_chain *chains;
+   int i, j;
+
+   for (i = 0; i < TABLES_MAX; i++) {
+   cur_table = xtables_ipv4[i].name;
+   chains = xtables_ipv4[i].chains;
+   if (strcmp(table, cur_table) == 0) {
+   for (j = 0; j < NF_INET_NUMHOOKS && chains[j].name; 
j++) {
+   if (strcmp(chain, chains[j].name) == 0)
+   return 0;
+   }
+   }
+   }
+
+   return 1;
+}
+
+static int nft_are_chains_compatible(struct nft_handle *h)
+{
+   struct nftnl_chain_list *list;
+   struct nftnl_chain_list_iter *iter;
+   struct nftnl_chain *chain;
+
+   list = nftnl_chain_list_get(h);
+   if (list == NULL)
+   return -1;
+
+   iter = nftnl_chain_list_iter_create(list);
+   if (iter == NULL)
+   return -1;
+
+   chain = nftnl_chain_list_iter_next(iter);
+   while (chain != NULL) {
+   if (nft_chain_builtin(chain)) {
+   const char *table = nftnl_chain_get(chain,
+   NFTNL_CHAIN_TABLE);
+   const char *name = nftnl_chain_get(chain,
+  NFTNL_CHAIN_NAME);
+
+   if (check_builtin_chain(table, name) == 1) {
+   nftnl_chain_list_iter_destroy(iter);
+   nftnl_chain_list_free(list);
+   return 1;
+   }
+   }
+
+   chain = nftnl_chain_list_iter_next(iter);
+   }
+
+   nftnl_chain_list_iter_destroy(iter);
+   nftnl_chain_list_free(list);
+   return 0;
+}
+
+static int nft_are_tables_compatible(struct nft_handle *h)
+{
+   struct nftnl_table_list *list;
+   struct nftnl_table_list_iter *iter;
+   struct nftnl_table *table;
+
+   list = nftnl_table_list_get(h);
+   if (list == NULL)
+   return -1;
+
+   iter = nftnl_table_list_iter_create(list);
+   if (iter == NULL)
+   return -1;
+
+   table = nftnl_table_list_iter_next(iter);
+   while (table != NULL) {
+   const char *name = nftnl_table_get(table, NFTNL_TABLE_NAME);
+
+   if (strcmp(name, "filter") &&
+   strcmp(name, "nat")&&
+   strcmp(name, "mangle") &&
+   strcmp(name, "raw")&&
+   strcmp(name, "security")) {
+   nftnl_table_list_iter_destroy(iter);
+   nftnl_table_list_free(list);
+   return 1;
+   }
+
+   table = nftnl_table_list_iter_next(iter);
+   }
+
+   nftnl_table_list_iter_destroy(iter);
+   nftnl_table_list_free(list);
+   return 0;
+}
+
+int nft_is_ruleset_compatible(struct 

[PATCH iptables] extensions: libxt_CLASSIFY: Add translation to nft

[Liping Zhang]

From: Liping Zhang 

For examples:
  # iptables-translate -A OUTPUT -j CLASSIFY --set-class 0:0
  nft add rule ip filter OUTPUT counter meta priority set none
  # iptables-translate -A OUTPUT -j CLASSIFY --set-class :
  nft add rule ip filter OUTPUT counter meta priority set root
  # iptables-translate -A OUTPUT -j CLASSIFY --set-class 1:234
  nft add rule ip filter OUTPUT counter meta priority set 1:234

Signed-off-by: Liping Zhang 
---
 extensions/libxt_CLASSIFY.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/extensions/libxt_CLASSIFY.c b/extensions/libxt_CLASSIFY.c
index cd016d8..ba88f75 100644
--- a/extensions/libxt_CLASSIFY.c
+++ b/extensions/libxt_CLASSIFY.c
@@ -80,6 +80,31 @@ arpCLASSIFY_print(const void *ip, const struct 
xt_entry_target *target,
CLASSIFY_save(ip, target);
 }
 
+static int CLASSIFY_xlate(struct xt_xlate *xl,
+ const struct xt_xlate_tg_params *params)
+{
+   const struct xt_classify_target_info *clinfo =
+   (const struct xt_classify_target_info *)params->target->data;
+   __u32 handle = clinfo->priority;
+
+   xt_xlate_add(xl, "meta priority set ");
+
+   switch (handle) {
+   case TC_H_ROOT:
+   xt_xlate_add(xl, "root");
+   break;
+   case TC_H_UNSPEC:
+   xt_xlate_add(xl, "none");
+   break;
+   default:
+   xt_xlate_add(xl, "%0x:%0x", TC_H_MAJ(handle) >> 16,
+TC_H_MIN(handle));
+   break;
+   }
+
+   return 1;
+}
+
 static struct xtables_target classify_target[] = {
{
.family = NFPROTO_UNSPEC,
@@ -92,6 +117,7 @@ static struct xtables_target classify_target[] = {
.save   = CLASSIFY_save,
.x6_parse   = CLASSIFY_parse,
.x6_options = CLASSIFY_opts,
+   .xlate  = CLASSIFY_xlate,
},
{
.family = NFPROTO_ARP,
@@ -103,6 +129,7 @@ static struct xtables_target classify_target[] = {
.print  = arpCLASSIFY_print,
.x6_parse   = CLASSIFY_parse,
.x6_options = CLASSIFY_opts,
+   .xlate  = CLASSIFY_xlate,
},
 };
 
-- 
2.5.5


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -next] netfilter: nft_hash: fix non static symbol warning

[Wei Yongjun]

Fixes the following sparse warning:

net/netfilter/nft_hash.c:40:25: warning:
 symbol 'nft_hash_policy' was not declared. Should it be static?

Signed-off-by: Wei Yongjun 
---
 net/netfilter/nft_hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index b82ff29..e090aee 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -37,7 +37,7 @@ static void nft_hash_eval(const struct nft_expr *expr,
 priv->modulus);
 }
 
-const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {
+static const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {
[NFTA_HASH_SREG]= { .type = NLA_U32 },
[NFTA_HASH_DREG]= { .type = NLA_U32 },
[NFTA_HASH_LEN] = { .type = NLA_U32 },

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html