Re: [PATCH nf-next] netfilter: xt_bpf: support ebpf

[Willem de Bruijn]

On Mon, Dec 5, 2016 at 7:20 PM, Florian Westphal  wrote:
> Willem de Bruijn  wrote:
>> While we're discussing the patch, another question, about revisions: I
>> tested both modified and original iptables binaries on both standard
>> and modified kernels. It all works as expected, except for the case
>> where both binaries are used on a single kernel. For instance:
>>
>>   iptables -A OUTPUT -m bpf --bytecode "`./nfbpf_compile RAW 'udp port
>> 8000'`" -j LOG
>>   ./iptables.new -L
>>
>> Here the new binary will interpret the object as xt_bpf_match_v1, but
>> iptables has inserted xt_bpf_match. The same problem happens the other
>> way around. A new binary can be made robust to detect old structs, but
>> not the other way around. Specific to bpf, the existing xt_bpf code
>> has an unfortunate bug that it always prints at least one line of
>> code, even if ->bpf_program_num_elems == 0.
>>
>> I notice that other extensions also do not necessarily only extend
>> struct vN in vN+1. Is the above a known issue?
>
> Yes, I guess noone ever bothered to fix this.
>
> The kernel blob should contain the match/target revision number,
> so userspace can in fact see that 'this is bpf v42', but iirc
> the netfilter userspace just loads the highest userspace revision
> supported by the kernel (which is then different for the 2 iptables
> binaries).

We can fall back on not parsing contents on mismatch:

diff --git a/iptables/iptables.c b/iptables/iptables.c
index 540d111..ada7c94 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -504,7 +504,8 @@ print_match(const struct xt_entry_match *m,
xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);

if (match) {
-   if (match->print)
+   if (match->print &&
+   m->u.user.revision == match->revision)
match->print(ip, m, numeric);
else
printf("%s ", match->name);

> But we *could* display message like 'kernel uses revision 2 but I can
> only find 0 and 1' or fall back to the lower supported revision without
> guess-the-struct-by-size games.

That's a good idea. A special case printf() with a notice, then.
--
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-next v2] netfilter: xt_bpf: support ebpf

[Willem de Bruijn]

From: Willem de Bruijn 

Add support for attaching an eBPF object by file descriptor.

The iptables binary can be called with a path to an elf object or a
pinned bpf object. Also pass the mode and path to the kernel to be
able to return it later for iptables dump and save.

Signed-off-by: Willem de Bruijn 

---

Changes
  v1 -> v2
- define XT_BPF_PATH_MAX (== 512: does not grow structure size)
---
 include/uapi/linux/netfilter/xt_bpf.h | 21 
 net/netfilter/xt_bpf.c| 96 +--
 2 files changed, 101 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/netfilter/xt_bpf.h 
b/include/uapi/linux/netfilter/xt_bpf.h
index 1fad2c2..b97725a 100644
--- a/include/uapi/linux/netfilter/xt_bpf.h
+++ b/include/uapi/linux/netfilter/xt_bpf.h
@@ -2,9 +2,11 @@
 #define _XT_BPF_H
 
 #include 
+#include 
 #include 
 
 #define XT_BPF_MAX_NUM_INSTR   64
+#define XT_BPF_PATH_MAX(XT_BPF_MAX_NUM_INSTR * sizeof(struct 
sock_filter))
 
 struct bpf_prog;
 
@@ -16,4 +18,23 @@ struct xt_bpf_info {
struct bpf_prog *filter __attribute__((aligned(8)));
 };
 
+enum xt_bpf_modes {
+   XT_BPF_MODE_BYTECODE,
+   XT_BPF_MODE_FD_PINNED,
+   XT_BPF_MODE_FD_ELF,
+};
+
+struct xt_bpf_info_v1 {
+   __u16 mode;
+   __u16 bpf_program_num_elem;
+   __s32 fd;
+   union {
+   struct sock_filter bpf_program[XT_BPF_MAX_NUM_INSTR];
+   char path[XT_BPF_PATH_MAX];
+   };
+
+   /* only used in the kernel */
+   struct bpf_prog *filter __attribute__((aligned(8)));
+};
+
 #endif /*_XT_BPF_H */
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index dffee9d47..2dedaa2 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -20,15 +21,15 @@ MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_bpf");
 MODULE_ALIAS("ip6t_bpf");
 
-static int bpf_mt_check(const struct xt_mtchk_param *par)
+static int __bpf_mt_check_bytecode(struct sock_filter *insns, __u16 len,
+  struct bpf_prog **ret)
 {
-   struct xt_bpf_info *info = par->matchinfo;
struct sock_fprog_kern program;
 
-   program.len = info->bpf_program_num_elem;
-   program.filter = info->bpf_program;
+   program.len = len;
+   program.filter = insns;
 
-   if (bpf_prog_create(>filter, )) {
+   if (bpf_prog_create(ret, )) {
pr_info("bpf: check failed: parse error\n");
return -EINVAL;
}
@@ -36,6 +37,42 @@ static int bpf_mt_check(const struct xt_mtchk_param *par)
return 0;
 }
 
+static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret)
+{
+   struct bpf_prog *prog;
+
+   prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_SOCKET_FILTER);
+   if (IS_ERR(prog))
+   return PTR_ERR(prog);
+
+   *ret = prog;
+   return 0;
+}
+
+static int bpf_mt_check(const struct xt_mtchk_param *par)
+{
+   struct xt_bpf_info *info = par->matchinfo;
+
+   return __bpf_mt_check_bytecode(info->bpf_program,
+  info->bpf_program_num_elem,
+  >filter);
+}
+
+static int bpf_mt_check_v1(const struct xt_mtchk_param *par)
+{
+   struct xt_bpf_info_v1 *info = par->matchinfo;
+
+   if (info->mode == XT_BPF_MODE_BYTECODE)
+   return __bpf_mt_check_bytecode(info->bpf_program,
+  info->bpf_program_num_elem,
+  >filter);
+   else if (info->mode == XT_BPF_MODE_FD_PINNED ||
+info->mode == XT_BPF_MODE_FD_ELF)
+   return __bpf_mt_check_fd(info->fd, >filter);
+   else
+   return -EINVAL;
+}
+
 static bool bpf_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
