Hi, On Wed, Oct 21, 2015 at 11:22:41PM +0800, Feng Gao wrote: > When execute "insmod nf_conntrack_ftp.ko ports=76,76", the kernel crashes > immediately.Because the old codes try to unregister the helper which is > not registered successfully. > Now add new function nf_conntrack_helpers_register to fix this bug. > It would unregister the right helpers automatically if someone fails. > > Signed-off-by: Gao Feng <fgao@xxxxxxxxxx> > --- > include/net/netfilter/nf_conntrack_helper.h | 5 ++++ > net/netfilter/nf_conntrack_ftp.c | 33 > ++++++-------------------- > net/netfilter/nf_conntrack_helper.c | 29 +++++++++++++++++++++++ > net/netfilter/nf_conntrack_irc.c | 20 ++++++--------- > net/netfilter/nf_conntrack_sane.c | 27 ++++++--------------- > net/netfilter/nf_conntrack_sip.c | 25 +++++-------------- > net/netfilter/nf_conntrack_tftp.c | 22 +++++------------ > 7 files changed, 72 insertions(+), 89 deletions(-) > > diff --git a/include/net/netfilter/nf_conntrack_helper.h > b/include/net/netfilter/nf_conntrack_helper.h > index 6cf614b..a6be9da 100644 > --- a/include/net/netfilter/nf_conntrack_helper.h > +++ b/include/net/netfilter/nf_conntrack_helper.h > @@ -62,6 +62,11 @@ struct nf_conntrack_helper > *nf_conntrack_helper_try_module_get(const char *name, > int nf_conntrack_helper_register(struct nf_conntrack_helper *); > void nf_conntrack_helper_unregister(struct nf_conntrack_helper *); > > +int nf_conntrack_helpers_register(struct nf_conntrack_helper *, > + unsigned > int); Thanks for following up on this. Please, get familiarized with the Linux kernel coding style: http://lxr.free-electrons.com/source/Documentation/CodingStyle On top of that, it would be great if you can add a nf_ct_helper_init() generic function that we can reuse to configure the helper in first place. This initialization happens over and over again, it would help to consolidate the existing infrastructure. Then, we can untangle the existing code by using a unidimensional array instead. Upon that initial patch to add nf_ct_helper_init() you can cook a second patch to add the new nf_conntrack_helpers_register() functions. I didn't compile tested this, but I would expect that the result looks similar to the code snippet below. -o- static struct nf_conntrack_helper sip[MAX_PORTS * 4] __read_mostly; static int __init nf_conntrack_sip_init(void) { struct nf_conntrack_helper tmpl[4]; int i, j, ret; if (ports_c == 0) ports[ports_c++] = SIP_PORT; memset(tmpl, 0, sizeof(tmpl)); nf_ct_helper_init(&tmpl[0], AF_INET, IPPROTO_UDP, sip_help_udp, sizeof(struct nf_ct_sip_master), sip_exp_policy, SIP_EXPECT_MAX, THIS_MODULE); nf_ct_helper_init(&tmpl[1], AF_INET, IPPROTO_TCP, sip_help_tcp, sizeof(struct nf_ct_sip_master), sip_exp_policy, SIP_EXPECT_MAX, THIS_MODULE); nf_ct_helper_init(&tmpl[2], AF_INET6, IPPROTO_UDP, sip_help_udp, sizeof(struct nf_ct_sip_master), sip_exp_policy, SIP_EXPECT_MAX, THIS_MODULE); nf_ct_helper_init(&tmpl[3], AF_INET6, IPPROTO_TCP, sip_help_tcp, sizeof(struct nf_ct_sip_master), sip_exp_policy, SIP_EXPECT_MAX, THIS_MODULE); for (i = 0; i < ARRAY_SIZE(sip); i += ARRAY_SIZE(tmpl)) { memcpy(&sip[i], tmpl, sizeof(tmpl)); for (j = 0; j < ARRAY_SIZE(tmpl); j++) { if (sip[i + j] == SIP_PORT) sprintf(sip[i][j].name, "sip"); else sprintf(sip[i][j].name, "sip-%u", i); } return nf_conntrack_helpers_register(sip, ARRAY_SIZE(sip)); } -o- The only variable part is the name, the remaining (most of it) can be arranged in the template form above. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
