[PATCH 2/2 nft] tests: shell: make sure split table definition works via nft -f

2016-07-13 Thread Pablo Neira Ayuso
Add test to cover split table definition in one single file.

Signed-off-by: Pablo Neira Ayuso 
---
 tests/shell/testcases/nft-f/0008split_tables_0 | 50 ++
 1 file changed, 50 insertions(+)
 create mode 100755 tests/shell/testcases/nft-f/0008split_tables_0

diff --git a/tests/shell/testcases/nft-f/0008split_tables_0 
b/tests/shell/testcases/nft-f/0008split_tables_0
new file mode 100755
index 000..2bc6e46
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0008split_tables_0
@@ -0,0 +1,50 @@
+#!/bin/bash
+
+set -e
+
+tmpfile=$(mktemp)
+if [ ! -w $tmpfile ] ; then
+echo "Failed to create tmp file" >&2
+exit 0
+fi
+
+trap "rm -rf $tmpfile" EXIT # cleanup if aborted
+
+RULESET="table inet filter {
+   chain ssh {
+   type filter hook input priority 0; policy accept;
+   tcp dport ssh accept;
+   }
+}
+
+table inet filter {
+   chain input {
+   type filter hook input priority 1; policy drop;
+   }
+}"
+
+echo "$RULESET" > $tmpfile
+$NFT -f $tmpfile
+if [ $? -ne 0 ] ; then
+echo "E: unable to load good ruleset" >&2
+exit 1
+fi
+
+EXPECTED="table inet filter {
+   chain ssh {
+   type filter hook input priority 0; policy accept;
+   tcp dport ssh accept
+   }
+
+   chain input {
+   type filter hook input priority 1; policy drop;
+   }
+}"
+
+GET="$($NFT list ruleset)"
+
+if [ "$EXPECTED" != "$GET" ] ; then
+   DIFF="$(which diff)"
+   [ -x $DIFF ] && $DIFF -u <(echo "$EXPECTED") <(echo "$GET")
+   exit 1
+fi
-- 
2.1.4

--
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/2 nft] parser_bison: fix typo in symbol redefinition error reporting

2016-07-13 Thread Pablo Neira Ayuso
"redefinition" instead of "redfinition".

Signed-off-by: Pablo Neira Ayuso 
---
 src/parser_bison.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index d946e0e..6a029d1 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -635,7 +635,7 @@ common_block:   INCLUDE 
QUOTED_STRING   stmt_seperator
struct scope *scope = current_scope(state);
 