const struct xt_bpf_info *info = par->matchinfo;
@@ -43,31 +80,58 @@ static bool bpf_mt(const struct sk_buff *skb, struct 
xt_action_param *par)
return BPF_PROG_RUN(info->filter, skb);
 }
 
+static bool bpf_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
+{
+   const struct xt_bpf_info_v1 *info = par->matchinfo;
+
+   return !!bpf_prog_run_save_cb(info->filter, (struct sk_buff *) skb);
+}
+
 static void bpf_mt_destroy(const struct xt_mtdtor_param *par)
 {
const struct xt_bpf_info *info = par->matchinfo;
+
+   bpf_prog_destroy(info->filter);
+}
+
+static void bpf_mt_destroy_v1(const struct xt_mtdtor_param *par)
+{
+   const struct xt_bpf_info_v1 *info = par->matchinfo;
+
bpf_prog_destroy(info->filter);
 }
 
-static struct xt_match bpf_mt_reg __read_mostly = {
-   .name   = "bpf",
-   .revision   = 0,
-   .family = NFPROTO_UNSPEC,
-   .checkentry = bpf_mt_check,
-   .match  = bpf_mt,
-   .destroy= 

Rebasing nf-next

[Pablo Neira Ayuso]

Hi,

In order to reduce the (already large) batch that I'm going to pass to
David, I'm rebasing nf-next to squash patches that fix bugs,
compilation warnings and other fallout from ongoing development. List
of patches that I have squashed into master patch are:

* netfilter: nf_tables: silence gcc warning with stateful object maps
* netfilter: nf_tables: restore check for NFTA_SET_ELEM_LIST_ELEMENTS
* netfilter: nft_counter: move counter reset into separated function
* netfilter: nft_quota: don't read quota twice on reset
* netfilter: add list element test to br_netfilter_hooks
* netfilter: fix build failure with CONNTRACK=n NF_DEFRAG=y
* netfilter: nft_payload: restrict l4 checksum updates to l3 header mangling

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

[nf-next:master 18/48] net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:94:9: error: 'struct net' has no member named 'ct'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
head:   34d360415a921406e622f60e202892af8cf4c57b
commit: 018914b2c913bcc9c571ae5a781280c76a15ddad [18/48] netfilter: defrag: 
only register defrag functionality if needed
config: x86_64-randconfig-h0-12062249 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 018914b2c913bcc9c571ae5a781280c76a15ddad
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the nf-next/master HEAD 34d360415a921406e622f60e202892af8cf4c57b builds 
fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c: In function 'defrag6_net_exit':
>> net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:94:9: error: 'struct net' has no 
>> member named 'ct'
 if (net->ct.defrag_ipv6) {
^
   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:97:6: error: 'struct net' has no 
member named 'ct'
  net->ct.defrag_ipv6 = false;
 ^
   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c: In function 
'nf_defrag_ipv6_enable':
   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:139:9: error: 'struct net' has no 
member named 'ct'
 if (net->ct.defrag_ipv6)
^
   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:143:9: error: 'struct net' has no 
member named 'ct'
 if (net->ct.defrag_ipv6)
^
   net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:149:6: error: 'struct net' has no 
member named 'ct'
  net->ct.defrag_ipv6 = true;
 ^

vim +94 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c

88  .priority   = NF_IP6_PRI_CONNTRACK_DEFRAG,
89  },
90  };
91  
92  static void __net_exit defrag6_net_exit(struct net *net)
93  {
  > 94  if (net->ct.defrag_ipv6) {
95  nf_unregister_net_hooks(net, ipv6_defrag_ops,
96  ARRAY_SIZE(ipv6_defrag_ops));
97  net->ct.defrag_ipv6 = false;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH nf-next] netfilter: add list element test to br_netfilter_hooks

[Aaron Conole]

The for-loop in the bridge hook entries assumes that the elements are
always present.  However, this assumption may not always be true.

Fixes: 66cfc1dd07c7 ("netfilter: convert while loops to for loops")
Signed-off-by: Aaron Conole 
--
Pablo, if possible could this be squashed into the commit instead?  I
only did a build test of this, but it should be correct.

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index fbe35b4..b12501a 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -1009,7 +1009,7 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net,
int ret;
 
for (elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
-nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF;
+elem && nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF;
 elem = rcu_dereference(elem->next))
;

--
2.5.0
--
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-next] netfilter: nft_quota: don't read quota twice on reset

[Pablo Neira Ayuso]

The quota is already fetched before reset, so there is no need to
re-fetch it, otherwise userspace reports consumed quota is always zero
on reset. This problem was likely introduced when rebasing this
patchset.

Signed-off-by: Pablo Neira Ayuso 
---
 net/netfilter/nft_quota.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/netfilter/nft_quota.c b/net/netfilter/nft_quota.c
index 7e825e7f6210..2cfcdd1d4d45 100644
--- a/net/netfilter/nft_quota.c
+++ b/net/netfilter/nft_quota.c
@@ -112,7 +112,6 @@ static int nft_quota_do_dump(struct sk_buff *skb, struct 
nft_quota *priv,
consumed = atomic64_read(>consumed);
}
 
