Re: [PATCH nf-next v2 2/2] netfilter: nft_hash: support of symmetric hash

2017-02-28 Thread Liping Zhang 
Hi,

2017-03-01 1:38 GMT+08:00 Laura Garcia Liebana :
[...]
> +static const struct nft_expr_ops *
> +nft_hash_select_ops(const struct nft_ctx *ctx,
> +   const struct nlattr * const tb[])
> +{
> +   u32 type;
> +
> +   if (!tb[NFTA_HASH_TYPE])
> +   return ERR_PTR(-EINVAL);

For compatibility reason, if tb[NFTA_HASH_TYPE] is NULL, maybe it's better
to return _jhash_ops instead of reporting error.
--
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: nft authentication

2017-02-28 Thread Florian Westphal 
Fabian Franz  wrote:
> I am working on my module but I cannot get the match visible to the nft
> tool. Could you please give me a hint, what is wrong in the code? I have
> uploaded it to my web server: http://files.fabian-franz.eu/nft_auth.c

I do not know what 'visible to the nft tool' means.
No 'obvious' mistake in the register department.

My only comment is that it looks like you are re-inventing the wheels
we already have, such as nf_log and nf_queue.

If this is a learning exercise, fine, but we have real missing
functionality that could be added instead.

If this targets upstream, you should really discuss what problem wants
to be solved.  The building blocks we already have should be enough
to do uid based authentication.

(something like
 nf_log/queue -> userspace daemon -> query -> update nft set w. uid)
--
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] netfilter: Use pr_cont where appropriate

2017-02-28 Thread Joe Perches 
Logging output was changed when simple printks without KERN_CONT
are now emitted on a new line and KERN_CONT is required to continue
lines so use pr_cont.

Miscellanea:

o realign arguments
o use print_hex_dump instead of a local variant

Signed-off-by: Joe Perches 
---
 net/bridge/netfilter/ebt_log.c | 34 +-
 net/ipv4/netfilter/nf_nat_snmp_basic.c | 15 ++-
 2 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/net/bridge/netfilter/ebt_log.c b/net/bridge/netfilter/ebt_log.c
index 98b9c8e8615e..707caea39743 100644
--- a/net/bridge/netfilter/ebt_log.c
+++ b/net/bridge/netfilter/ebt_log.c
@@ -62,10 +62,10 @@ print_ports(const struct sk_buff *skb, uint8_t protocol, 
int offset)
pptr = skb_header_pointer(skb, offset,
  sizeof(_ports), &_ports);
if (pptr == NULL) {
-   printk(" INCOMPLETE TCP/UDP header");
+   pr_cont(" INCOMPLETE TCP/UDP header");
return;
}
-   printk(" SPT=%u DPT=%u", ntohs(pptr->src), ntohs(pptr->dst));
+   pr_cont(" SPT=%u DPT=%u", ntohs(pptr->src), ntohs(pptr->dst));
}
 }
 
