| Message ID | 1337003799-2517-2-git-send-email-alban.crequy@collabora.co.uk |
|---|---|
| State | Accepted |
| Headers | show |
> 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
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
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
@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
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
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
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, };