-   consumed = atomic64_read(>consumed);
/* Since we inconditionally increment consumed quota for each packet
 * that we see, don't go over the quota boundary in what we send to
 * userspace.
-- 
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 nf-next 1/2] netfilter: nf_tables: restore check for NFTA_SET_ELEM_LIST_ELEMENTS

[Pablo Neira Ayuso]

It seems git rebase and branch rmerge resulted patching the wrong spot,
restore check when adding elements, remove it from the deletion path so
flushing sets still works. The original patch applying this chunk in the
right spot: http://patchwork.ozlabs.org/patch/702919/.

Fixes: 34d360415a92 ("netfilter: nf_tables: support for set flushing")
Signed-off-by: Pablo Neira Ayuso 
---
 net/netfilter/nf_tables_api.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9ead6a7514c3..a019a87e58ee 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3767,6 +3767,9 @@ static int nf_tables_newsetelem(struct net *net, struct 
sock *nlsk,
struct nft_ctx ctx;
int rem, err = 0;
 
+   if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL)
+   return -EINVAL;
+
err = nft_ctx_init_from_elemattr(, net, skb, nlh, nla, genmask);
if (err < 0)
return err;
@@ -3917,9 +3920,6 @@ static int nf_tables_delsetelem(struct net *net, struct 
sock *nlsk,
struct nft_ctx ctx;
int rem, err = 0;
 
-   if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL)
-   return -EINVAL;
-
err = nft_ctx_init_from_elemattr(, net, skb, nlh, nla, genmask);
if (err < 0)
return err;
-- 
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 nf-next 2/2] netfilter: nft_counter: move counter reset into separated function

[Pablo Neira Ayuso]

This patch moves the reset path from nft_counter_fetch() to
nft_counter_reset(), this patch aims to solve gcc compilation warning:

   net/netfilter/nft_counter.c: In function 'nft_counter_fetch':
>> net/netfilter/nft_counter.c:128:18: warning: 'packets' may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
  total->packets += packets;
 ^~
>> net/netfilter/nft_counter.c:129:16: warning: 'bytes' may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
  total->bytes += bytes;
   ^~

Reported-by: kbuild test robot 
Signed-off-by: Pablo Neira Ayuso 
---
 net/netfilter/nft_counter.c | 43 +++
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
index 90e42140ee7b..3d2ff23be8d6 100644
--- a/net/netfilter/nft_counter.c
+++ b/net/netfilter/nft_counter.c
@@ -101,7 +101,7 @@ static void nft_counter_obj_destroy(struct nft_object *obj)
 }
 
 static void nft_counter_fetch(struct nft_counter_percpu __percpu *counter,
- struct nft_counter *total, bool reset)
+ struct nft_counter *total)
 {
struct nft_counter_percpu *cpu_stats;
u64 bytes, packets;
@@ -110,19 +110,35 @@ static void nft_counter_fetch(struct nft_counter_percpu 
__percpu *counter,
 
memset(total, 0, sizeof(*total));
for_each_possible_cpu(cpu) {
-   if (reset)
-   bytes = packets = 0;
+   cpu_stats = per_cpu_ptr(counter, cpu);
+   do {
+   seq = u64_stats_fetch_begin_irq(_stats->syncp);
+   bytes   = cpu_stats->counter.bytes;
+   packets = cpu_stats->counter.packets;
+   } while (u64_stats_fetch_retry_irq(_stats->syncp, seq));
+
+   total->packets += packets;
+   total->bytes += bytes;
+   }
+}
+
+static void nft_counter_reset(struct nft_counter_percpu __percpu *counter,
+ struct nft_counter *total)
+{
+   struct nft_counter_percpu *cpu_stats;
+   u64 bytes, packets;
+   unsigned int seq;
+   int cpu;
+
+   memset(total, 0, sizeof(*total));
+   for_each_possible_cpu(cpu) {
+   bytes = packets = 0;
 
cpu_stats = per_cpu_ptr(counter, cpu);
do {
seq = u64_stats_fetch_begin_irq(_stats->syncp);
-   if (reset) {
-   packets += xchg(_stats->counter.packets, 0);
-   bytes   += xchg(_stats->counter.bytes, 0);
-   } else {
-   bytes   = cpu_stats->counter.bytes;
-   packets = cpu_stats->counter.packets;
-   }
+   packets += xchg(_stats->counter.packets, 0);
+   bytes   += xchg(_stats->counter.bytes, 0);
} while (u64_stats_fetch_retry_irq(_stats->syncp, seq));
 
total->packets += packets;
@@ -136,7 +152,10 @@ static int nft_counter_do_dump(struct sk_buff *skb,
 {
struct nft_counter total;
 
-   nft_counter_fetch(priv->counter, , reset);
+   if (reset)
+   nft_counter_reset(priv->counter, );
+   else
+   nft_counter_fetch(priv->counter, );
 
if (nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes),
 NFTA_COUNTER_PAD) ||
@@ -215,7 +234,7 @@ static int nft_counter_clone(struct nft_expr *dst, const 
struct nft_expr *src)
struct nft_counter_percpu *this_cpu;
struct nft_counter total;
 
-   nft_counter_fetch(priv->counter, , false);
+   nft_counter_fetch(priv->counter, );
 
cpu_stats = __netdev_alloc_pcpu_stats(struct nft_counter_percpu,
  GFP_ATOMIC);
-- 
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

[bug report] netfilter: convert while loops to for loops

[Dan Carpenter]

Hello Aaron Conole,

This is a semi-automatic email about new static checker warnings.

The patch 66cfc1dd07c7: "netfilter: convert while loops to for loops" 
from Nov 15, 2016, leads to the following Smatch complaint:

net/bridge/br_netfilter_hooks.c:1016 br_nf_hook_thresh()
 warn: variable dereferenced before check 'elem' (see line 1012)

net/bridge/br_netfilter_hooks.c
  1011  for (elem = 
rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
  1012   nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF;
 
Dereference inside function.

  1013   elem = rcu_dereference(elem->next))
  1014  ;
  1015  
  1016  if (!elem)
 
This can't be reached without already dereferencing "elem".

  1017  return okfn(net, sk, skb);
  1018  

regards,
dan carpenter
--
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

[nf-next:master 37/48] net/netfilter/nft_counter.c:128:18: warning: 'packets' may be used uninitialized in this function

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
head:   34d360415a921406e622f60e202892af8cf4c57b
commit: 9fbf2fcf1734cbcd01ea3818242b6ed0c4d32869 [37/48] netfilter: nf_tables: 
atomic dump and reset for stateful objects
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 9fbf2fcf1734cbcd01ea3818242b6ed0c4d32869
# save the attached .config to linux build tree
make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   net/netfilter/nft_counter.c: In function 'nft_counter_fetch':
>> net/netfilter/nft_counter.c:128:18: warning: 'packets' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
  total->packets += packets;
 ^~
>> net/netfilter/nft_counter.c:129:16: warning: 'bytes' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
  total->bytes += bytes;
   ^~
   In file included from arch/x86/include/asm/atomic.h:7:0,
from arch/x86/include/asm/msr.h:66,
from arch/x86/include/asm/processor.h:20,
from arch/x86/include/asm/cpufeature.h:4,
from arch/x86/include/asm/thread_info.h:52,
from include/linux/thread_info.h:58,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:59,
from include/linux/spinlock.h:50,
from include/linux/seqlock.h:35,
from include/linux/time.h:5,
from include/linux/stat.h:18,
from include/linux/module.h:10,
from net/netfilter/nft_counter.c:13:
>> arch/x86/include/asm/cmpxchg.h:66:4: error: call to '__xchg_wrong_size' 
>> declared with attribute error: Bad argument size for xchg
   __ ## op ## _wrong_size();   \
   ^
   arch/x86/include/asm/cmpxchg.h:77:22: note: in expansion of macro '__xchg_op'
#define xchg(ptr, v) __xchg_op((ptr), (v), xchg, "")
 ^
>> net/netfilter/nft_counter.c:120:16: note: in expansion of macro 'xchg'
packets += xchg(_stats->counter.packets, 0);
   ^~~~
>> arch/x86/include/asm/cmpxchg.h:66:4: error: call to '__xchg_wrong_size' 
>> declared with attribute error: Bad argument size for xchg
   __ ## op ## _wrong_size();   \
   ^
   arch/x86/include/asm/cmpxchg.h:77:22: note: in expansion of macro '__xchg_op'
#define xchg(ptr, v) __xchg_op((ptr), (v), xchg, "")
 ^
   net/netfilter/nft_counter.c:121:14: note: in expansion of macro 'xchg'
bytes += xchg(_stats->counter.bytes, 0);
 ^~~~

vim +/packets +128 net/netfilter/nft_counter.c

96518518 Patrick McHardy   2013-10-147   *
96518518 Patrick McHardy   2013-10-148   * Development of this code funded 
by Astaro AG (http://www.astaro.com/)
96518518 Patrick McHardy   2013-10-149   */
96518518 Patrick McHardy   2013-10-14   10  
96518518 Patrick McHardy   2013-10-14   11  #include 
96518518 Patrick McHardy   2013-10-14   12  #include 
96518518 Patrick McHardy   2013-10-14  @13  #include 
96518518 Patrick McHardy   2013-10-14   14  #include 
96518518 Patrick McHardy   2013-10-14   15  #include 
96518518 Patrick McHardy   2013-10-14   16  #include 
96518518 Patrick McHardy   2013-10-14   17  #include 