@@ -100,11 +100,11 @@ ebt_log_packet(struct net *net, u_int8_t pf, unsigned int 
hooknum,
 
ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
if (ih == NULL) {
-   printk(" INCOMPLETE IP header");
+   pr_cont(" INCOMPLETE IP header");
goto out;
}
-   printk(" IP SRC=%pI4 IP DST=%pI4, IP tos=0x%02X, IP proto=%d",
-  >saddr, >daddr, ih->tos, ih->protocol);
+   pr_cont(" IP SRC=%pI4 IP DST=%pI4, IP tos=0x%02X, IP proto=%d",
+   >saddr, >daddr, ih->tos, ih->protocol);
print_ports(skb, ih->protocol, ih->ihl*4);
goto out;
}
@@ -120,11 +120,11 @@ ebt_log_packet(struct net *net, u_int8_t pf, unsigned int 
hooknum,
 
ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
if (ih == NULL) {
-   printk(" INCOMPLETE IPv6 header");
+   pr_cont(" INCOMPLETE IPv6 header");
goto out;
}
-   printk(" IPv6 SRC=%pI6 IPv6 DST=%pI6, IPv6 priority=0x%01X, 
Next Header=%d",
-  >saddr, >daddr, ih->priority, ih->nexthdr);
+   pr_cont(" IPv6 SRC=%pI6 IPv6 DST=%pI6, IPv6 priority=0x%01X, 
Next Header=%d",
+   >saddr, >daddr, ih->priority, ih->nexthdr);
nexthdr = ih->nexthdr;
offset_ph = ipv6_skip_exthdr(skb, sizeof(_iph), , 
_off);
if (offset_ph == -1)
@@ -142,12 +142,12 @@ ebt_log_packet(struct net *net, u_int8_t pf, unsigned int 
hooknum,
 
ah = skb_header_pointer(skb, 0, sizeof(_arph), &_arph);
if (ah == NULL) {
-   printk(" INCOMPLETE ARP header");
+   pr_cont(" INCOMPLETE ARP header");
goto out;
}
-   printk(" ARP HTYPE=%d, PTYPE=0x%04x, OPCODE=%d",
-  ntohs(ah->ar_hrd), ntohs(ah->ar_pro),
-  ntohs(ah->ar_op));
+   pr_cont(" ARP HTYPE=%d, PTYPE=0x%04x, OPCODE=%d",
+   ntohs(ah->ar_hrd), ntohs(ah->ar_pro),
+   ntohs(ah->ar_op));
 
/* If it's for Ethernet and the lengths are OK,
 * then log the ARP payload
@@ -161,17 +161,17 @@ ebt_log_packet(struct net *net, u_int8_t pf, unsigned int 
hooknum,
ap = skb_header_pointer(skb, sizeof(_arph),
sizeof(_arpp), &_arpp);
if (ap == NULL) {
-   printk(" INCOMPLETE ARP payload");
+   pr_cont(" INCOMPLETE ARP payload");
goto out;
}
-   printk(" ARP MAC SRC=%pM ARP IP SRC=%pI4 ARP MAC 
DST=%pM ARP IP DST=%pI4",
-   ap->mac_src, ap->ip_src, ap->mac_dst, 
ap->ip_dst);
+   pr_cont(" ARP MAC SRC=%pM ARP IP SRC=%pI4 ARP MAC 
DST=%pM ARP IP DST=%pI4",
+   ap->mac_src, ap->ip_src,
+   ap->mac_dst, ap->ip_dst);
}
}
 out:
-   printk("\n");
+   pr_cont("\n");
spin_unlock_bh(_log_lock);
-
 }
 
 static unsigned int
diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c 
b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index c9b52c361da2..ef49989c93b1 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -998,18 +998,6 @@ static unsigned char snmp_trap_decode(struct asn1_ctx

Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition

2017-02-28 Thread Ken-ichirou MATSUZAWA 
 Hi, Pablo
 
On Tue, Feb 28, 2017 at 12:48:09PM +0100, Pablo Neira Ayuso wrote:
> So you want to check if the addresses mismatch, so we infer from there
> if there is NAT or not when status bits are not available.
> 
> Are you trying to catch up some case in netlink event specifically?

It's nothing. My skimping test for lnfct binding for another languate
which set only ATTR_REPL_IPV4DST then get NFCT_GOPT_IS_NAT was success,
but it fails now.

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

nft authentication

2017-02-28 Thread Fabian Franz 
Hi all,


I am working on my module but I cannot get the match visible to the nft
tool. Could you please give me a hint, what is wrong in the code? I have
uploaded it to my web server: http://files.fabian-franz.eu/nft_auth.c

The match should be "auth ".


Kind regards


Fabian Franz

--
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 nft v2] src: hash: support of symmetric hash

2017-02-28 Thread Laura Garcia Liebana 
This patch provides symmetric hash support according to source
ip address and port, and destination ip address and port.

The new attribute NFTA_HASH_TYPE has been included to support
different types of hashing functions. Currently supported
NFT_HASH_JENKINS through jhash and NFT_HASH_SYM through symhash.

The main difference between both types are:
 - jhash requires an expression with sreg, symhash doesn't.
 - symhash supports modulus and offset, but not seed.

Examples:

 nft add rule ip nat prerouting ct mark set jhash ip saddr mod 2
 nft add rule ip nat prerouting ct mark set symhash mod 2

Signed-off-by: Laura Garcia Liebana 
---
v2:
- Discard new line remove

 include/expression.h|  1 +
 include/hash.h  |  2 +-
 include/linux/netfilter/nf_tables.h | 13 +
 src/evaluate.c  |  3 ++-
 src/hash.c  | 28 +---
 src/netlink_delinearize.c   | 35 +--
 src/netlink_linearize.c | 19 ---
 src/parser_bison.y  | 16 ++--
 src/scanner.l   |  1 +
 tests/py/ip/hash.t  |  1 +
 tests/py/ip/hash.t.payload  |  4 
 11 files changed, 87 insertions(+), 36 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index ec90265..56cb310 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -308,6 +308,7 @@ struct expr {
uint32_tmod;
uint32_tseed;
uint32_toffset;
+   enum nft_hash_types type;
} hash;
struct {
/* EXPR_FIB */
diff --git a/include/hash.h b/include/hash.h
index 8bf53e2..7f9c6f1 100644
--- a/include/hash.h
+++ b/include/hash.h
@@ -3,6 +3,6 @@
 
 extern struct expr *hash_expr_alloc(const struct location *loc,
uint32_t modulus, uint32_t seed,
-   uint32_t offset);
+   uint32_t offset, enum nft_hash_types type);
 
 #endif /* NFTABLES_HASH_H */
diff --git a/include/linux/netfilter/nf_tables.h 
b/include/linux/netfilter/nf_tables.h
index b00a05d..74a42fa 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -793,6 +793,17 @@ enum nft_rt_keys {
 };
 
 /**
+ * enum nft_hash_types - nf_tables hash expression types
+ *
+ * @NFT_HASH_JENKINS: Jenkins Hash
+ * @NFT_HASH_SYM: Symmetric Hash
+ */
+enum nft_hash_types {
+   NFT_HASH_JENKINS,
+   NFT_HASH_SYM,
+};
+
+/**
  * enum nft_hash_attributes - nf_tables hash expression netlink attributes
  *
  * @NFTA_HASH_SREG: source register (NLA_U32)
@@ -801,6 +812,7 @@ enum nft_rt_keys {
  * @NFTA_HASH_MODULUS: modulus value (NLA_U32)
  * @NFTA_HASH_SEED: seed value (NLA_U32)
  * @NFTA_HASH_OFFSET: add this offset value to hash result (NLA_U32)
+ * @NFTA_HASH_TYPE: hash operation (NLA_U32: nft_hash_types)
  */
 enum nft_hash_attributes {
NFTA_HASH_UNSPEC,
@@ -810,6 +822,7 @@ enum nft_hash_attributes {
NFTA_HASH_MODULUS,
NFTA_HASH_SEED,
NFTA_HASH_OFFSET,
+   NFTA_HASH_TYPE,
__NFTA_HASH_MAX,
 };
 #define NFTA_HASH_MAX  (__NFTA_HASH_MAX - 1)
diff --git a/src/evaluate.c b/src/evaluate.c
index 94412f2..18884be 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1249,7 +1249,8 @@ static int expr_evaluate_hash(struct eval_ctx *ctx, 
struct expr **exprp)
expr_dtype_integer_compatible(ctx, expr);
 
expr_set_context(>ectx, NULL, 0);
-   if (expr_evaluate(ctx, >hash.expr) < 0)
+   if (expr->hash.expr &&
+   expr_evaluate(ctx, >hash.expr) < 0)
return -1;
 
/* expr_evaluate_primary() sets the context to what to the input
diff --git a/src/hash.c b/src/hash.c
index d26b2ed..a7a9612 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -17,10 +17,18 @@
 
 static void hash_expr_print(const struct expr *expr)
 {
-   printf("jhash ");
-   expr_print(expr->hash.expr);
+   switch (expr->hash.type) {
+   case NFT_HASH_SYM:
+   printf("symhash");
+   break;
+   case NFT_HASH_JENKINS:
+   default:
+   printf("jhash ");
+   expr_print(expr->hash.expr);
+   }
+
printf(" mod %u", expr->hash.mod);
-   if (expr->hash.seed)
+   if (expr->hash.type & NFT_HASH_JENKINS && expr->hash.seed)
printf(" seed 0x%x", expr->hash.seed);
if (expr->hash.offset)
printf(" offset %u", expr->hash.offset);
@@ -28,18 +36,22 @@ static void hash_expr_print(const struct expr *expr)
 
 static bool hash_expr_cmp(const struct expr *e1, const struct expr *e2)
 {
-   return expr_cmp(e1->hash.expr, e2->hash.expr) &&
+   return (e1->hash.expr ||
+

[PATCH nf-next v2 2/2] netfilter: nft_hash: support of symmetric hash

2017-02-28 Thread Laura Garcia Liebana 
This patch provides symmetric hash support according to source
ip address and port, and destination ip address and port.

For this purpose, the __skb_get_hash_symmetric() is used to
identify the flow as it uses FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL
flag by default.

The new attribute NFTA_HASH_TYPE has been included to support
different types of hashing functions. Currently supported
NFT_HASH_JENKINS through jhash and NFT_HASH_SYM through symhash.

The main difference between both types are:
 - jhash requires an expression with sreg, symhash doesn't.
 - symhash supports modulus and offset, but not seed.

Examples:

 nft add rule ip nat prerouting ct mark set jhash ip saddr mod 2
 nft add rule ip nat prerouting ct mark set symhash mod 2

Signed-off-by: Laura Garcia Liebana 
---
v2:
- Avoid warning due to 'const' from symhash eval skb

 include/uapi/linux/netfilter/nf_tables.h | 13 +
 net/netfilter/nft_hash.c | 98 +++-
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h 
b/include/uapi/linux/netfilter/nf_tables.h
index 7b730cab99bd..a444a63a9eee 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -793,6 +793,17 @@ enum nft_rt_keys {
 };
 
 /**
+ * enum nft_hash_types - nf_tables hash expression types
+ *
+ * @NFT_HASH_JENKINS: Jenkins Hash
+ * @NFT_HASH_SYM: Symmetric Hash
+ */
+enum nft_hash_types {
+   NFT_HASH_JENKINS,
+   NFT_HASH_SYM,
+};
+
+/**
  * enum nft_hash_attributes - nf_tables hash expression netlink attributes
  *
  * @NFTA_HASH_SREG: source register (NLA_U32)
@@ -801,6 +812,7 @@ enum nft_rt_keys {
  * @NFTA_HASH_MODULUS: modulus value (NLA_U32)
  * @NFTA_HASH_SEED: seed value (NLA_U32)
  * @NFTA_HASH_OFFSET: add this offset value to hash result (NLA_U32)
+ * @NFTA_HASH_TYPE: hash operation (NLA_U32: nft_hash_types)
  */
 enum nft_hash_attributes {
NFTA_HASH_UNSPEC,
@@ -810,6 +822,7 @@ enum nft_hash_attributes {
NFTA_HASH_MODULUS,
NFTA_HASH_SEED,
NFTA_HASH_OFFSET,
+   NFTA_HASH_TYPE,
__NFTA_HASH_MAX,
 };
 #define NFTA_HASH_MAX  (__NFTA_HASH_MAX - 1)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index ccb834ef049b..8eaf3d25b970 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -38,6 +38,25 @@ static void nft_jhash_eval(const struct nft_expr *expr,
regs->data[priv->dreg] = h + priv->offset;
 }
 
+struct nft_symhash {
+   enum nft_registers  dreg:8;
+   u32 modulus;
+   u32 offset;
+};
+
+static void nft_symhash_eval(const struct nft_expr *expr,
+struct nft_regs *regs,
+const struct nft_pktinfo *pkt)
+{
+   struct nft_symhash *priv = nft_expr_priv(expr);
+   struct sk_buff *skb = pkt->skb;
+   u32 h;
+
+   h = reciprocal_scale(__skb_get_hash_symmetric(skb), priv->modulus);
+
+   regs->data[priv->dreg] = h + priv->offset;
+}
+
 static const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {
[NFTA_HASH_SREG]= { .type = NLA_U32 },
[NFTA_HASH_DREG]= { .type = NLA_U32 },
@@ -45,6 +64,7 @@ static const struct nla_policy nft_hash_policy[NFTA_HASH_MAX 
+ 1] = {
[NFTA_HASH_MODULUS] = { .type = NLA_U32 },
[NFTA_HASH_SEED]= { .type = NLA_U32 },
[NFTA_HASH_OFFSET]  = { .type = NLA_U32 },
+   [NFTA_HASH_TYPE]= { .type = NLA_U32 },
 };
 
 static int nft_jhash_init(const struct nft_ctx *ctx,
@@ -92,6 +112,32 @@ static int nft_jhash_init(const struct nft_ctx *ctx,
   NFT_DATA_VALUE, sizeof(u32));
 }
 
+static int nft_symhash_init(const struct nft_ctx *ctx,
+   const struct nft_expr *expr,
+   const struct nlattr * const tb[])
+{
+   struct nft_symhash *priv = nft_expr_priv(expr);
+
+   if (!tb[NFTA_HASH_DREG]||
+   !tb[NFTA_HASH_MODULUS])
+   return -EINVAL;
+
+   if (tb[NFTA_HASH_OFFSET])
+   priv->offset = ntohl(nla_get_be32(tb[NFTA_HASH_OFFSET]));
+
+   priv->dreg = nft_parse_register(tb[NFTA_HASH_DREG]);
+
+   priv->modulus = ntohl(nla_get_be32(tb[NFTA_HASH_MODULUS]));
+   if (priv->modulus <= 1)
+   return -ERANGE;
+
+   if (priv->offset + priv->modulus - 1 < priv->offset)
+   return -EOVERFLOW;
+
+   return nft_validate_register_store(ctx, priv->dreg, NULL,
+  NFT_DATA_VALUE, sizeof(u32));
+}
+
 static int nft_jhash_dump(struct sk_buff *skb,
  const struct nft_expr *expr)
 {
@@ -110,6 +156,28 @@ static int nft_jhash_dump(struct sk_buff *skb,
if (priv->offset != 0)
if (nla_put_be32(skb, NFTA_HASH_OFFSET, htonl(priv->offset)))

ANNOUNCE: Netdev 2.1 update Feb 28

2017-02-28 Thread Jamal Hadi Salim 

A few announcements:


1) Going forward we are going to be sending more frequent
announcements to the conference discussion/announcement list:
peo...@netdevconf.org

You can subscribe via mailman here:
https://lists.netdevconf.org/cgi-bin/mailman/listinfo/people

We urge people to subscribe to that list to get more up to date
information and conference discussions. We will still be sending
weekly summaries and announcements on the netdev list.

If you have already registered for the conference we will
very likely have registered you already.

2) Reminder: Early registration was extended to March 5.

Register early so we can plan better (and so you can save some $$).
https://onlineregistrations.ca/netdev21/

- hotel (If you can get the hotel cheaper online than conference
rates please send us email, dont book ):
https://www.netdevconf.org/2.1/hotel.html

3) Tech committee has accepted another exciting talk. This
time from Gilberto Bertin on how Cloudfare is integrating XDP in
their DDoS mitigation approach. More description:


This talk provides an introduction to how we are planning to integrate
XDP into our DDoS mitigation pipeline at Cloudflare, which every day
helps defend our infrastructure from thousands of different attacks,
in the range of 50-150 Gbps (with peaks of 500 Gbps).

The main points of the presentation are:
* introduction to the current architecture: from how attack traffic
is sampled, to attack detection and mitigation, to how BPF fits in
the current pipeline
* architecture changes we are expecting, and new tools required
after we move from our current network kernel bypass setup to XDP
* issues and questions regarding the current state of XDP
---

cheers,
jamal
--
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 v3] libiptc: don't set_changed() when checking rules with module jumps

2017-02-28 Thread Pablo Neira Ayuso 
On Tue, Feb 28, 2017 at 01:03:07PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso  wrote:
> > On Sat, Feb 25, 2017 at 10:02:03PM -0600, Dan Williams wrote:
> > > Checking a rule that includes a jump to a module-based target currently
> > > sets the "changed" flag on the handle, which then causes TC_COMMIT() to
> > > run through the whole SO_SET_REPLACE/SO_SET_ADD_COUNTERS path.  This
> > > seems wrong for simply checking rules, an operation which is documented
> > > as "...does not alter the existing iptables configuration..." but yet
> > > it clearly could do so.
> > > 
> > > Fix that by ensuring that rule check operations for module targets
> > > don't set the changed flag, and thus exit early from TC_COMMIT().
> > 
> > Thanks for explaining. How are you hitting this problem? I'm curious
> > to see if I can reproduce it.
> 
> Its easy to reproduce.
> 
> iptables -t nat -o lo -A POSTROUTING -s 10.2.3.4 -j MASQUERADE
> iptables -t nat -o lo -C POSTROUTING -s 10.2.3.4 -j MASQUERADE
> 
> you should see (via strace) that 2nd command also issues the iptables
> setsockopt calls.

Thanks for explain. 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: [4.9.10] ip_route_me_harder() reading off-slab

2017-02-28 Thread Pablo Neira Ayuso 
On Mon, Feb 27, 2017 at 10:41:48PM +0800, Daniel J Blueman wrote:
> On 17 February 2017 at 15:39, Florian Westphal  wrote:
> > Daniel J Blueman  wrote:
> >
> > [ CC nf-devel, pablo ]
> >
> >> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
> >> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
> >> access [1].
> >>
> >> Let me know if you'd like more debug.
> >
> > Does this patch help?
> >
> > Subject: [PATCH nf] netfilter: use skb_to_full_sk in ip_route_me_harder
> >
> > inet_sk(skb->sk) is illegal in case skb is attached to request socket.
> >
> > Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets 
> > instead of listener")
> > Reported by: Daniel J Blueman 
> > Signed-off-by: Florian Westphal 
[...]
> Apologies for the delays; this also addresses the issue just fine.
> 
> Tested-by: Daniel J Blueman 

Applied, thanks for testing.
--
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 v3] libiptc: don't set_changed() when checking rules with module jumps

