[PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook_entry
@ 2016-10-27 18:27 Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries Aaron Conole
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Aaron Conole @ 2016-10-27 18:27 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Pablo Neira Ayuso, Florian Westphal

This series reduces the size of the nf_hook_entry objects, and converts the
loops which traverse into for-loops.  This will facilitate future work to
switch from a singly-linked list into an array.

This series was built and tested on top of Pablo's series posted at
https://www.spinics.net/lists/netfilter-devel/msg44408.html

Aaron Conole (3):
  netfilter: introduce accessor functions for hook entries
  netfilter: decouple nf_hook_entry and nf_hook_ops
  netfilter: Convert while loops to for-loops

 include/linux/netfilter.h       | 42 +++++++++++++++++++++++++++++++++++++----
 net/bridge/br_netfilter_hooks.c |  8 ++++----
 net/netfilter/core.c            | 14 +++++---------
 net/netfilter/nf_queue.c        | 11 +++++------
 4 files changed, 52 insertions(+), 23 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries
  2016-10-27 18:27 [PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook_entry Aaron Conole
@ 2016-10-27 18:27 ` Aaron Conole
  2016-11-03 16:15   ` Pablo Neira Ayuso
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 2/3] netfilter: decouple nf_hook_entry and nf_hook_ops Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 3/3] netfilter: convert while loops to for loops Aaron Conole
  2 siblings, 1 reply; 6+ messages in thread
From: Aaron Conole @ 2016-10-27 18:27 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Pablo Neira Ayuso, Florian Westphal

This allows easier future refactoring.

Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
 include/linux/netfilter.h       | 35 ++++++++++++++++++++++++++++++++++-
 net/bridge/br_netfilter_hooks.c |  2 +-
 net/netfilter/core.c            |  8 +++-----
 net/netfilter/nf_queue.c        |  8 ++++----
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index d0beb607..b25386b 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -80,6 +80,38 @@ struct nf_hook_entry {
 	const struct nf_hook_ops	*orig_ops;
 };
 