96518518 Patrick McHardy   2013-10-14   18  #include 
96518518 Patrick McHardy   2013-10-14   19  
96518518 Patrick McHardy   2013-10-14   20  struct nft_counter {
96518518 Patrick McHardy   2013-10-14   21  u64 bytes;
96518518 Patrick McHardy   2013-10-14   22  u64 packets;
96518518 Patrick McHardy   2013-10-14   23  };
96518518 Patrick McHardy   2013-10-14   24  
0c45e769 Pablo Neira Ayuso 2015-06-08   25  struct nft_counter_percpu {
0c45e769 Pablo Neira Ayuso 2015-06-08   26  struct nft_counter  counter;
0c45e769 Pablo Neira Ayuso 2015-06-08   27  struct u64_stats_sync   syncp;
0c45e769 Pablo Neira Ayuso 2015-06-08   28  };
0c45e769 Pablo Neira Ayuso 2015-06-08   29  
0c45e769 Pablo Neira Ayuso 2015-06-08   30  struct nft_counter_percpu_priv {
0c45e769 Pablo Neira Ayuso 2015-06-08   31  struct nft_counter_percpu 
__percpu *counter;
0c45e769 Pablo Neira Ayuso 2015-06-08   32  };
0c45e769 Pablo Neira Ayuso 2015-06-08   33  
225b8725 Pablo Neira Ayuso 2016-11-28   34  static inline void 
nft_counter_do_eval(struct nft_counter_percpu_priv *priv,
a55e22e9 Patrick McHardy   2015-04-11   35 
struct nft_regs *regs,
96518518 Patrick McHardy   2013-10-14   36 
const struct nft_pktinfo *pkt)
96518518 Patrick McHardy   2013-10-14   37  {
0c45e769 Pablo Neira Ayuso 2015-06-08   38  struct nft_counter_percpu 
*this_cpu;
0c45e769 Pablo Neira Ayuso 

Re: [bug report] netfilter: nft_payload: layer 4 checksum adjustment for pseudoheader fields

[Pablo Neira Ayuso]

On Tue, Dec 06, 2016 at 03:24:53PM +0300, Dan Carpenter wrote:
> On Tue, Dec 06, 2016 at 01:16:08PM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Dec 06, 2016 at 02:57:34PM +0300, Dan Carpenter wrote:
> > > Hello Pablo Neira Ayuso,
> > > 
> > > The patch 556c291b3a1b: "netfilter: nft_payload: layer 4 checksum
> > > adjustment for pseudoheader fields" from Nov 24, 2016, leads to the
> > > following static checker warning:
> > > 
> > >   net/netfilter/nft_payload.c:301 nft_payload_set_eval()
> > >   error: uninitialized symbol 'fsum'.
> > 
> > This should restrict this case you're reporting here:
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/commit/?id=a3c91a548b9b9f05f81e7bfe7fec3544a376269a
> > 
> > Otherwise let me know, thanks.
> 
> That works...  Seems like it would have been easier to just move it
> under the if statement though...

Right. Will revisit this, 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 nf-next,v2] netfilter: nf_tables: silence gcc warning with stateful object maps

[Pablo Neira Ayuso]

Not a problem in practise since objtype is ignored if the NFT_SET_OBJECT
flag is not set.

But call down gcc warnings:

   net/netfilter/nf_tables_api.c: In function 'nf_tables_newset':
>> net/netfilter/nf_tables_api.c:3003:15: warning: 'objtype' may be used
+uninitialized in this function

Reported-by: kbuild test robot 
Signed-off-by: Pablo Neira Ayuso 
---
v2: should be NFT_OBJECT_UNSPEC, not NFT_OBJ_UNSPEC

 net/netfilter/nf_tables_api.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 799eaa6808b9..9ead6a7514c3 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2920,6 +2920,8 @@ static int nf_tables_newset(struct net *net, struct sock 
*nlsk,
return -EINVAL;
} else if (flags & NFT_SET_OBJECT)
return -EINVAL;
+   else
+   objtype = NFT_OBJECT_UNSPEC;
 
