Patchwork [2/6] netfilter: decnet: switch hook PFs to nfproto

login
register
mail settings
Submitter Alban Crequy
Date May 14, 2012, 1:56 p.m.
Message ID <1337003799-2517-2-git-send-email-alban.crequy@collabora.co.uk>
Download mbox | patch
Permalink /patch/159023/
State Accepted
Headers show

Comments

Alban Crequy - May 14, 2012, 1:56 p.m.
NFPROTO_* constants were usually equal to PF_* constants but it is not
necessary and it will waste less memory if we don't do so (see commit 7e9c6e
"netfilter: Introduce NFPROTO_* constants")

Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
---
 net/decnet/netfilter/dn_rtmsg.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
David Laight - May 14, 2012, 2:18 p.m.
> NFPROTO_* constants were usually equal to PF_* constants but it is not
> necessary and it will waste less memory if we don't do so 
> (see commit 7e9c6e
> "netfilter: Introduce NFPROTO_* constants")
...
>  
>  static struct nf_hook_ops dnrmg_ops __read_mostly = {
>  	.hook		= dnrmg_hook,
> -	.pf		= PF_DECnet,
> +	.pf		= NFPROTO_DECNET,
>  	.hooknum	= NF_DN_ROUTE,
>  	.priority	= NF_DN_PRI_DNRTMSG,
>  };

Might it be worth renaming the .pf member to (say) .nfproto
to help avoid confusion?

	David


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Westphal - May 14, 2012, 2:22 p.m.
David Laight <David.Laight@ACULAB.COM> wrote:
>  
> > NFPROTO_* constants were usually equal to PF_* constants but it is not
> > necessary and it will waste less memory if we don't do so 
> > (see commit 7e9c6e
> > "netfilter: Introduce NFPROTO_* constants")
> ...
> >  
> >  static struct nf_hook_ops dnrmg_ops __read_mostly = {
> >  	.hook		= dnrmg_hook,
> > -	.pf		= PF_DECnet,
> > +	.pf		= NFPROTO_DECNET,
> >  	.hooknum	= NF_DN_ROUTE,
> >  	.priority	= NF_DN_PRI_DNRTMSG,
> >  };
> 
> Might it be worth renaming the .pf member to (say) .nfproto
> to help avoid confusion?

NFPROTO_* values are exported to userspace, so I don't think
its safe to change these values.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - May 14, 2012, 2:38 p.m.
On Mon, May 14, 2012 at 03:18:16PM +0100, David Laight wrote:
>  
> > NFPROTO_* constants were usually equal to PF_* constants but it is not
> > necessary and it will waste less memory if we don't do so 
> > (see commit 7e9c6e
> > "netfilter: Introduce NFPROTO_* constants")
> ...
> >  
> >  static struct nf_hook_ops dnrmg_ops __read_mostly = {
> >  	.hook		= dnrmg_hook,
> > -	.pf		= PF_DECnet,
> > +	.pf		= NFPROTO_DECNET,
> >  	.hooknum	= NF_DN_ROUTE,
> >  	.priority	= NF_DN_PRI_DNRTMSG,
> >  };
> 
> Might it be worth renaming the .pf member to (say) .nfproto
> to help avoid confusion?

That can be done follow-up patch, I guess.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - May 14, 2012, 2:45 p.m.
@Jan,

I remember you introduced all this NFPROTO_* thing time ago.

Any complain on this patchset?

Thanks.

On Mon, May 14, 2012 at 02:56:35PM +0100, Alban Crequy wrote:
> NFPROTO_* constants were usually equal to PF_* constants but it is not
> necessary and it will waste less memory if we don't do so (see commit 7e9c6e
> "netfilter: Introduce NFPROTO_* constants")
> 
> Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
> ---
>  net/decnet/netfilter/dn_rtmsg.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
> index 1531135..7fb7250 100644
> --- a/net/decnet/netfilter/dn_rtmsg.c
> +++ b/net/decnet/netfilter/dn_rtmsg.c
> @@ -118,7 +118,7 @@ static inline void dnrmg_receive_user_skb(struct sk_buff *skb)
>  
>  static struct nf_hook_ops dnrmg_ops __read_mostly = {
>  	.hook		= dnrmg_hook,
> -	.pf		= PF_DECnet,
> +	.pf		= NFPROTO_DECNET,
>  	.hooknum	= NF_DN_ROUTE,
>  	.priority	= NF_DN_PRI_DNRTMSG,
>  };
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt - May 14, 2012, 3:06 p.m.
On Monday 2012-05-14 16:18, David Laight wrote:

> 
>> NFPROTO_* constants were usually equal to PF_* constants but it is not
>> necessary and it will waste less memory if we don't do so 
>> (see commit 7e9c6e
>> "netfilter: Introduce NFPROTO_* constants")
>...
>>  
>>  static struct nf_hook_ops dnrmg_ops __read_mostly = {
>>  	.hook		= dnrmg_hook,
>> -	.pf		= PF_DECnet,
>> +	.pf		= NFPROTO_DECNET,
>>  	.hooknum	= NF_DN_ROUTE,
>>  	.priority	= NF_DN_PRI_DNRTMSG,
>>  };
>
>Might it be worth renaming the .pf member to (say) .nfproto
>to help avoid confusion?

Yes that seems like a sensible thing.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - June 6, 2012, 12:02 a.m.
On Mon, May 14, 2012 at 02:56:35PM +0100, Alban Crequy wrote:
> NFPROTO_* constants were usually equal to PF_* constants but it is not
> necessary and it will waste less memory if we don't do so (see commit 7e9c6e
> "netfilter: Introduce NFPROTO_* constants")

Applied, thanks.

But I rewrote the description to just say that this is a consistency
cleanup (other hooks use NFPROTO_*).

Hm, by grepping net/netfilter for PF_* still some netfilter subsystems
(like ipset) show up using it.

Perhaps we can take another patch for these.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index 1531135..7fb7250 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -118,7 +118,7 @@  static inline void dnrmg_receive_user_skb(struct sk_buff *skb)
 
 static struct nf_hook_ops dnrmg_ops __read_mostly = {
 	.hook		= dnrmg_hook,
-	.pf		= PF_DECnet,
+	.pf		= NFPROTO_DECNET,
 	.hooknum	= NF_DN_ROUTE,
 	.priority	= NF_DN_PRI_DNRTMSG,
 };