2017-02-28 Thread Florian Westphal 
Pablo Neira Ayuso  wrote:
> On Sat, Feb 25, 2017 at 10:02:03PM -0600, Dan Williams wrote:
> > Checking a rule that includes a jump to a module-based target currently
> > sets the "changed" flag on the handle, which then causes TC_COMMIT() to
> > run through the whole SO_SET_REPLACE/SO_SET_ADD_COUNTERS path.  This
> > seems wrong for simply checking rules, an operation which is documented
> > as "...does not alter the existing iptables configuration..." but yet
> > it clearly could do so.
> > 
> > Fix that by ensuring that rule check operations for module targets
> > don't set the changed flag, and thus exit early from TC_COMMIT().
> 
> Thanks for explaining. How are you hitting this problem? I'm curious
> to see if I can reproduce it.

Its easy to reproduce.

iptables -t nat -o lo -A POSTROUTING -s 10.2.3.4 -j MASQUERADE
iptables -t nat -o lo -C POSTROUTING -s 10.2.3.4 -j MASQUERADE

you should see (via strace) that 2nd command also issues the iptables
setsockopt calls.
--
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 lnfct 2/2] conntrack: revert getobjopt_is_nat condition

2017-02-28 Thread Ken-ichirou MATSUZAWA 
 Hi, Pablo