timeout = 0;
if (nla[NFTA_SET_TIMEOUT] != NULL) {
-- 
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 nf-next] netfilter: nf_tables: silence gcc warning with stateful object maps

[Pablo Neira Ayuso]

Not a problem in practise since objtype is ignored if the NFT_SET_OBJECT
flag is not set.

But call down gcc warnings:

   net/netfilter/nf_tables_api.c: In function 'nf_tables_newset':
>> net/netfilter/nf_tables_api.c:3003:15: warning: 'objtype' may be used
+uninitialized in this function

Signed-off-by: Pablo Neira Ayuso 
---
 net/netfilter/nf_tables_api.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 799eaa6808b9..789229663028 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2920,6 +2920,8 @@ static int nf_tables_newset(struct net *net, struct sock 
*nlsk,
return -EINVAL;
} else if (flags & NFT_SET_OBJECT)
return -EINVAL;
+   else
+   objtype = NFT_OBJ_UNSPEC;
 
timeout = 0;
if (nla[NFTA_SET_TIMEOUT] != NULL) {
-- 
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: [bug report] netfilter: nft_payload: layer 4 checksum adjustment for pseudoheader fields

[Dan Carpenter]

On Tue, Dec 06, 2016 at 01:16:08PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Dec 06, 2016 at 02:57:34PM +0300, Dan Carpenter wrote:
> > Hello Pablo Neira Ayuso,
> > 
> > The patch 556c291b3a1b: "netfilter: nft_payload: layer 4 checksum
> > adjustment for pseudoheader fields" from Nov 24, 2016, leads to the
> > following static checker warning:
> > 
> > net/netfilter/nft_payload.c:301 nft_payload_set_eval()
> > error: uninitialized symbol 'fsum'.
> 
> This should restrict this case you're reporting here:
> 
> http://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/commit/?id=a3c91a548b9b9f05f81e7bfe7fec3544a376269a
> 
> Otherwise let me know, thanks.

That works...  Seems like it would have been easier to just move it
under the if statement though...

regards,
dan carpenter

--
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 v2] netfilter: avoid warn and OOM killer on vmalloc call

[Pablo Neira Ayuso]

On Fri, Dec 02, 2016 at 07:46:38AM -0200, Marcelo Ricardo Leitner wrote:
> Andrey Konovalov reported that this vmalloc call is based on an
> userspace request and that it's spewing traces, which may flood the logs
> and cause DoS if abused.
> 
> Florian Westphal also mentioned that this call should not trigger OOM
> killer.
> 
> This patch brings the vmalloc call in sync to kmalloc and disables the
> warn trace on allocation failure and also disable OOM killer invocation.
> 
> Note, however, that under such stress situation, other places may
> trigger OOM killer invocation.

Unless anyone has an objection, I'm going to place this in nf-next.

Thanks.

> Reported-by: Andrey Konovalov 
> Cc: Florian Westphal 
> Signed-off-by: Marcelo Ricardo Leitner 
> ---
>  net/netfilter/x_tables.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 
> fc4977456c30e098197b4f987b758072c9cf60d9..dece525bf83a0098dad607fce665cd0bde228362
>  100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -958,7 +958,9 @@ struct xt_table_info *xt_alloc_table_info(unsigned int 
> size)
>   if (sz <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
>   info = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
>   if (!info) {
> - info = vmalloc(sz);
> + info = __vmalloc(sz, GFP_KERNEL | __GFP_NOWARN |
> +  __GFP_NORETRY | __GFP_HIGHMEM,
> +  PAGE_KERNEL);
>   if (!info)
>   return NULL;
>   }
> -- 
> 2.9.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

[nf-next:master 41/48] net/netfilter/nf_tables_api.c:3003:15: warning: 'objtype' may be used uninitialized in this function

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
head:   34d360415a921406e622f60e202892af8cf4c57b
commit: bbf56b9c79b71b247caa9188998cd54b084445c1 [41/48] netfilter: nf_tables: 
add stateful object reference to set elements
config: x86_64-randconfig-x014-201649 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout bbf56b9c79b71b247caa9188998cd54b084445c1
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/netfilter/nf_tables_api.c: In function 'nf_tables_newset':
>> net/netfilter/nf_tables_api.c:3003:15: warning: 'objtype' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
 set->objtype = objtype;
 ~^

vim +/objtype +3003 net/netfilter/nf_tables_api.c

  2987  nla_strlcpy(name, nla[NFTA_SET_NAME], sizeof(set->name));
  2988  err = nf_tables_set_alloc_name(, set, name);
  2989  if (err < 0)
  2990  goto err2;
  2991  
  2992  udata = NULL;
  2993  if (udlen) {
  2994  udata = set->data + size;
  2995  nla_memcpy(udata, nla[NFTA_SET_USERDATA], udlen);
  2996  }
  2997  
  2998  INIT_LIST_HEAD(>bindings);
  2999  set->ops   = ops;
  3000  set->ktype = ktype;
  3001  set->klen  = desc.klen;
  3002  set->dtype = dtype;
> 3003  set->objtype = objtype;
  3004  set->dlen  = desc.dlen;
  3005  set->flags = flags;
  3006  set->size  = desc.size;
  3007  set->policy = policy;
  3008  set->udlen  = udlen;
  3009  set->udata  = udata;
  3010  set->timeout = timeout;
  3011  set->gc_int = gc_int;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [bug report] netfilter: nft_payload: layer 4 checksum adjustment for pseudoheader fields

[Pablo Neira Ayuso]

On Tue, Dec 06, 2016 at 02:57:34PM +0300, Dan Carpenter wrote:
> Hello Pablo Neira Ayuso,
> 
> The patch 556c291b3a1b: "netfilter: nft_payload: layer 4 checksum
> adjustment for pseudoheader fields" from Nov 24, 2016, leads to the
> following static checker warning:
> 
>   net/netfilter/nft_payload.c:301 nft_payload_set_eval()
>   error: uninitialized symbol 'fsum'.

This should restrict this case you're reporting here:

http://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/commit/?id=a3c91a548b9b9f05f81e7bfe7fec3544a376269a

Otherwise let me know, 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

[bug report] netfilter: nft_payload: layer 4 checksum adjustment for pseudoheader fields

[Dan Carpenter]

Hello Pablo Neira Ayuso,

The patch 556c291b3a1b: "netfilter: nft_payload: layer 4 checksum
adjustment for pseudoheader fields" from Nov 24, 2016, leads to the
following static checker warning:

net/netfilter/nft_payload.c:301 nft_payload_set_eval()
error: uninitialized symbol 'fsum'.

net/netfilter/nft_payload.c
   253  static void nft_payload_set_eval(const struct nft_expr *expr,
   254   struct nft_regs *regs,
   255   const struct nft_pktinfo *pkt)
   256  {
   257  const struct nft_payload_set *priv = nft_expr_priv(expr);
   258  struct sk_buff *skb = pkt->skb;
   259  const u32 *src = >data[priv->sreg];
   260  int offset, csum_offset;
   261  __wsum fsum, tsum;
   262  __sum16 sum;
   263  
   264  switch (priv->base) {
   265  case NFT_PAYLOAD_LL_HEADER:
   266  if (!skb_mac_header_was_set(skb))
   267  goto err;
   268  offset = skb_mac_header(skb) - skb->data;
   269  break;
   270  case NFT_PAYLOAD_NETWORK_HEADER:
   271  offset = skb_network_offset(skb);
   272  break;
   273  case NFT_PAYLOAD_TRANSPORT_HEADER:
   274  if (!pkt->tprot_set)
   275  goto err;
   276  offset = pkt->xt.thoff;
   277  break;
   278  default:
   279  BUG();
   280  }
   281  
   282  csum_offset = offset + priv->csum_offset;
   283  offset += priv->offset;
   284  
   285  if (priv->csum_type == NFT_PAYLOAD_CSUM_INET &&
   286  (priv->base != NFT_PAYLOAD_TRANSPORT_HEADER ||
   287   skb->ip_summed != CHECKSUM_PARTIAL)) {
   288  if (skb_copy_bits(skb, csum_offset, , sizeof(sum)) 
< 0)
   289  goto err;
   290  
   291  fsum = skb_checksum(skb, offset, priv->len, 0);

fsum is only set inside this if statement.

   292  tsum = csum_partial(src, priv->len, 0);
   293  nft_csum_replace(, fsum, tsum);
   294  
   295  if (!skb_make_writable(skb, csum_offset + sizeof(sum)) 
||
   296  skb_store_bits(skb, csum_offset, , sizeof(sum)) 
< 0)
   297  goto err;
   298  }
   299  
   300  if (priv->csum_flags &&
   301  nft_payload_l4csum_update(pkt, skb, fsum, tsum) < 0)

but we use it here.  I don't know for sure this is a bug...

   302  goto err;
   303  
   304  if (!skb_make_writable(skb, max(offset + priv->len, 0)) ||
   305  skb_store_bits(skb, offset, src, priv->len) < 0)
   306  goto err;
   307  
   308  return;
   309  err:
   310  regs->verdict.code = NFT_BREAK;
   311  }

regards,
dan carpenter
--
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] src: add support to flush sets

