Re: [PATCH nf-next,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Florian Westphal 
Pablo Neira Ayuso  wrote:
> int br_nf_hook_thresh(unsigned int hook, struct net *net,
>   struct sock *sk, struct sk_buff *skb,
>   struct net_device *indev,
>   struct net_device *outdev,
>   int (*okfn)(struct net *, struct sock *,
>   struct sk_buff *))
> {
> struct nf_hook_entry *elem;
> struct nf_hook_state state;
> int ret;
> 
> elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
> 
> while (elem && (elem->ops.priority <= NF_BR_PRI_BRNF))
> elem = rcu_dereference(elem->next);
> 
> ...
> 
> nf_hook_state_init(, elem, hook, NFPROTO_BRIDGE, indev, ...
> 
> Hm, but this code (before actually calling nf_hook_slow) is skipping
> the hook until we get to NF_BR_PRI_BRNF + 1.
> 
> Then hook state sets hook_entry to elem.
> 
> Am I missing anything?

Yes, I'm a moron -- Ignore.  I'll turn off the computer now.
--
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,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Pablo Neira Ayuso 
On Thu, Oct 13, 2016 at 05:10:55PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso  wrote:
> > On Thu, Oct 13, 2016 at 02:25:45PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso  wrote:
> > > > Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
> > > > introduced br_nf_hook_thresh().
> > > > 
> > > > Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
> > > > br_nf_forward_finish(), so we have no more callers for this macro.
> > > > 
> > > > As a result, state->thresh and explicit thresh parameter in the hook
> > > > state structure is not required anymore.
> > > > 
> > > > And we can get rid of fast forward code in nf_iterate() in the core path
> > > > that is only used by br_netfilter to search for the filter hook.
> > > 
> > > Note that you will need to move more parts of nf_hook_slow() into
> > > br_nf_hook_thresh(); the bridge netfilter does need to thresh feature
> > > that we have in nf_iterate().
> > 
> > br_nf_hook_thresh() is already skipping hooks before NF_BR_PRI_BRNF to
> > emulate thresh. What else is missing?
> 
> AFAICS you are removing the NF_BR_PRI_BRNF skipping in this patch,
> it relied on nf_hook_slow to do this (plus the state->thresh thing).

int br_nf_hook_thresh(unsigned int hook, struct net *net,
  struct sock *sk, struct sk_buff *skb,
  struct net_device *indev,
  struct net_device *outdev,
  int (*okfn)(struct net *, struct sock *,
  struct sk_buff *))
{
struct nf_hook_entry *elem;
struct nf_hook_state state;
int ret;

elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);

while (elem && (elem->ops.priority <= NF_BR_PRI_BRNF))
elem = rcu_dereference(elem->next);

...

nf_hook_state_init(, elem, hook, NFPROTO_BRIDGE, indev, ...

Hm, but this code (before actually calling nf_hook_slow) is skipping
the hook until we get to NF_BR_PRI_BRNF + 1.

Then hook state sets hook_entry to elem.

Am I missing anything?
--
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,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Florian Westphal 
Pablo Neira Ayuso  wrote:
> On Thu, Oct 13, 2016 at 02:25:45PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso  wrote:
> > > Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
> > > introduced br_nf_hook_thresh().
> > > 
> > > Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
> > > br_nf_forward_finish(), so we have no more callers for this macro.
> > > 
> > > As a result, state->thresh and explicit thresh parameter in the hook
> > > state structure is not required anymore.
> > > 
> > > And we can get rid of fast forward code in nf_iterate() in the core path
> > > that is only used by br_netfilter to search for the filter hook.
> > 
> > Note that you will need to move more parts of nf_hook_slow() into
> > br_nf_hook_thresh(); the bridge netfilter does need to thresh feature
> > that we have in nf_iterate().
> 
> br_nf_hook_thresh() is already skipping hooks before NF_BR_PRI_BRNF to
> emulate thresh. What else is missing?

AFAICS you are removing the NF_BR_PRI_BRNF skipping in this patch,
it relied on nf_hook_slow to do this (plus the state->thresh thing).
--
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,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Pablo Neira Ayuso 
On Thu, Oct 13, 2016 at 02:25:45PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso  wrote:
> > Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
> > introduced br_nf_hook_thresh().
> > 
> > Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
> > br_nf_forward_finish(), so we have no more callers for this macro.
> > 
> > As a result, state->thresh and explicit thresh parameter in the hook
> > state structure is not required anymore.
> > 
> > And we can get rid of fast forward code in nf_iterate() in the core path
> > that is only used by br_netfilter to search for the filter hook.
> 
> Note that you will need to move more parts of nf_hook_slow() into
> br_nf_hook_thresh(); the bridge netfilter does need to thresh feature
> that we have in nf_iterate().

br_nf_hook_thresh() is already skipping hooks before NF_BR_PRI_BRNF to
emulate thresh. What else is missing?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Florian Westphal 
Pablo Neira Ayuso  wrote:
> Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
> introduced br_nf_hook_thresh().
> 
> Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
> br_nf_forward_finish(), so we have no more callers for this macro.
> 
> As a result, state->thresh and explicit thresh parameter in the hook
> state structure is not required anymore.
> 
> And we can get rid of fast forward code in nf_iterate() in the core path
> that is only used by br_netfilter to search for the filter hook.

Note that you will need to move more parts of nf_hook_slow() into
br_nf_hook_thresh(); the bridge netfilter does need to thresh feature
that we have in nf_iterate().

--
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,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Pablo Neira Ayuso 
Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
introduced br_nf_hook_thresh().

Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
br_nf_forward_finish(), so we have no more callers for this macro.

As a result, state->thresh and explicit thresh parameter in the hook
state structure is not required anymore.

And we can get rid of fast forward code in nf_iterate() in the core path
that is only used by br_netfilter to search for the filter hook.

Suggested-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 
---
 include/linux/netfilter.h | 50 +--
 include/linux/netfilter_ingress.h |  2 +-
 net/bridge/br_netfilter_hooks.c   |  8 +++---
 net/bridge/netfilter/ebtable_broute.c |  2 +-
 net/netfilter/core.c  |  4 ---
 net/netfilter/nf_queue.c  |  1 -
 6 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index abc7fdcb9eb1..e0d000f6c9bf 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -49,7 +49,6 @@ struct sock;
 
 struct nf_hook_state {
unsigned int hook;
-   int thresh;
u_int8_t pf;
struct net_device *in;
struct net_device *out;
@@ -84,7 +83,7 @@ struct nf_hook_entry {
 static inline void nf_hook_state_init(struct nf_hook_state *p,
  struct nf_hook_entry *hook_entry,
  unsigned int hook,
- int thresh, u_int8_t pf,
+ u_int8_t pf,
  struct net_device *indev,
  struct net_device *outdev,
  struct sock *sk,
@@ -92,7 +91,6 @@ static inline void nf_hook_state_init(struct nf_hook_state *p,
  int (*okfn)(struct net *, struct sock *, 
struct sk_buff *))
 {
p->hook = hook;
-   p->thresh = thresh;
p->pf = pf;
p->in = indev;
p->out = outdev;
@@ -155,20 +153,16 @@ extern struct static_key 
nf_hooks_needed[NFPROTO_NUMPROTO][NF_MAX_HOOKS];
 int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state);
 
 /**
- * nf_hook_thresh - call a netfilter hook
+ * nf_hook - call a netfilter hook
  *
  * Returns 1 if the hook has allowed the packet to pass.  The function
  * okfn must be invoked by the caller in this case.  Any other return
  * value indicates the packet has been consumed by the hook.
  */
-static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook,
-struct net *net,
-struct sock *sk,
-struct sk_buff *skb,
-struct net_device *indev,
-struct net_device *outdev,
-int (*okfn)(struct net *, struct sock *, 
struct sk_buff *),
-int thresh)
+static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
+ struct sock *sk, struct sk_buff *skb,
+ struct net_device *indev, struct net_device *outdev,
+ int (*okfn)(struct net *, struct sock *, struct 
sk_buff *))
 {
struct nf_hook_entry *hook_head;
int ret = 1;
@@ -185,8 +179,8 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int 
hook,
if (hook_head) {
struct nf_hook_state state;
 
-   nf_hook_state_init(, hook_head, hook, thresh,
-  pf, indev, outdev, sk, net, okfn);
+   nf_hook_state_init(, hook_head, hook, pf, indev, outdev,
+  sk, net, okfn);
 
ret = nf_hook_slow(skb, );
}
@@ -195,14 +189,6 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int 
hook,
return ret;
 }
 
-static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
- struct sock *sk, struct sk_buff *skb,
- struct net_device *indev, struct net_device *outdev,
- int (*okfn)(struct net *, struct sock *, struct 
sk_buff *))
-{
-   return nf_hook_thresh(pf, hook, net, sk, skb, indev, outdev, okfn, 
INT_MIN);
-}
-   
 /* Activate hook; either okfn or kfree_skb called, unless a hook
returns NF_STOLEN (in which case, it's up to the hook to deal with
the consequences).
@@ -221,19 +207,6 @@ static inline int nf_hook(u_int8_t pf, unsigned int hook, 
struct net *net,
 */
 
 static inline int
-NF_HOOK_THRESH(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
-  struct sk_buff *skb, struct net_device *in,
-  struct net_device *out,
-  int (*okfn)(struct net *, struct sock *,

[PATCH nf-next,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

2016-10-13 Thread Pablo Neira Ayuso 
Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
introduced br_nf_hook_thresh().

Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
br_nf_forward_finish(), so we have no more callers for this macro.

As a result, state->thresh and explicit thresh parameter in the hook
state structure is not required anymore.

And we can get rid of fast forward code in nf_iterate() in the core path
that is only used by br_netfilter to search for the filter hook.

Suggested-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 
---
 include/linux/netfilter.h | 50 +--
 include/linux/netfilter_ingress.h |  2 +-
 net/bridge/br_netfilter_hooks.c   |  8 +++---
 net/bridge/netfilter/ebtable_broute.c |  2 +-
 net/netfilter/core.c  |  4 ---
 net/netfilter/nf_queue.c  |  1 -
 6 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index abc7fdcb9eb1..e0d000f6c9bf 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -49,7 +49,6 @@ struct sock;
 
 struct nf_hook_state {
unsigned int hook;
-   int thresh;
u_int8_t pf;
struct net_device *in;
struct net_device *out;
@@ -84,7 +83,7 @@ struct nf_hook_entry {
 static inline void nf_hook_state_init(struct nf_hook_state *p,
  struct nf_hook_entry *hook_entry,
  unsigned int hook,
- int thresh, u_int8_t pf,
+ u_int8_t pf,
  struct net_device *indev,
  struct net_device *outdev,
  struct sock *sk,
@@ -92,7 +91,6 @@ static inline void nf_hook_state_init(struct nf_hook_state *p,
  int (*okfn)(struct net *, struct sock *, 
struct sk_buff *))
 {
p->hook = hook;
-   p->thresh = thresh;
p->pf = pf;
p->in = indev;
p->out = outdev;
@@ -155,20 +153,16 @@ extern struct static_key 
nf_hooks_needed[NFPROTO_NUMPROTO][NF_MAX_HOOKS];
 int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state);
 
 /**
- * nf_hook_thresh - call a netfilter hook
+ * nf_hook - call a netfilter hook
  *
  * Returns 1 if the hook has allowed the packet to pass.  The function
  * okfn must be invoked by the caller in this case.  Any other return
  * value indicates the packet has been consumed by the hook.
  */
-static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook,
-struct net *net,
-struct sock *sk,
-struct sk_buff *skb,
-struct net_device *indev,
-struct net_device *outdev,
-int (*okfn)(struct net *, struct sock *, 
struct sk_buff *),
-int thresh)
+static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
+ struct sock *sk, struct sk_buff *skb,
+ struct net_device *indev, struct net_device *outdev,
+ int (*okfn)(struct net *, struct sock *, struct 
sk_buff *))
 {
struct nf_hook_entry *hook_head;
int ret = 1;
@@ -185,8 +179,8 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int 
hook,
if (hook_head) {
struct nf_hook_state state;
 
-   nf_hook_state_init(, hook_head, hook, thresh,
-  pf, indev, outdev, sk, net, okfn);
+   nf_hook_state_init(, hook_head, hook, pf, indev, outdev,
+  sk, net, okfn);
 
ret = nf_hook_slow(skb, );
}
@@ -195,14 +189,6 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int 
hook,
return ret;
 }
 
-static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
- struct sock *sk, struct sk_buff *skb,
- struct net_device *indev, struct net_device *outdev,
- int (*okfn)(struct net *, struct sock *, struct 
sk_buff *))
-{
-   return nf_hook_thresh(pf, hook, net, sk, skb, indev, outdev, okfn, 
INT_MIN);
-}
-   
 /* Activate hook; either okfn or kfree_skb called, unless a hook
returns NF_STOLEN (in which case, it's up to the hook to deal with
the consequences).
@@ -221,19 +207,6 @@ static inline int nf_hook(u_int8_t pf, unsigned int hook, 
struct net *net,
 */
 
 static inline int
-NF_HOOK_THRESH(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
-  struct sk_buff *skb, struct net_device *in,
-  struct net_device *out,
-  int (*okfn)(struct net *, struct sock *,