On Tue, Feb 28, 2017 at 11:47:25AM +0100, Pablo Neira Ayuso wrote:
> > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c
> > index fb43d6c..1581480 100644
> > --- a/src/conntrack/objopt.c
> > +++ b/src/conntrack/objopt.c
> > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int 
> > option)
> >  
> >  static int getobjopt_is_snat(const struct nf_conntrack *ct)
> >  {
> > -   if (!(test_bit(ATTR_STATUS, ct->head.set)))
> > -   return 0;
> > -
> > -   if (!(ct->status & IPS_SRC_NAT_DONE))
> > +   if (test_bit(ATTR_STATUS, ct->head.set) &&
> > +   !(ct->status & IPS_SRC_NAT_DONE))
> 
> However, if ATTR_STATUS is not set, we keep checking ahead. What are
> you trying to fix?

It was:

-   return ((test_bit(ATTR_STATUS, ct->head.set) ?
-   ct->status & IPS_SRC_NAT_DONE : 1) &&
-   ct->repl.dst.v4 !=
-   ct->head.orig.src.v4);

I thought it keeps checking even ATTR_STATUS is not set.
But it's ok not to apply, returning false in case of
ATTR_STATUS is not set.

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 lnfct 2/2] conntrack: revert getobjopt_is_nat condition