[Anatole Denis]

On lun., déc. 5, 2016 at 11:37 , Pablo Neira Ayuso 
 wrote:

You can use this new command to remove all existing elements in a set:

 # nft flush set filter xyz

After this command, the set 'xyz' in table 'filter' becomes empty.

Signed-off-by: Pablo Neira Ayuso 
---
 include/netlink.h | 2 ++
 src/evaluate.c| 3 +++
 src/netlink.c | 9 -
 src/rule.c| 3 +++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/netlink.h b/include/netlink.h
index 28c11f603ed2..363b5251968f 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -165,6 +165,8 @@ extern int netlink_delete_setelems(struct 
netlink_ctx *ctx, const struct handle

   const struct expr *expr);
 extern int netlink_get_setelems(struct netlink_ctx *ctx, const 
struct handle *h,

const struct location *loc, struct set *set);
+extern int netlink_flush_setelems(struct netlink_ctx *ctx, const 
struct handle *h,

+ const struct location *loc);

 extern void netlink_dump_table(const struct nftnl_table *nlt);
 extern void netlink_dump_chain(const struct nftnl_chain *nlc);
diff --git a/src/evaluate.c b/src/evaluate.c
index e11a455a5f53..8a3da54e5b2d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2857,9 +2857,11 @@ static int cmd_evaluate_list(struct eval_ctx 
*ctx, struct cmd *cmd)

 static int cmd_evaluate_flush(struct eval_ctx *ctx, struct cmd *cmd)
 {
int ret;
+
ret = cache_update(cmd->op, ctx->msgs);
if (ret < 0)
return ret;
+
switch (cmd->obj) {
case CMD_OBJ_RULESET:
cache_flush();
@@ -2870,6 +2872,7 @@ static int cmd_evaluate_flush(struct eval_ctx 
*ctx, struct cmd *cmd)

 */
case CMD_OBJ_CHAIN:
/* Chains don't hold sets */
+   case CMD_OBJ_SET:
break;


I think you need to empty the cache for said set here, otherwise this 
will lead to the same errors we had earlier with flush ruleset not 
emptying the set cache.


Example test which fails:
Running
```
table t{
 set s { type ipv4_addr; flags interval
   elements={127.0.0.1/8}
 }
}
```
then:
```
flush set t s
add element t s {
 127.0.0.1/8,
}
```
errors out with:
Error: interval overlaps with an existing one
 127.0.0.1/8,
 ^^^

[...]

Maybe a question for another patch but: if there is support for 
emptying specific sets, maybe flushing a table should empty sets in the 
table as well as the chains ? (currently it only empties the chains and 
leaves the sets intact, which is kind of unintuitive)




--
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] src: add support to flush sets

[Pablo Neira Ayuso]

On Tue, Dec 06, 2016 at 12:13:37PM +0100, Anatole Denis wrote:
> On lun., déc. 5, 2016 at 11:37 , Pablo Neira Ayuso 
> wrote:
> >You can use this new command to remove all existing elements in a set:
> >
> > # nft flush set filter xyz
> >
> >After this command, the set 'xyz' in table 'filter' becomes empty.
> >
> >Signed-off-by: Pablo Neira Ayuso 
> >---
> > include/netlink.h | 2 ++
> > src/evaluate.c| 3 +++
> > src/netlink.c | 9 -
> > src/rule.c| 3 +++
> > 4 files changed, 16 insertions(+), 1 deletion(-)
> >
> >diff --git a/include/netlink.h b/include/netlink.h
> >index 28c11f603ed2..363b5251968f 100644
> >--- a/include/netlink.h
> >+++ b/include/netlink.h
> >@@ -165,6 +165,8 @@ extern int netlink_delete_setelems(struct netlink_ctx
> >*ctx, const struct handle
> >const struct expr *expr);
> > extern int netlink_get_setelems(struct netlink_ctx *ctx, const struct
> >handle *h,
> > const struct location *loc, struct set *set);
> >+extern int netlink_flush_setelems(struct netlink_ctx *ctx, const struct
> >handle *h,
> >+  const struct location *loc);
> >
> > extern void netlink_dump_table(const struct nftnl_table *nlt);
> > extern void netlink_dump_chain(const struct nftnl_chain *nlc);
> >diff --git a/src/evaluate.c b/src/evaluate.c
> >index e11a455a5f53..8a3da54e5b2d 100644
> >--- a/src/evaluate.c
> >+++ b/src/evaluate.c
> >@@ -2857,9 +2857,11 @@ static int cmd_evaluate_list(struct eval_ctx *ctx,
> >struct cmd *cmd)
> > static int cmd_evaluate_flush(struct eval_ctx *ctx, struct cmd *cmd)
> > {
> > int ret;
> >+
> > ret = cache_update(cmd->op, ctx->msgs);
> > if (ret < 0)
> > return ret;
> >+
> > switch (cmd->obj) {
> > case CMD_OBJ_RULESET:
> > cache_flush();
> >@@ -2870,6 +2872,7 @@ static int cmd_evaluate_flush(struct eval_ctx *ctx,
> >struct cmd *cmd)
> >  */
> > case CMD_OBJ_CHAIN:
> > /* Chains don't hold sets */
> >+case CMD_OBJ_SET:
> > break;
> 
> I think you need to empty the cache for said set here, otherwise this will
> lead to the same errors we had earlier with flush ruleset not emptying the
> set cache.

Thanks for spotting this, I'd appreciate if you send me a follow up
patch that I can apply to fix this.

> Example test which fails:
> Running
> ```
> table t{
>  set s { type ipv4_addr; flags interval
>elements={127.0.0.1/8}
>  }
> }
> ```
> then:
> ```
> flush set t s
> add element t s {
>  127.0.0.1/8,
> }
> ```
> errors out with:
> Error: interval overlaps with an existing one
>  127.0.0.1/8,
>  ^^^
> 
> [...]
> 
> Maybe a question for another patch but: if there is support for emptying
> specific sets, maybe flushing a table should empty sets in the table as well
> as the chains ? (currently it only empties the chains and leaves the sets
> intact, which is kind of unintuitive)

That's a good point.

If we follow this approach, main problem is that set flushing is going
to be supported by recent kernels, while older don't. So if nft flush
table also flush sets, then we'll have people experiencing different
behaviours, which is not good.

So I would prefer an explicit 'nft flush sets' to empty elements from
all sets on top of this new 'nft flush set x y', and we document that
table flushing only empties rules.
--
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 nf-next] NAT: skip checksum on offload SCTP packets

[Pablo Neira Ayuso]

On Mon, Dec 05, 2016 at 03:33:57PM +0100, Davide Caratti wrote:
> SCTP GSO and hardware can do CRC32c computation after netfilter processing,
> so we can avoid calling sctp_compute_checksum() on skb if skb->ip_summed
> is equal to CHECKSUM_PARTIAL. Moreover, set skb->ip_summed to CHECKSUM_NONE
> when the NAT code computes the CRC, to prevent offloaders from computing
> it again (on ixgbe this resulted in a transmission with wrong L4 checksum).

Applied, thanks Davide.
--
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 nf-next 1/2] netfilter: conntrack: merge udp and udplite helpers

[Pablo Neira Ayuso]

On Fri, Dec 02, 2016 at 07:50:36PM +0100, Florian Westphal wrote:
[...]
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 44410d30d461..9c34c2cabd76 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -169,16 +169,6 @@ config NF_CT_PROTO_SCTP
> If you want to compile it as a module, say M here and read
> .  If unsure, say `N'.
>  
> -config NF_CT_PROTO_UDPLITE
> - tristate 'UDP-Lite protocol connection tracking support'
> - depends on NETFILTER_ADVANCED
> - help
> -   With this option enabled, the layer 3 independent connection
> -   tracking code will be able to do state tracking on UDP-Lite
> -   connections.
> -
> -   To compile it as a module, choose M here.  If unsure, say N.
> -

Better keep this so someone explicitly willing to keep this out for
some reason can still do it, and we keep this inlined with other
protocols.
--
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 nf-next] netfilter: rpfilter: bypass ipv4 lbcast packets with zeronet source

[Pablo Neira Ayuso]

On Sat, Dec 03, 2016 at 09:25:08PM +0800, Liping Zhang wrote:
> From: Liping Zhang 
> 
> Otherwise, DHCP Discover packets(0.0.0.0->255.255.255.255) may be
> dropped incorrectly.

Applied, thanks Liping.
--
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 nf-next 1/2] netfilter: conntrack: merge udp and udplite helpers

[Pablo Neira Ayuso]

On Sat, Dec 03, 2016 at 02:02:10PM +0100, Florian Westphal wrote:
> kbuild test robot  wrote:
> > All error/warnings (new ones prefixed by >>):
> > 
> >In file included from include/uapi/linux/stddef.h:1:0,
> > from include/linux/stddef.h:4,
> > from include/uapi/linux/posix_types.h:4,
> > from include/uapi/linux/types.h:13,
> > from include/linux/types.h:5,
> > from net/netfilter/nf_conntrack_proto_udp.c:10:
> >net/netfilter/nf_conntrack_proto_udp.c: In function 'udp_get_net_proto':
> > >> net/netfilter/nf_conntrack_proto_udp.c:308:15: error: 
> > >> 'CTA_TIMEOUT_UDP_MAX' undeclared (first use in this function)
> >  BUILD_BUG_ON(CTA_TIMEOUT_UDP_MAX != CTA_TIMEOUT_UDPLITE_MAX);
> 
> Yay for conditional includes :-/
> 
> Pablo, I just saw there are still patches from Davide in your queue so I
> will not resend this (it will surely conflict).

They are now there. Please rebase and resubmit. Thanks for working on
this merging.
--
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 nf-next] netfilter: ingress: translate 0 nf_hook_slow retval to -EINPROGRESS

[Pablo Neira Ayuso]

On Mon, Nov 28, 2016 at 11:40:05AM +0100, Florian Westphal wrote:
> The caller assumes that < 0 means that skb was stolen (or free'd).
> 
> All other return values continue skb processing.
> 
> nf_hook_slow returns 3 different return value types:
> 
> A) a (negative) errno value: the skb was dropped (NF_DROP, e.g.
> by iptables '-j DROP' rule).
> 
> B) 0. The skb was stolen by the hook or queued to userspace.
> 
> C) 1. all hooks returned NF_ACCEPT so the caller should invoke
>the okfn so packet processing can continue.
> 
> nft ingress facility currently doesn't have the 'okfn' that
> the NF_HOOK() macros use; there is no nfqueue support either.
> 
> So 1 means that nf_hook_ingress() caller should go on processing the skb.
> 
> In order to allow use of NF_STOLEN from ingress we need to translate
> this to an errno number, else we'd crash because we continue with
> already-free'd (or about to be free-d) skb.
> 
> Random dice roll has chosen EINPROGRESS.  The errno value isn't checked,
> its just important that its less than 0.

Not really comments to block anything, so take them lightly.

Probably we can just set this to -1, I know this maps to some errno
value, but at least from code reading it will be make think that we
meant to return EINPROGRESS. If you agree, I can just mangle the
patch.

One more question below.

> Signed-off-by: Florian Westphal 
> ---
>  include/linux/netfilter_ingress.h | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/netfilter_ingress.h 
> b/include/linux/netfilter_ingress.h
> index 2dc3b49b804a..d5027e470a24 100644
> --- a/include/linux/netfilter_ingress.h
> +++ b/include/linux/netfilter_ingress.h
> @@ -19,6 +19,7 @@ static inline int nf_hook_ingress(struct sk_buff *skb)
>  {
>   struct nf_hook_entry *e = rcu_dereference(skb->dev->nf_hooks_ingress);
>   struct nf_hook_state state;
> + int ret;
>  
>   /* Must recheck the ingress hook head, in the event it became NULL
>* after the check in nf_hook_ingress_active evaluated to true.
> @@ -29,7 +30,11 @@ static inline int nf_hook_ingress(struct sk_buff *skb)
>   nf_hook_state_init(, NF_NETDEV_INGRESS,
>  NFPROTO_NETDEV, skb->dev, NULL, NULL,
>  dev_net(skb->dev), NULL);
> - return nf_hook_slow(skb, , e);
> + ret = nf_hook_slow(skb, , e);
> + if (unlikely(ret == 0))
> + return -EINPROGRESS;

Do you prefer to keep this branch as unlikely? I understand most
people are not using fwd much so far, but we're targeting to enrich
ingress so I would expect users will be using this more and more.
Anyway, we can revisit this later on.
--
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 nf-next] netfilter: fix build failure with CONNTRACK=n NF_DEFRAG=y

[Pablo Neira Ayuso]

On Sun, Dec 04, 2016 at 11:37:22PM +0100, Florian Westphal wrote:
> conntrack depends on defrag support, but not vice versa, so we cannot
> place defrag_ipv4/6 into netns->ct:
> 
> net/ipv4/netfilter/nf_defrag_ipv4.c:110:9: error: 'struct net' has no member 
> named 'ct'
> 
> Move it into net->nf.

Applied, thanks Florian.
--
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] datatype: Display pre-defined inet_service values in host byte order

[Pablo Neira Ayuso]

On Mon, Dec 05, 2016 at 10:30:38PM -0200, Elise Lennion wrote:
> nft describe displays, to the user, which values are available for a selector,
> then the values should be in host byte order.
> 
> Reported-by: Pablo Neira Ayuso 
> Fixes: ccc5da470e76 ("datatype: Replace getnameinfo() by internal lookup 
> table")
> Signed-off-by: Elise Lennion 
> ---
>  include/datatype.h |  3 ++-
>  src/datatype.c | 14 +++---
>  src/expression.c   |  3 ++-
>  3 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/include/datatype.h b/include/datatype.h
> index d4fe817..a7db1df 100644
> --- a/include/datatype.h
> +++ b/include/datatype.h
> @@ -191,7 +191,8 @@ extern struct error_record *symbolic_constant_parse(const 
> struct expr *sym,
>  extern void symbolic_constant_print(const struct symbol_table *tbl,
>   const struct expr *expr, bool quotes);
>  extern void symbol_table_print(const struct symbol_table *tbl,
> -const struct datatype *dtype);
> +const struct datatype *dtype,
> +enum byteorder byteorder);
>  
>  extern struct symbol_table *rt_symbol_table_init(const char *filename);
>  extern void rt_symbol_table_free(struct symbol_table *tbl);
> diff --git a/src/datatype.c b/src/datatype.c
> index b5d73bc..4f98a83 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -181,14 +181,22 @@ void symbolic_constant_print(const struct symbol_table 
> *tbl,
>  }
>  
>  void symbol_table_print(const struct symbol_table *tbl,
> - const struct datatype *dtype)
> + const struct datatype *dtype,
> + enum byteorder byteorder)
>  {
>   const struct symbolic_constant *s;
>   unsigned int size = 2 * dtype->size / BITS_PER_BYTE;
> + long unsigned int value;
> +
> + for (s = tbl->symbols; s->identifier != NULL; s++) {
> + if (byteorder == BYTEORDER_BIG_ENDIAN)
> + value = __constant_ntohs(s->value);

Is this going to work for other cases other than 16-bit?

You can probably add a function that, based on the datatype, performs
the byteswapping based on the integer length.

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