if (symbol_lookup(scope, $2) != NULL) {
-   erec_queue(error(&@2, "redfinition of 
symbol '%s'", $2),
+   erec_queue(error(&@2, "redefinition of 
symbol '%s'", $2),
   state->msgs);
YYERROR;
}
-- 
2.1.4

--
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


Re: [PATCH nft v5 3/3] src: add xt compat support

2016-07-13 Thread Arturo Borrero Gonzalez
good to see this finally merged :-)

-- 
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


Re: [PATCH nft v5 1/3] include: cache ip_tables.h, ip6_tables.h, arp_tables.h and ebtables.h

2016-07-13 Thread Pablo Neira Ayuso
On Tue, Jul 12, 2016 at 10:04:15PM +0200, Pablo M. Bermudo Garay wrote:
> From: Pablo Neira 
> 
> The xt over nft support that comes in follow up patches need this, and update
> the corresponding Makefile.am.
> 
> Based on patch from Arturo Borrero Gonzalez.

Applied, thanks.
--
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


Re: [PATCH nft v5 3/3] src: add xt compat support

2016-07-13 Thread Pablo Neira Ayuso
On Tue, Jul 12, 2016 at 10:04:17PM +0200, Pablo M. Bermudo Garay wrote:
> From: Pablo Neira 
> 
> At compilation time, you have to pass this option.
> 
>   # ./configure --with-xtables
> 
> And libxtables needs to be installed in your system.
> 
> This patch allows to list a ruleset containing xt extensions loaded
> through iptables-compat-restore tool.
> 
> Example:
> 
> $ iptables-save > ruleset
> 
> $ cat ruleset
> *filter
> :INPUT ACCEPT [0:0]
> :FORWARD ACCEPT [0:0]
> :OUTPUT ACCEPT [0:0]
> -A INPUT -p tcp -m multiport --dports 80,81 -j REJECT
> COMMIT
> 
> $ sudo iptables-compat-restore ruleset
> 
> $ sudo nft list rulseset
> table ip filter {
> chain INPUT {
> type filter hook input priority 0; policy accept;
> ip protocol tcp tcp dport { 80,81} counter packets 0 bytes 0 reject
> }
> 
> chain FORWARD {
> type filter hook forward priority 0; policy drop;
> }
> 
> chain OUTPUT {
> type filter hook output priority 0; policy accept;
> }
> }
> 
> A translation of the extension is shown if this is available. In other
> case, match or target definition is preceded by a hash. For example,
> classify target has not translation:
> 
> $ sudo nft list chain mangle POSTROUTING
> table ip mangle {
> chain POSTROUTING {
> type filter hook postrouting priority -150; policy accept;
> ip protocol tcp tcp dport 80 counter packets 0 bytes 0 # CLASSIFY set 
> 20:10
>   ^^^
> }
> }
> 
> If the whole ruleset is translatable, the users can (re)load it using
> "nft -f" and get nft native support for all their rules.

Applied, thanks!
--
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 v2 nf] netfilter: x_tables: speed up jump target validation

2016-07-13 Thread Florian Westphal
The dummy ruleset I used to test the original validation change was broken,
most rules were unreachable and were not tested by mark_source_chains().

In some cases rulesets that used to load in a few seconds now require
several minutes.

sample ruleset that shows the behaviour:

echo "*filter"
for i in $(seq 0 10);do
printf ":chain_%06x - [0:0]\n" $i
done
for i in $(seq 0 10);do
   printf -- "-A INPUT -j chain_%06x\n" $i
   printf -- "-A INPUT -j chain_%06x\n" $i
   printf -- "-A INPUT -j chain_%06x\n" $i
done
echo COMMIT

[ pipe result into iptables-restore ]

 78401416, count 59
This ruleset will be about 74mbyte in size, with ~500k searches
though all 500k[1] rule entries. iptables-restore will take forever
(gave up after 10 minutes)

Instead of always searching the entire blob for a match, fill an
array with the start offsets of every single ipt_entry struct,
then do a binary search to check if the jump target is present or not.

After this change ruleset restore times get again close to what one
gets when reverting 36472341017529e (~3 seconds on my workstation).

[1] every user-defined rule gets an implicit RETURN, so we get
300k jumps + 100k userchains + 100k returns -> 500k rule entries

Fixes: 36472341017529e ("netfilter: x_tables: validate targets of jumps")
Reported-by: Jeff Wu 
Tested-by: Jeff Wu 
Signed-off-by: Florian Westphal 
---
 Changes since v2:
  - get rid of unneeded 'offsets &&' clause in if-test
  - add Tested-by Tag from Jeff Wu
  - fixup some parts of commit message wrt. size of example ruleset

 include/linux/netfilter/x_tables.h |  4 +++
 net/ipv4/netfilter/ip_tables.c | 45 ++
 net/ipv6/netfilter/ip6_tables.c| 45 ++
 net/netfilter/x_tables.c   | 50 ++
 4 files changed, 102 insertions(+), 42 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h 
b/include/linux/netfilter/x_tables.h
index dc4f58a..5f968a3 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -246,6 +246,10 @@ int xt_check_entry_offsets(const void *base, const char 
*elems,
   unsigned int target_offset,
   unsigned int next_offset);
 
+unsigned int *xt_alloc_entry_offsets(unsigned int size);
+bool xt_find_jump_offset(const unsigned int *offsets,
+unsigned int target, unsigned int size);
+
 int xt_check_match(struct xt_mtchk_param *, unsigned int size, u_int8_t proto,
   bool inv_proto);
 int xt_check_target(struct xt_tgchk_param *, unsigned int size, u_int8_t proto,
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 54906e0..1c909f3 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -375,23 +375,12 @@ ipt_do_table(struct sk_buff *skb,
else return verdict;
 }
 
-static bool find_jump_target(const struct xt_table_info *t,
-const struct ipt_entry *target)
-{
-   struct ipt_entry *iter;
-
-   xt_entry_foreach(iter, t->entries, t->size) {
-if (iter == target)
-   return true;
-   }
-   return false;
-}
-
 /* Figures out from what hook each rule can be called: returns 0 if
there are loops.  Puts hook bitmask in comefrom. */
 static int
 mark_source_chains(const struct xt_table_info *newinfo,
-  unsigned int valid_hooks, void *entry0)
+  unsigned int valid_hooks, void *entry0,
+  unsigned int *offsets)
 {
unsigned int hook;
 
@@ -460,10 +449,11 @@ mark_source_chains(const struct xt_table_info *newinfo,
   XT_STANDARD_TARGET) == 0 &&
newpos >= 0) {
/* This a jump; chase it. */
+   if (!xt_find_jump_offset(offsets, 
newpos,
+
newinfo->number))
+   return 0;
e = (struct ipt_entry *)
(entry0 + newpos);
-   if (!find_jump_target(newinfo, e))
-   return 0;
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
@@ -696,6 +686,7 @@ translate_table(struct net *net, struct xt_table_info 
*newinfo, void *entry0,
const struct ipt_replace *repl)
 {
struct ipt_entry *iter;
+   unsigned int *offsets;
unsigned int i;
int ret = 0;
 
@@ -708,6 +699,9 @@ translate_table(struct net *net, struct xt_table_info 
*newinfo, void 

Re: [PATCH nf-next 2/2] netfilter: conntrack: simplify the code by using nf_conntrack_get_ht

2016-07-13 Thread Liping Zhang
Hi Florian,

At 2016-07-12 21:03:03, "Florian Westphal"  wrote:
>Liping Zhang  wrote:
>> +inline void
>> +nf_conntrack_get_ht(struct hlist_nulls_head **hash, unsigned int *hsize)
>
>Which "inline void"?  This is very unusual.
>
>I would suggest to not add it, and ...

Yes, but we can still find a very few sample codes using inline without static 
in current kernel source tree.
For example:
inline void raise_softirq_irqoff(unsigned int nr) { ... }
inline const struct nf_nat_l3proto * __nf_nat_l3proto_find(u8 family) { ... }

And from the description of the gcc's 
document(https://gcc.gnu.org/onlinedocs/gcc/Inline.html):
"When an inline function is not static, then the compiler must assume that 
there may be calls from
other source files; since a global symbol can be defined only once in any 
program, the function must
not be defined in the other source files, so the calls therein cannot be 
integrated". 

We can find that "inline void nf_conntrack_get_ht() { ... }" here will be 
integrated in nf_conntrack_core.c
and in other places will be a function call. And I test this on my x86 mechain, 
it works as expected.

>leave nf_conntrack_find alone, but convert
>
>> @@ -801,18 +799,15 @@ nf_conntrack_tuple_taken(const struct 
>> nf_conntrack_tuple *tuple,
>
>This one ..
>
>> @@ -878,14 +873,11 @@ static noinline int early_drop(struct net *net, 
>> unsigned int _hash)
>
>... and this one too.
>

IMO, leave nf_conntrack_find alone seems not very good. If you strongly 
dislike "inline void",
I can keep nf_conntrack_get_ht unchanged. But I will convert all these three to 
use nf_conntrack_get_ht.
I think even in nf_conntrack_find, inline nf_conntrack_get_ht will earn 
very very little performace
here, almost none.


Re: nftables: Dynamically updating sets gives syntax error

2016-07-13 Thread Anders K. Pedersen
On tir, 2016-07-12 at 17:22 +0200, Pablo Neira Ayuso wrote:
> On Sat, Jul 02, 2016 at 04:12:56PM +0200, Anders K. Pedersen wrote:
> > Hello,
> > 
> > On lør, 2016-06-25 at 15:30 +0200, Anders K. Pedersen wrote:
> > > With nftables 0.6 I'm getting a syntax error, when I try to use
> > > the
> > > feature that was introduced
> > > by http://git.netfilter.org/nftables/commit
> > > /?id=a9467e55973b10c2e8fe37525514c961580f8506 . For example:
> > > 
> > > # nft filter input set add tcp dport @myset
> > > :1:26-30: Error: syntax error, unexpected dport
> > > filter input set add tcp dport @myset
> > >  ^
> > > # nft filter input set add ip saddr timeout 10s @myset
> > > :1:25-29: Error: syntax error, unexpected saddr
> > > filter input set add ip saddr timeout 10s @myset
> > > ^
> > > # nft filter input set update ip saddr timeout 10s @myset
> > > :1:28-32: Error: syntax error, unexpected saddr
> > > filter input set update ip saddr timeout 10s @myset
> > >    ^
> > 
> > I did a git bisect on this and found that it was broken by
> > 
> > commit a3e60492a684be09374d0649735da42bdadc6b48
> > Author: Pablo Neira Ayuso 
> > Date:   Sun Dec 27 22:15:17 2015 +0100
> > 
> > parser: restrict relational rhs expression recursion
> > 
> > After studying it a bit I reverted a part of it with the following
> > change, which made the commands above work again:
> > 
> > --- a/src/parser_bison.y
> > +++ b/src/parser_bison.y
> > @@ -2054,7 +2054,7 @@ set_elem_option   :   TIMEO
> > UT  time_spec
> >     }
> >     ;
> >  
> > -set_lhs_expr   :   concat_rhs_expr
> > +set_lhs_expr   :   concat_expr
> >     |   multiton_rhs_expr
> >     ;
> >  
> > The commit message indicated that it intended to handle rhs
> > expressions, but this is a lhs expression, so maybe this change was
> > unintended?
> 
> Thanks for reporting. This results however in shift/reduce conflicts
> in the grammar, just sent patches to address this:
> 
> http://patchwork.ozlabs.org/patch/647448/
> http://patchwork.ozlabs.org/patch/647447/
> 
> Please, test and let me know if this works for you. Thanks.

I applied these patches on top of current nftables git, and tested that
they solve the problem. Thanks for fixing this.

Regards,
Anders K. Pedersen
--
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