2017-02-28 Thread Pablo Neira Ayuso 
On Tue, Feb 28, 2017 at 08:44:53PM +0900, Ken-ichirou MATSUZAWA wrote:
>  Hi, Pablo
> 
> On Tue, Feb 28, 2017 at 11:47:25AM +0100, Pablo Neira Ayuso wrote:
> > > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c
> > > index fb43d6c..1581480 100644
> > > --- a/src/conntrack/objopt.c
> > > +++ b/src/conntrack/objopt.c
> > > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned 
> > > int option)
> > >  
> > >  static int getobjopt_is_snat(const struct nf_conntrack *ct)
> > >  {
> > > - if (!(test_bit(ATTR_STATUS, ct->head.set)))
> > > - return 0;
> > > -
> > > - if (!(ct->status & IPS_SRC_NAT_DONE))
> > > + if (test_bit(ATTR_STATUS, ct->head.set) &&
> > > + !(ct->status & IPS_SRC_NAT_DONE))
> > 
> > However, if ATTR_STATUS is not set, we keep checking ahead. What are
> > you trying to fix?
> 
> It was:
> 
> -   return ((test_bit(ATTR_STATUS, ct->head.set) ?
> -   ct->status & IPS_SRC_NAT_DONE : 1) &&
> -   ct->repl.dst.v4 !=
> -   ct->head.orig.src.v4);
> 
> I thought it keeps checking even ATTR_STATUS is not set.
> But it's ok not to apply, returning false in case of
> ATTR_STATUS is not set.

Ah, I see.

static int getobjopt_is_snat(const struct nf_conntrack *ct)
{
if (!(test_bit(ATTR_STATUS, ct->head.set)))
return 0;

if (!(ct->status & IPS_SRC_NAT_DONE))
return 0;

switch (ct->head.orig.l3protonum) {
case AF_INET:
return ct->repl.dst.v4 != ct->head.orig.src.v4;
case AF_INET6:
if (memcmp(>repl.dst.v6, >head.orig.src.v6,
   sizeof(struct in6_addr)) != 0)
return 1;
else
return 0;
default:
return 0;
}
}

So you want to check if the addresses mismatch, so we infer from there
if there is NAT or not when status bits are not available.

Are you trying to catch up some case in netlink event specifically?

Thanks for explaining.
--
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 v3] libiptc: don't set_changed() when checking rules with module jumps