+static inline void
+nf_hook_entry_init(struct nf_hook_entry *entry,	const struct nf_hook_ops *ops)
+{
+	entry->next = NULL;
+	entry->ops = *ops;
+	entry->orig_ops = ops;
+}
+
+static inline int
+nf_hook_entry_priority(const struct nf_hook_entry *entry)
+{
+	return entry->ops.priority;
+}
+
+static inline nf_hookfn *
+nf_hook_entry_hookfn(const struct nf_hook_entry *entry)
+{
+	return entry->ops.hook;
+}
+
+static inline void *
+nf_hook_entry_priv(const struct nf_hook_entry *entry)
+{
+	return entry->ops.priv;
+}
+
+static inline const struct nf_hook_ops *
+nf_hook_entry_ops(const struct nf_hook_entry *entry)
+{
+	return entry->orig_ops;
+}
+
 static inline void nf_hook_state_init(struct nf_hook_state *p,
 				      struct nf_hook_entry *hook_entry,
 				      unsigned int hook,
@@ -164,7 +196,8 @@ static inline int nf_hook_iterate(struct sk_buff *skb,
 	while (entry) {
 		RCU_INIT_POINTER(state->hook_entries, entry);
 repeat:
-		verdict = entry->ops.hook(entry->ops.priv, skb, state);
+		verdict = nf_hook_entry_hookfn(entry)(nf_hook_entry_priv(entry),
+						      skb, state);
 		switch (verdict) {
 		case NF_ACCEPT:
 			entry = rcu_dereference(entry->next);
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index d153925..37c4d59 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -1010,7 +1010,7 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net,
 
 	elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
 
-	while (elem && (elem->ops.priority <= NF_BR_PRI_BRNF))
+	while (elem && (nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF))
 		elem = rcu_dereference(elem->next);
 
 	if (!elem)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 5cf9415..48782d4 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -102,15 +102,13 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 	if (!entry)
 		return -ENOMEM;
 
-	entry->orig_ops	= reg;
-	entry->ops	= *reg;
-	entry->next	= NULL;
+	nf_hook_entry_init(entry, reg);
 
 	mutex_lock(&nf_hook_mutex);
 
 	/* Find the spot in the list */
 	while ((p = nf_entry_dereference(*pp)) != NULL) {
-		if (reg->priority < p->orig_ops->priority)
+		if (reg->priority < nf_hook_entry_priority(p))
 			break;
 		pp = &p->next;
 	}
@@ -140,7 +138,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 
 	mutex_lock(&nf_hook_mutex);
 	while ((p = nf_entry_dereference(*pp)) != NULL) {
-		if (p->orig_ops == reg) {
+		if (nf_hook_entry_ops(p) == reg) {
 			rcu_assign_pointer(*pp, p->next);
 			break;
 		}
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 2d5dca0..2d0dc56 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -186,7 +186,8 @@ static unsigned int nf_iterate(struct sk_buff *skb,
 	while (*entryp) {
 		RCU_INIT_POINTER(state->hook_entries, *entryp);
 repeat:
-		verdict = (*entryp)->ops.hook((*entryp)->ops.priv, skb, state);
+		verdict = nf_hook_entry_hookfn(*entryp)
+			(nf_hook_entry_priv(*entryp), skb, state);
 		if (verdict != NF_ACCEPT) {
 			if (verdict != NF_REPEAT)
 				return verdict;
@@ -202,7 +203,6 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
 	struct nf_hook_entry *hook_entry;
 	struct sk_buff *skb = entry->skb;
 	const struct nf_afinfo *afinfo;
-	struct nf_hook_ops *elem;
 	int err;
 
 	/* Userspace may request to enqueue this packet again. */
@@ -220,13 +220,13 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
 	}
 
 	hook_entry = rcu_dereference(entry->state.hook_entries);
-	elem = &hook_entry->ops;
 
 	nf_queue_entry_release_refs(entry);
 
 	/* Continue traversal iff userspace said ok... */
 	if (verdict == NF_REPEAT)
-		verdict = elem->hook(elem->priv, skb, &entry->state);
+		verdict = nf_hook_entry_hookfn(hook_entry)
+			(nf_hook_entry_priv(hook_entry), skb, &entry->state);
 
 	if (verdict == NF_ACCEPT) {
 		afinfo = nf_get_afinfo(entry->state.pf);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries Aaron Conole
@ 2016-11-03 16:15   ` Pablo Neira Ayuso
  2016-11-03 16:32     ` Aaron Conole
  0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-11-03 16:15 UTC (permalink / raw)
  To: Aaron Conole; +Cc: netfilter-devel, Florian Westphal

On Thu, Oct 27, 2016 at 02:27:51PM -0400, Aaron Conole wrote:
> This allows easier future refactoring.
> 
> Signed-off-by: Aaron Conole <aconole@bytheb.org>
> ---
>  include/linux/netfilter.h       | 35 ++++++++++++++++++++++++++++++++++-
>  net/bridge/br_netfilter_hooks.c |  2 +-
>  net/netfilter/core.c            |  8 +++-----
>  net/netfilter/nf_queue.c        |  8 ++++----
>  4 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> index d0beb607..b25386b 100644
> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -80,6 +80,38 @@ struct nf_hook_entry {
>  	const struct nf_hook_ops	*orig_ops;
>  };
>  
> +static inline void
> +nf_hook_entry_init(struct nf_hook_entry *entry,	const struct nf_hook_ops *ops)
> +{
> +	entry->next = NULL;
> +	entry->ops = *ops;
> +	entry->orig_ops = ops;
> +}
> +
> +static inline int
> +nf_hook_entry_priority(const struct nf_hook_entry *entry)
> +{
> +	return entry->ops.priority;
> +}
> +
> +static inline nf_hookfn *
> +nf_hook_entry_hookfn(const struct nf_hook_entry *entry)
> +{
> +	return entry->ops.hook;
> +}

I'd suggest something like:

static inline int
nf_entry_hookfn(const struct nf_hook_entry *entry,
                struct sk_buff *skb, struct nf_hook_state *state)
{
        return entry->ops.hook(entry, nf_hook_entry_priv(entry), skb, state);
}

So you can avoid this:

		verdict = nf_hook_entry_hookfn(*entryp)
			(nf_hook_entry_priv(*entryp), skb, state);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries
  2016-11-03 16:15   ` Pablo Neira Ayuso
@ 2016-11-03 16:32     ` Aaron Conole
  0 siblings, 0 replies; 6+ messages in thread
From: Aaron Conole @ 2016-11-03 16:32 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Florian Westphal

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> On Thu, Oct 27, 2016 at 02:27:51PM -0400, Aaron Conole wrote:
>> This allows easier future refactoring.
>> 
>> Signed-off-by: Aaron Conole <aconole@bytheb.org>
>> ---
>>  include/linux/netfilter.h       | 35 ++++++++++++++++++++++++++++++++++-
>>  net/bridge/br_netfilter_hooks.c |  2 +-
>>  net/netfilter/core.c            |  8 +++-----
>>  net/netfilter/nf_queue.c        |  8 ++++----
>>  4 files changed, 42 insertions(+), 11 deletions(-)

...

> I'd suggest something like:
>
> static inline int
> nf_entry_hookfn(const struct nf_hook_entry *entry,
>                 struct sk_buff *skb, struct nf_hook_state *state)
> {
>         return entry->ops.hook(entry, nf_hook_entry_priv(entry), skb, state);
> }
>
> So you can avoid this:
>
> 		verdict = nf_hook_entry_hookfn(*entryp)
> 			(nf_hook_entry_priv(*entryp), skb, state);

Makes sense, I'll do that.  Thanks for the review!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH RFC netfilter-next 2/3] netfilter: decouple nf_hook_entry and nf_hook_ops
  2016-10-27 18:27 [PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook_entry Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries Aaron Conole
@ 2016-10-27 18:27 ` Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 3/3] netfilter: convert while loops to for loops Aaron Conole
  2 siblings, 0 replies; 6+ messages in thread
From: Aaron Conole @ 2016-10-27 18:27 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Pablo Neira Ayuso, Florian Westphal

During nfhook traversal we only need a very small subset of
nf_hook_ops members.

We need:
- next element
- hook function to call
- hook function priv argument

Bridge netfilter also needs 'thresh'; can be obtained via ->orig_ops.

nf_hook_entry struct is now 32 bytes on x86_64.

Followup patch will turn the run-time list into an array that only
stores hook functions plus their priv arguments.

Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
 include/linux/netfilter.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index b25386b..cc2d977 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -76,7 +76,8 @@ struct nf_hook_ops {
 
 struct nf_hook_entry {
 	struct nf_hook_entry __rcu	*next;
-	struct nf_hook_ops		ops;
+	nf_hookfn			*hook;
+	void				*priv;
 	const struct nf_hook_ops	*orig_ops;
 };
 
@@ -84,26 +85,27 @@ static inline void
 nf_hook_entry_init(struct nf_hook_entry *entry,	const struct nf_hook_ops *ops)
 {
 	entry->next = NULL;
-	entry->ops = *ops;
+	entry->hook = ops->hook;
+	entry->priv = ops->priv;
 	entry->orig_ops = ops;
 }
 
 static inline int
 nf_hook_entry_priority(const struct nf_hook_entry *entry)
 {
-	return entry->ops.priority;
+	return entry->orig_ops->priority;
 }
 
 static inline nf_hookfn *
 nf_hook_entry_hookfn(const struct nf_hook_entry *entry)
 {
-	return entry->ops.hook;
+	return entry->hook;
 }
 
 static inline void *
 nf_hook_entry_priv(const struct nf_hook_entry *entry)
 {
-	return entry->ops.priv;
+	return entry->priv;
 }
 
 static inline const struct nf_hook_ops *
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RFC netfilter-next 3/3] netfilter: convert while loops to for loops
  2016-10-27 18:27 [PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook_entry Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries Aaron Conole
  2016-10-27 18:27 ` [PATCH RFC netfilter-next 2/3] netfilter: decouple nf_hook_entry and nf_hook_ops Aaron Conole
@ 2016-10-27 18:27 ` Aaron Conole
  2 siblings, 0 replies; 6+ messages in thread
From: Aaron Conole @ 2016-10-27 18:27 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Pablo Neira Ayuso, Florian Westphal

This is to facilitate converting from a singly-linked list to an array
of elements.

Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
 include/linux/netfilter.h       | 3 +--
 net/bridge/br_netfilter_hooks.c | 8 ++++----
 net/netfilter/core.c            | 6 ++----
 net/netfilter/nf_queue.c        | 3 +--
 4 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index cc2d977..89bb370 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -195,14 +195,13 @@ static inline int nf_hook_iterate(struct sk_buff *skb,
 	int ret;
 
 	entry = rcu_dereference(state->hook_entries);
-	while (entry) {
+	for (; entry; entry = rcu_dereference(entry->next)) {
 		RCU_INIT_POINTER(state->hook_entries, entry);
 repeat:
 		verdict = nf_hook_entry_hookfn(entry)(nf_hook_entry_priv(entry),
 						      skb, state);
 		switch (verdict) {
 		case NF_ACCEPT:
-			entry = rcu_dereference(entry->next);
 			break;
 		case NF_DROP:
 			kfree_skb(skb);
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 37c4d59..ec4c409 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -1008,10 +1008,10 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net,
 	struct nf_hook_state state;
 	int ret;
 
-	elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
-
-	while (elem && (nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF))
-		elem = rcu_dereference(elem->next);
+	for (elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);
+	     elem && (nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF);
+	     elem = rcu_dereference(elem->next))
+		;
 
 	if (!elem)
 		return okfn(net, sk, skb);
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 48782d4..52ded39 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -107,10 +107,9 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 	mutex_lock(&nf_hook_mutex);
 
 	/* Find the spot in the list */
-	while ((p = nf_entry_dereference(*pp)) != NULL) {
+	for (; (p = nf_entry_dereference(*pp)) != NULL; pp = &p->next) {
 		if (reg->priority < nf_hook_entry_priority(p))
 			break;
-		pp = &p->next;
 	}
 	rcu_assign_pointer(entry->next, p);
 	rcu_assign_pointer(*pp, entry);
@@ -137,12 +136,11 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 		return;
 
 	mutex_lock(&nf_hook_mutex);
-	while ((p = nf_entry_dereference(*pp)) != NULL) {
+	for (; (p = nf_entry_dereference(*pp)) != NULL; pp = &p->next) {
 		if (nf_hook_entry_ops(p) == reg) {
 			rcu_assign_pointer(*pp, p->next);
 			break;
 		}
-		pp = &p->next;
 	}
 	mutex_unlock(&nf_hook_mutex);
 	if (!p) {
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 2d0dc56..2c53357 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -183,7 +183,7 @@ static unsigned int nf_iterate(struct sk_buff *skb,
 {
 	unsigned int verdict;
 
-	while (*entryp) {
+	for (; *entryp; *entryp = rcu_dereference((*entryp)->next)) {
 		RCU_INIT_POINTER(state->hook_entries, *entryp);
 repeat:
 		verdict = nf_hook_entry_hookfn(*entryp)
@@ -193,7 +193,6 @@ static unsigned int nf_iterate(struct sk_buff *skb,
 				return verdict;
 			goto repeat;
 		}
-		*entryp = rcu_dereference((*entryp)->next);
 	}
 	return NF_ACCEPT;
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-11-03 16:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-27 18:27 [PATCH RFC netfilter-next 0/3] Additional refactoring enhancements for nf_hook_entry Aaron Conole
2016-10-27 18:27 ` [PATCH RFC netfilter-next 1/3] netfilter: introduce accessor functions for hook entries Aaron Conole
2016-11-03 16:15   ` Pablo Neira Ayuso
2016-11-03 16:32     ` Aaron Conole
2016-10-27 18:27 ` [PATCH RFC netfilter-next 2/3] netfilter: decouple nf_hook_entry and nf_hook_ops Aaron Conole
2016-10-27 18:27 ` [PATCH RFC netfilter-next 3/3] netfilter: convert while loops to for loops Aaron Conole

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.