| Message ID | 1339818083-31356-6-git-send-email-gaofeng@cn.fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote: > merge udpv4_net_init and udpv6_net_init into udp_net_init to > reduce the redundancy codes. > > and use nf_proto_net.users to identify if it's the first time > we use the nf_proto_net. when it's the first time,we will > initialized it. > > Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> > --- > net/netfilter/nf_conntrack_proto_udp.c | 56 ++++++++++--------------------- > 1 files changed, 18 insertions(+), 38 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c > index 2b978e6..61bca4f 100644 > --- a/net/netfilter/nf_conntrack_proto_udp.c > +++ b/net/netfilter/nf_conntrack_proto_udp.c > @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) > return 0; > } > > -static void udp_init_net_data(struct nf_udp_net *un) > +static int udp_init_net(struct net *net, u_int16_t proto) > { > - int i; > -#ifdef CONFIG_SYSCTL > - if (!un->pn.ctl_table) { > -#else > - if (!un->pn.users++) { > -#endif > + int ret; > + struct nf_udp_net *un = udp_pernet(net); > + struct nf_proto_net *pn = &un->pn; > + > + if (!pn->users) { > + int i; > for (i = 0; i < UDP_CT_MAX; i++) > un->timeouts[i] = udp_timeouts[i]; > } > -} > - > -static int udpv4_init_net(struct net *net, u_int16_t proto) > -{ > - int ret; > - struct nf_udp_net *un = udp_pernet(net); > - struct nf_proto_net *pn = (struct nf_proto_net *)un; > > - udp_init_net_data(un); > + if (proto == AF_INET) { I think we can remove that u_int16_t proto that I proposed to make something like: static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) { #ifdef CONFIG_SYSCTL #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT struct nf_udp_net *un = (struct nf_udp_net *)pn; + + if (pn->ctl_compat_table) + return 0; + pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table, sizeof(udp_compat_sysctl_table), GFP_KERNEL); if (!pn->ctl_compat_table) return -ENOMEM; That should be enough to ensure that the compat is registered once. No matter if it's done by the IPv4 or IPv6 invocation of udp_init_net. Thus, it will look consistent with udp_kmemdup_sysctl_table. > + ret = udp_kmemdup_compat_sysctl_table(pn); > + if (ret < 0) > + return ret; > > - ret = udp_kmemdup_compat_sysctl_table(pn); > - if (ret < 0) > - return ret; > + ret = udp_kmemdup_sysctl_table(pn); > + if (ret < 0) > + nf_ct_kfree_compat_sysctl_table(pn); > + } else > + ret = udp_kmemdup_sysctl_table(pn); > > - ret = udp_kmemdup_sysctl_table(pn); > -#ifdef CONFIG_SYSCTL > -#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT > - if (ret < 0) { > - kfree(pn->ctl_compat_table); > - pn->ctl_compat_table = NULL; > - } > -#endif > -#endif > return ret; > } > > -static int udpv6_init_net(struct net *net, u_int16_t proto) > -{ > - struct nf_udp_net *un = udp_pernet(net); > - struct nf_proto_net *pn = (struct nf_proto_net *)un; > - > - udp_init_net_data(un); > - return udp_kmemdup_sysctl_table(pn); > -} > - > struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly = > { > .l3proto = PF_INET, > @@ -343,7 +323,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly = > .nla_policy = udp_timeout_nla_policy, > }, > #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */ > - .init_net = udpv4_init_net, > + .init_net = udp_init_net, > }; > EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp4); > > @@ -374,6 +354,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp6 __read_mostly = > .nla_policy = udp_timeout_nla_policy, > }, > #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */ > - .init_net = udpv6_init_net, > + .init_net = udp_init_net, > }; > EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp6); > -- > 1.7.7.6 > > -- > 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
于 2012年06月16日 19:22, Pablo Neira Ayuso 写道: > On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote: >> merge udpv4_net_init and udpv6_net_init into udp_net_init to >> reduce the redundancy codes. >> >> and use nf_proto_net.users to identify if it's the first time >> we use the nf_proto_net. when it's the first time,we will >> initialized it. >> >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> >> --- >> net/netfilter/nf_conntrack_proto_udp.c | 56 ++++++++++--------------------- >> 1 files changed, 18 insertions(+), 38 deletions(-) >> >> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c >> index 2b978e6..61bca4f 100644 >> --- a/net/netfilter/nf_conntrack_proto_udp.c >> +++ b/net/netfilter/nf_conntrack_proto_udp.c >> @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) >> return 0; >> } >> >> -static void udp_init_net_data(struct nf_udp_net *un) >> +static int udp_init_net(struct net *net, u_int16_t proto) >> { >> - int i; >> -#ifdef CONFIG_SYSCTL >> - if (!un->pn.ctl_table) { >> -#else >> - if (!un->pn.users++) { >> -#endif >> + int ret; >> + struct nf_udp_net *un = udp_pernet(net); >> + struct nf_proto_net *pn = &un->pn; >> + >> + if (!pn->users) { >> + int i; >> for (i = 0; i < UDP_CT_MAX; i++) >> un->timeouts[i] = udp_timeouts[i]; >> } >> -} >> - >> -static int udpv4_init_net(struct net *net, u_int16_t proto) >> -{ >> - int ret; >> - struct nf_udp_net *un = udp_pernet(net); >> - struct nf_proto_net *pn = (struct nf_proto_net *)un; >> >> - udp_init_net_data(un); >> + if (proto == AF_INET) { > > I think we can remove that u_int16_t proto that I proposed to make > something like: > > static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) > { > #ifdef CONFIG_SYSCTL > #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT > struct nf_udp_net *un = (struct nf_udp_net *)pn; > + > + if (pn->ctl_compat_table) > + return 0; > + > pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table, > sizeof(udp_compat_sysctl_table), > GFP_KERNEL); > if (!pn->ctl_compat_table) > return -ENOMEM; > > That should be enough to ensure that the compat is registered once. No > matter if it's done by the IPv4 or IPv6 invocation of udp_init_net. > > Thus, it will look consistent with udp_kmemdup_sysctl_table. yes, but this will be very terrible to unregister compat sysctl and free compat sysctl table. thinking about, we may insmod nf_conntrack_ipv6 only, as your idea, we will allocate compat_sysctl_table.so we have to free it when rmmod nf_conntrack_ipv6. in order to implement it, we have to change the logic of nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we only free the sysctl table when we unregister the proto. -- 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
于 2012年06月19日 16:08, Gao feng 写道: > > in order to implement it, we have to change the logic of > nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we > only free the sysctl table when we unregister the proto. I means free the sysctl table when we register sysctl success (pn->ctl_table_header != NULL). -- 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 Tue, Jun 19, 2012 at 04:08:31PM +0800, Gao feng wrote: > 于 2012年06月16日 19:22, Pablo Neira Ayuso 写道: > > On Sat, Jun 16, 2012 at 11:41:17AM +0800, Gao feng wrote: > >> merge udpv4_net_init and udpv6_net_init into udp_net_init to > >> reduce the redundancy codes. > >> > >> and use nf_proto_net.users to identify if it's the first time > >> we use the nf_proto_net. when it's the first time,we will > >> initialized it. > >> > >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> > >> --- > >> net/netfilter/nf_conntrack_proto_udp.c | 56 ++++++++++--------------------- > >> 1 files changed, 18 insertions(+), 38 deletions(-) > >> > >> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c > >> index 2b978e6..61bca4f 100644 > >> --- a/net/netfilter/nf_conntrack_proto_udp.c > >> +++ b/net/netfilter/nf_conntrack_proto_udp.c > >> @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) > >> return 0; > >> } > >> > >> -static void udp_init_net_data(struct nf_udp_net *un) > >> +static int udp_init_net(struct net *net, u_int16_t proto) > >> { > >> - int i; > >> -#ifdef CONFIG_SYSCTL > >> - if (!un->pn.ctl_table) { > >> -#else > >> - if (!un->pn.users++) { > >> -#endif > >> + int ret; > >> + struct nf_udp_net *un = udp_pernet(net); > >> + struct nf_proto_net *pn = &un->pn; > >> + > >> + if (!pn->users) { > >> + int i; > >> for (i = 0; i < UDP_CT_MAX; i++) > >> un->timeouts[i] = udp_timeouts[i]; > >> } > >> -} > >> - > >> -static int udpv4_init_net(struct net *net, u_int16_t proto) > >> -{ > >> - int ret; > >> - struct nf_udp_net *un = udp_pernet(net); > >> - struct nf_proto_net *pn = (struct nf_proto_net *)un; > >> > >> - udp_init_net_data(un); > >> + if (proto == AF_INET) { > > > > I think we can remove that u_int16_t proto that I proposed to make > > something like: > > > > static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) > > { > > #ifdef CONFIG_SYSCTL > > #ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT > > struct nf_udp_net *un = (struct nf_udp_net *)pn; > > + > > + if (pn->ctl_compat_table) > > + return 0; > > + > > pn->ctl_compat_table = kmemdup(udp_compat_sysctl_table, > > sizeof(udp_compat_sysctl_table), > > GFP_KERNEL); > > if (!pn->ctl_compat_table) > > return -ENOMEM; > > > > That should be enough to ensure that the compat is registered once. No > > matter if it's done by the IPv4 or IPv6 invocation of udp_init_net. > > > > Thus, it will look consistent with udp_kmemdup_sysctl_table. > > > yes, but this will be very terrible to unregister compat sysctl > and free compat sysctl table. > > thinking about, we may insmod nf_conntrack_ipv6 only, as your idea, > we will allocate compat_sysctl_table.so we have to free it when > rmmod nf_conntrack_ipv6. You're right. > in order to implement it, we have to change the logic of > nf_ct_l4proto_unregister_sysctl and nf_ct_unregister_sysctl. because we > only free the sysctl table when we unregister the proto. OK, let's stick to what we have then. -- 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/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c index 2b978e6..61bca4f 100644 --- a/net/netfilter/nf_conntrack_proto_udp.c +++ b/net/netfilter/nf_conntrack_proto_udp.c @@ -270,52 +270,32 @@ static int udp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn) return 0; } -static void udp_init_net_data(struct nf_udp_net *un) +static int udp_init_net(struct net *net, u_int16_t proto) { - int i; -#ifdef CONFIG_SYSCTL - if (!un->pn.ctl_table) { -#else - if (!un->pn.users++) { -#endif + int ret; + struct nf_udp_net *un = udp_pernet(net); + struct nf_proto_net *pn = &un->pn; + + if (!pn->users) { + int i; for (i = 0; i < UDP_CT_MAX; i++) un->timeouts[i] = udp_timeouts[i]; } -} - -static int udpv4_init_net(struct net *net, u_int16_t proto) -{ - int ret; - struct nf_udp_net *un = udp_pernet(net); - struct nf_proto_net *pn = (struct nf_proto_net *)un; - udp_init_net_data(un); + if (proto == AF_INET) { + ret = udp_kmemdup_compat_sysctl_table(pn); + if (ret < 0) + return ret; - ret = udp_kmemdup_compat_sysctl_table(pn); - if (ret < 0) - return ret; + ret = udp_kmemdup_sysctl_table(pn); + if (ret < 0) + nf_ct_kfree_compat_sysctl_table(pn); + } else + ret = udp_kmemdup_sysctl_table(pn); - ret = udp_kmemdup_sysctl_table(pn); -#ifdef CONFIG_SYSCTL -#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT - if (ret < 0) { - kfree(pn->ctl_compat_table); - pn->ctl_compat_table = NULL; - } -#endif -#endif return ret; } -static int udpv6_init_net(struct net *net, u_int16_t proto) -{ - struct nf_udp_net *un = udp_pernet(net); - struct nf_proto_net *pn = (struct nf_proto_net *)un; - - udp_init_net_data(un); - return udp_kmemdup_sysctl_table(pn); -} - struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly = { .l3proto = PF_INET, @@ -343,7 +323,7 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp4 __read_mostly = .nla_policy = udp_timeout_nla_policy, }, #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */ - .init_net = udpv4_init_net, + .init_net = udp_init_net, }; EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp4); @@ -374,6 +354,6 @@ struct nf_conntrack_l4proto nf_conntrack_l4proto_udp6 __read_mostly = .nla_policy = udp_timeout_nla_policy, }, #endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */ - .init_net = udpv6_init_net, + .init_net = udp_init_net, }; EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_udp6);
merge udpv4_net_init and udpv6_net_init into udp_net_init to reduce the redundancy codes. and use nf_proto_net.users to identify if it's the first time we use the nf_proto_net. when it's the first time,we will initialized it. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- net/netfilter/nf_conntrack_proto_udp.c | 56 ++++++++++--------------------- 1 files changed, 18 insertions(+), 38 deletions(-)