2017-02-28 Thread Pablo Neira Ayuso 
On Sat, Feb 25, 2017 at 10:02:03PM -0600, Dan Williams wrote:
> Checking a rule that includes a jump to a module-based target currently
> sets the "changed" flag on the handle, which then causes TC_COMMIT() to
> run through the whole SO_SET_REPLACE/SO_SET_ADD_COUNTERS path.  This
> seems wrong for simply checking rules, an operation which is documented
> as "...does not alter the existing iptables configuration..." but yet
> it clearly could do so.
> 
> Fix that by ensuring that rule check operations for module targets
> don't set the changed flag, and thus exit early from TC_COMMIT().

Thanks for explaining. How are you hitting this problem? I'm curious
to see if I can reproduce it.

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] netfilter: remove redundant check on ret being non-zero

2017-02-28 Thread Colin King 
From: Colin Ian King 

ret is initialized to zero and if it is set to non-zero in the
xt_entry_foreach loop then we exit via the out_free label. Hence
the check for ret being non-zero is redundant and can be removed.

Detected by CoverityScan, CID#1357132 ("Logically Dead Code")

Signed-off-by: Colin Ian King 
---
 net/ipv4/netfilter/arp_tables.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 6241a81..f17dab1 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -562,8 +562,6 @@ static int translate_table(struct xt_table_info *newinfo, 
void *entry0,
XT_ERROR_TARGET) == 0)
++newinfo->stacksize;
}
-   if (ret != 0)
-   goto out_free;
 
ret = -EINVAL;
if (i != repl->num_entries)
-- 
2.10.2

--
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 0/7] nftables: add ct helper set support

2017-02-28 Thread Pablo Neira Ayuso 
On Mon, Feb 27, 2017 at 04:02:48PM +0100, Florian Westphal wrote:
> This series adds initial support to set conntrack helpers via
> the nft objref infrastructure.
> 
> As -next is closed I will not push this yet since kernel support
> is still missing.
> 
> Currently only supported attributes are:
> 
> type (e.g. "ftp", "sip)
> protocol (udp or tcp)
> l3proto (ip, ip6).
> 
> l3proto is optional, the kernel will infer it from
> the family, i.e. in "inet" case kernel will check for both ipv4 and ipv6.
> 
> Let me know in case you spot any isses, patches #3 and #5 could already
> be pushed to nft master since they are just preparation patches.

Series looks good, thanks Florian.

Acked-by: Pablo Neira Ayuso 
--
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 iptables] extensions: libxt_hashlimit: Add translation to nft

2017-02-28 Thread Pablo Neira Ayuso 
On Mon, Feb 27, 2017 at 02:43:08PM -0300, Elise Lennion wrote:
> Hashlimit has similar functionality to flow tables in nftables. Some
> usage examples are:
> 
> $ iptables-translate -A OUTPUT -m tcp -p tcp --dport 443 -m hashlimit \
> --hashlimit-above 20kb/s --hashlimit-burst 1mb --hashlimit-mode dstip \
> --hashlimit-name https --hashlimit-dstmask 24 -m state --state NEW \
> -j DROP
> 
> nft add rule ip filter OUTPUT tcp dport 443 flow table https { ip \
> daddr and 255.255.255.0 timeout 60s limit rate over 20 kbytes/second \
> burst 1 mbytes} ct state new  counter drop
> 
> $ iptables-translate -A OUTPUT -m tcp -p tcp --dport 443 -m hashlimit \
> --hashlimit-upto 300 --hashlimit-burst 15 --hashlimit-mode \
> srcip,dstip --hashlimit-name https --hashlimit-htable-expire 30 \
> -m state --state NEW -j DROP
> 
> nft add rule ip filter OUTPUT tcp dport 443 flow table https { ip \
> daddr . ip saddr timeout 300s limit rate 300/second burst 15 packets} \
> ct state new  counter drop
> 
> The translation isn't supported when --hashlimit-mode isn't specified.
> Also, the following options don't apply to flow tables:
> 
> --hashlimit-htable-size
> --hashlimit-htable-max
> --hashlimit-htable-gcinterval

Applied, thanks Elise.

I think it would be good to enhance the flow table article in the wiki
to describe how people can do hashlimit with nft.
--
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 iptables V2 2/2] xshared: using the blocking file lock request when we wait indefinitely

2017-02-28 Thread Pablo Neira Ayuso 
On Mon, Feb 06, 2017 at 07:47:47PM +0800, Liping Zhang wrote:
> From: Liping Zhang 
> 
> When using "-w" to avoid concurrent instances, we try to do flock() every
> one second until it success. But one second maybe too long in some
> situations, and it's hard to select a suitable interval time. So when
> using "iptables -w" to wait indefinitely, it's better to block until
> it become success.
> 
> Now do some performance tests. First, flush all the iptables rules in
> filter table, and run "iptables -w -S" endlessly:
>   # iptables -F
>   # iptables -X
>   # while : ; do
>   iptables -w -S >&- &
>   done
> 
> Second, after adding and deleting the iptables rules 100 times, measure
> the time cost:
>   # time for i in $(seq 100); do
>   iptables -w -A INPUT
>   iptables -w -D INPUT
>   done
> 
> Before this patch:
>   real  1m15.962s
>   user  0m0.224s
>   sys   0m1.475s
> 
> Apply this patch:
>   real  0m1.830s
>   user  0m0.168s
>   sys   0m1.130s

Also 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 iptables 1/2] xshared: do not lock again and again if "-w" option is not specified

2017-02-28 Thread Pablo Neira Ayuso 
On Sun, Feb 05, 2017 at 09:57:34PM +0800, Liping Zhang wrote:
> From: Liping Zhang 
> 
> After running the following commands, some confusing messages was printed
> out:
>   # while : ; do
>   iptables -A INPUT &
>   iptables -D INPUT &
>   done
>   [...]
>   Another app is currently holding the xtables lock; still -9s 0us time
>   ahead to have a chance to grab the lock...
>   Another app is currently holding the xtables lock; still -29s 0us time
>   ahead to have a chance to grab the lock...
> 
> If "-w" option is not specified, the "wait" will be zero, so we should
> check whether the timer_left is less than wait_interval before we call
> select to sleep.
> 
> Also remove unused "BASE_MICROSECONDS" and "struct timeval waited_time"
> introduced by commit e8f857a5a151 ("xtables: Add an interval option for
> xtables lock wait").

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 lnfct 2/2] conntrack: revert getobjopt_is_nat condition

2017-02-28 Thread Pablo Neira Ayuso 
Hi Ken-ichirou,

On Tue, Feb 28, 2017 at 02:00:41PM +0900, Ken-ichirou MATSUZAWA wrote:
> From 9e8aa4ed079b526faf190b69a2c1032f22776602 Mon Sep 17 00:00:00 2001
> From: Ken-ichirou MATSUZAWA 
> Date: Tue, 28 Feb 2017 11:34:29 +0900
> Subject: [PATCH 2/2] conntrack: revert getobjopt_is_nat condition
> 
> Signed-off-by: Ken-ichirou MATSUZAWA 
> ---
>  src/conntrack/objopt.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c
> index fb43d6c..1581480 100644
> --- a/src/conntrack/objopt.c
> +++ b/src/conntrack/objopt.c
> @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int 
> option)
>  
>  static int getobjopt_is_snat(const struct nf_conntrack *ct)
>  {
> - if (!(test_bit(ATTR_STATUS, ct->head.set)))
> - return 0;
> -
> - if (!(ct->status & IPS_SRC_NAT_DONE))
> + if (test_bit(ATTR_STATUS, ct->head.set) &&
> + !(ct->status & IPS_SRC_NAT_DONE))

However, if ATTR_STATUS is not set, we keep checking ahead. What are
you trying to fix?
--
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 lnfct 1/2] conntrack: fix missing break

2017-02-28 Thread Pablo Neira Ayuso 
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