* [GIT PULL nf-next] IPVS
@ 2012-07-11 0:25 Simon Horman
2012-07-11 0:25 ` [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper Simon Horman
2012-07-11 0:25 ` [PATCH 2/2] ipvs: generalize app registration in netns Simon Horman
0 siblings, 2 replies; 12+ messages in thread
From: Simon Horman @ 2012-07-11 0:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer
Hi Pablo,
please consider the following enhancements to IPVS for inclusion in 3.6.
----------------------------------------------------------------
The following changes since commit 46ba5a25f521e3c50d7bb81b1abb977769047456:
netfilter: nfnetlink_queue: do not allow to set unsupported flag bits (2012-07-04 19:51:50 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git master
for you to fetch changes up to 1fd130ebf10e1185022a9c0470f2298943bad1c4:
ipvs: generalize app registration in netns (2012-07-10 17:58:10 +0900)
----------------------------------------------------------------
Julian Anastasov (2):
ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper
ipvs: generalize app registration in netns
include/net/ip_vs.h | 5 ++--
net/netfilter/ipvs/Kconfig | 3 ++-
net/netfilter/ipvs/ip_vs_app.c | 61 +++++++++++++++++++++++++++++++-----------
net/netfilter/ipvs/ip_vs_ftp.c | 21 ++++-----------
4 files changed, 54 insertions(+), 36 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-11 0:25 [GIT PULL nf-next] IPVS Simon Horman @ 2012-07-11 0:25 ` Simon Horman 2012-07-12 15:39 ` Pablo Neira Ayuso 2012-07-11 0:25 ` [PATCH 2/2] ipvs: generalize app registration in netns Simon Horman 1 sibling, 1 reply; 12+ messages in thread From: Simon Horman @ 2012-07-11 0:25 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang, Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer, Simon Horman From: Julian Anastasov <ja@ssi.bg> The FTP application indirectly depends on the nf_conntrack_ftp helper for proper NAT support. If the module is not loaded, IPVS can resize the packets for the command connection, eg. PASV response but the SEQ adjustment logic in ipv4_confirm is not called without helper. Signed-off-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Simon Horman <horms@verge.net.au> --- net/netfilter/ipvs/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig index f987138..8b2cffd 100644 --- a/net/netfilter/ipvs/Kconfig +++ b/net/netfilter/ipvs/Kconfig @@ -250,7 +250,8 @@ comment 'IPVS application helper' config IP_VS_FTP tristate "FTP protocol helper" - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ + NF_CONNTRACK_FTP select IP_VS_NFCT ---help--- FTP is a protocol that transfers IP address and/or port number in -- 1.7.10.2.484.gcd07cc5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-11 0:25 ` [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper Simon Horman @ 2012-07-12 15:39 ` Pablo Neira Ayuso 2012-07-12 19:43 ` Julian Anastasov 0 siblings, 1 reply; 12+ messages in thread From: Pablo Neira Ayuso @ 2012-07-12 15:39 UTC (permalink / raw) To: Simon Horman Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang, Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote: > From: Julian Anastasov <ja@ssi.bg> > > The FTP application indirectly depends on the > nf_conntrack_ftp helper for proper NAT support. If the > module is not loaded, IPVS can resize the packets for the > command connection, eg. PASV response but the SEQ adjustment > logic in ipv4_confirm is not called without helper. > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > Signed-off-by: Simon Horman <horms@verge.net.au> > --- > net/netfilter/ipvs/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig > index f987138..8b2cffd 100644 > --- a/net/netfilter/ipvs/Kconfig > +++ b/net/netfilter/ipvs/Kconfig > @@ -250,7 +250,8 @@ comment 'IPVS application helper' > > config IP_VS_FTP > tristate "FTP protocol helper" > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ > + NF_CONNTRACK_FTP If you require FTP NAT support, then this depends on NF_NAT_FTP instead of NF_CONNTRACK_FTP. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-12 15:39 ` Pablo Neira Ayuso @ 2012-07-12 19:43 ` Julian Anastasov 2012-07-23 6:48 ` Simon Horman 0 siblings, 1 reply; 12+ messages in thread From: Julian Anastasov @ 2012-07-12 19:43 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer Hello, On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote: > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote: > > From: Julian Anastasov <ja@ssi.bg> > > > > The FTP application indirectly depends on the > > nf_conntrack_ftp helper for proper NAT support. If the > > module is not loaded, IPVS can resize the packets for the > > command connection, eg. PASV response but the SEQ adjustment > > logic in ipv4_confirm is not called without helper. > > > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > > Signed-off-by: Simon Horman <horms@verge.net.au> > > --- > > net/netfilter/ipvs/Kconfig | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig > > index f987138..8b2cffd 100644 > > --- a/net/netfilter/ipvs/Kconfig > > +++ b/net/netfilter/ipvs/Kconfig > > @@ -250,7 +250,8 @@ comment 'IPVS application helper' > > > > config IP_VS_FTP > > tristate "FTP protocol helper" > > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT > > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ > > + NF_CONNTRACK_FTP > > If you require FTP NAT support, then this depends on NF_NAT_FTP > instead of NF_CONNTRACK_FTP. No, I just checked again, it works without nf_nat_ftp, only nf_nat, nf_conntrack_ftp and iptable_nat are needed. We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet). Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-12 19:43 ` Julian Anastasov @ 2012-07-23 6:48 ` Simon Horman 2012-07-23 17:39 ` Pablo Neira Ayuso 0 siblings, 1 reply; 12+ messages in thread From: Simon Horman @ 2012-07-23 6:48 UTC (permalink / raw) To: Julian Anastasov Cc: Pablo Neira Ayuso, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer On Thu, Jul 12, 2012 at 10:43:22PM +0300, Julian Anastasov wrote: > > Hello, > > On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote: > > > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote: > > > From: Julian Anastasov <ja@ssi.bg> > > > > > > The FTP application indirectly depends on the > > > nf_conntrack_ftp helper for proper NAT support. If the > > > module is not loaded, IPVS can resize the packets for the > > > command connection, eg. PASV response but the SEQ adjustment > > > logic in ipv4_confirm is not called without helper. > > > > > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > > > Signed-off-by: Simon Horman <horms@verge.net.au> > > > --- > > > net/netfilter/ipvs/Kconfig | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig > > > index f987138..8b2cffd 100644 > > > --- a/net/netfilter/ipvs/Kconfig > > > +++ b/net/netfilter/ipvs/Kconfig > > > @@ -250,7 +250,8 @@ comment 'IPVS application helper' > > > > > > config IP_VS_FTP > > > tristate "FTP protocol helper" > > > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT > > > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ > > > + NF_CONNTRACK_FTP > > > > If you require FTP NAT support, then this depends on NF_NAT_FTP > > instead of NF_CONNTRACK_FTP. > > No, I just checked again, it works without nf_nat_ftp, > only nf_nat, nf_conntrack_ftp and iptable_nat are needed. > We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet). Is there a consensus on this? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-23 6:48 ` Simon Horman @ 2012-07-23 17:39 ` Pablo Neira Ayuso 2012-07-23 23:11 ` Simon Horman 0 siblings, 1 reply; 12+ messages in thread From: Pablo Neira Ayuso @ 2012-07-23 17:39 UTC (permalink / raw) To: Simon Horman Cc: Julian Anastasov, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer On Mon, Jul 23, 2012 at 03:48:18PM +0900, Simon Horman wrote: > On Thu, Jul 12, 2012 at 10:43:22PM +0300, Julian Anastasov wrote: > > > > Hello, > > > > On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote: > > > > > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote: > > > > From: Julian Anastasov <ja@ssi.bg> > > > > > > > > The FTP application indirectly depends on the > > > > nf_conntrack_ftp helper for proper NAT support. If the > > > > module is not loaded, IPVS can resize the packets for the > > > > command connection, eg. PASV response but the SEQ adjustment > > > > logic in ipv4_confirm is not called without helper. > > > > > > > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > > > > Signed-off-by: Simon Horman <horms@verge.net.au> > > > > --- > > > > net/netfilter/ipvs/Kconfig | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig > > > > index f987138..8b2cffd 100644 > > > > --- a/net/netfilter/ipvs/Kconfig > > > > +++ b/net/netfilter/ipvs/Kconfig > > > > @@ -250,7 +250,8 @@ comment 'IPVS application helper' > > > > > > > > config IP_VS_FTP > > > > tristate "FTP protocol helper" > > > > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT > > > > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ > > > > + NF_CONNTRACK_FTP > > > > > > If you require FTP NAT support, then this depends on NF_NAT_FTP > > > instead of NF_CONNTRACK_FTP. > > > > No, I just checked again, it works without nf_nat_ftp, > > only nf_nat, nf_conntrack_ftp and iptable_nat are needed. > > We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet). > > Is there a consensus on this? Fine with me, just wanted to make sure this what you wanted. Thanks Simon. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper 2012-07-23 17:39 ` Pablo Neira Ayuso @ 2012-07-23 23:11 ` Simon Horman 0 siblings, 0 replies; 12+ messages in thread From: Simon Horman @ 2012-07-23 23:11 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Julian Anastasov, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer On Mon, Jul 23, 2012 at 07:39:06PM +0200, Pablo Neira Ayuso wrote: > On Mon, Jul 23, 2012 at 03:48:18PM +0900, Simon Horman wrote: > > On Thu, Jul 12, 2012 at 10:43:22PM +0300, Julian Anastasov wrote: > > > > > > Hello, > > > > > > On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote: > > > > > > > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote: > > > > > From: Julian Anastasov <ja@ssi.bg> > > > > > > > > > > The FTP application indirectly depends on the > > > > > nf_conntrack_ftp helper for proper NAT support. If the > > > > > module is not loaded, IPVS can resize the packets for the > > > > > command connection, eg. PASV response but the SEQ adjustment > > > > > logic in ipv4_confirm is not called without helper. > > > > > > > > > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > > > > > Signed-off-by: Simon Horman <horms@verge.net.au> > > > > > --- > > > > > net/netfilter/ipvs/Kconfig | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig > > > > > index f987138..8b2cffd 100644 > > > > > --- a/net/netfilter/ipvs/Kconfig > > > > > +++ b/net/netfilter/ipvs/Kconfig > > > > > @@ -250,7 +250,8 @@ comment 'IPVS application helper' > > > > > > > > > > config IP_VS_FTP > > > > > tristate "FTP protocol helper" > > > > > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT > > > > > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \ > > > > > + NF_CONNTRACK_FTP > > > > > > > > If you require FTP NAT support, then this depends on NF_NAT_FTP > > > > instead of NF_CONNTRACK_FTP. > > > > > > No, I just checked again, it works without nf_nat_ftp, > > > only nf_nat, nf_conntrack_ftp and iptable_nat are needed. > > > We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet). > > > > Is there a consensus on this? > > Fine with me, just wanted to make sure this what you wanted. Thanks > Simon. Thanks. I'll include this in a pull request after rebasing ipvs-next. I plan to do that today. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] ipvs: generalize app registration in netns 2012-07-11 0:25 [GIT PULL nf-next] IPVS Simon Horman 2012-07-11 0:25 ` [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper Simon Horman @ 2012-07-11 0:25 ` Simon Horman 2012-07-12 16:22 ` Pablo Neira Ayuso 1 sibling, 1 reply; 12+ messages in thread From: Simon Horman @ 2012-07-11 0:25 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang, Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer, Simon Horman From: Julian Anastasov <ja@ssi.bg> Get rid of the ftp_app pointer and allow applications to be registered without adding fields in the netns_ipvs structure. Signed-off-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Simon Horman <horms@verge.net.au> --- include/net/ip_vs.h | 5 ++-- net/netfilter/ipvs/ip_vs_app.c | 61 +++++++++++++++++++++++++++++++----------- net/netfilter/ipvs/ip_vs_ftp.c | 21 ++++----------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index d6146b4..6cb4699 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -808,8 +808,6 @@ struct netns_ipvs { struct list_head rs_table[IP_VS_RTAB_SIZE]; /* ip_vs_app */ struct list_head app_list; - /* ip_vs_ftp */ - struct ip_vs_app *ftp_app; /* ip_vs_proto */ #define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */ struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE]; @@ -1179,7 +1177,8 @@ extern void ip_vs_service_net_cleanup(struct net *net); * (from ip_vs_app.c) */ #define IP_VS_APP_MAX_PORTS 8 -extern int register_ip_vs_app(struct net *net, struct ip_vs_app *app); +extern struct ip_vs_app *register_ip_vs_app(struct net *net, + struct ip_vs_app *app); extern void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app); extern int ip_vs_bind_app(struct ip_vs_conn *cp, struct ip_vs_protocol *pp); extern void ip_vs_unbind_app(struct ip_vs_conn *cp); diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c index 64f9e8f..11caaea 100644 --- a/net/netfilter/ipvs/ip_vs_app.c +++ b/net/netfilter/ipvs/ip_vs_app.c @@ -180,22 +180,41 @@ register_ip_vs_app_inc(struct net *net, struct ip_vs_app *app, __u16 proto, } -/* - * ip_vs_app registration routine - */ -int register_ip_vs_app(struct net *net, struct ip_vs_app *app) +/* Register application for netns */ +struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app) { struct netns_ipvs *ipvs = net_ipvs(net); - /* increase the module use count */ - ip_vs_use_count_inc(); + struct ip_vs_app *a; + int err = 0; + + if (!ipvs) + return ERR_PTR(-ENOENT); mutex_lock(&__ip_vs_app_mutex); - list_add(&app->a_list, &ipvs->app_list); + list_for_each_entry(a, &ipvs->app_list, a_list) { + if (!strcmp(app->name, a->name)) { + err = -EEXIST; + break; + } + } + if (!err) { + a = kmemdup(app, sizeof(*app), GFP_KERNEL); + if (!a) + err = -ENOMEM; + } + if (!err) { + INIT_LIST_HEAD(&a->incs_list); + list_add(&a->a_list, &ipvs->app_list); + /* increase the module use count */ + ip_vs_use_count_inc(); + } mutex_unlock(&__ip_vs_app_mutex); - return 0; + if (err) + return ERR_PTR(err); + return a; } @@ -205,20 +224,29 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app) */ void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app) { - struct ip_vs_app *inc, *nxt; + struct netns_ipvs *ipvs = net_ipvs(net); + struct ip_vs_app *a, *anxt, *inc, *nxt; + + if (!ipvs) + return; mutex_lock(&__ip_vs_app_mutex); - list_for_each_entry_safe(inc, nxt, &app->incs_list, a_list) { - ip_vs_app_inc_release(net, inc); - } + list_for_each_entry_safe(a, anxt, &ipvs->app_list, a_list) { + if (app && strcmp(app->name, a->name)) + continue; + list_for_each_entry_safe(inc, nxt, &a->incs_list, a_list) { + ip_vs_app_inc_release(net, inc); + } - list_del(&app->a_list); + list_del(&a->a_list); + kfree(a); - mutex_unlock(&__ip_vs_app_mutex); + /* decrease the module use count */ + ip_vs_use_count_dec(); + } - /* decrease the module use count */ - ip_vs_use_count_dec(); + mutex_unlock(&__ip_vs_app_mutex); } @@ -586,5 +614,6 @@ int __net_init ip_vs_app_net_init(struct net *net) void __net_exit ip_vs_app_net_cleanup(struct net *net) { + unregister_ip_vs_app(net, NULL /* all */); proc_net_remove(net, "ip_vs_app"); } diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index b20b29c..ad70b7e 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -441,16 +441,10 @@ static int __net_init __ip_vs_ftp_init(struct net *net) if (!ipvs) return -ENOENT; - app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL); - if (!app) - return -ENOMEM; - INIT_LIST_HEAD(&app->a_list); - INIT_LIST_HEAD(&app->incs_list); - ipvs->ftp_app = app; - ret = register_ip_vs_app(net, app); - if (ret) - goto err_exit; + app = register_ip_vs_app(net, &ip_vs_ftp); + if (IS_ERR(app)) + return PTR_ERR(app); for (i = 0; i < ports_count; i++) { if (!ports[i]) @@ -464,9 +458,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net) return 0; err_unreg: - unregister_ip_vs_app(net, app); -err_exit: - kfree(ipvs->ftp_app); + unregister_ip_vs_app(net, &ip_vs_ftp); return ret; } /* @@ -474,10 +466,7 @@ err_exit: */ static void __ip_vs_ftp_exit(struct net *net) { - struct netns_ipvs *ipvs = net_ipvs(net); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ipvs: generalize app registration in netns 2012-07-11 0:25 ` [PATCH 2/2] ipvs: generalize app registration in netns Simon Horman @ 2012-07-12 16:22 ` Pablo Neira Ayuso 2012-07-12 20:04 ` Julian Anastasov 2012-07-12 20:06 ` [PATCH v2] " Julian Anastasov 0 siblings, 2 replies; 12+ messages in thread From: Pablo Neira Ayuso @ 2012-07-12 16:22 UTC (permalink / raw) To: Simon Horman Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang, Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer On Wed, Jul 11, 2012 at 09:25:27AM +0900, Simon Horman wrote: > From: Julian Anastasov <ja@ssi.bg> > > Get rid of the ftp_app pointer and allow applications > to be registered without adding fields in the netns_ipvs structure. > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > Signed-off-by: Simon Horman <horms@verge.net.au> > --- > include/net/ip_vs.h | 5 ++-- > net/netfilter/ipvs/ip_vs_app.c | 61 +++++++++++++++++++++++++++++++----------- > net/netfilter/ipvs/ip_vs_ftp.c | 21 ++++----------- > 3 files changed, 52 insertions(+), 35 deletions(-) > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index d6146b4..6cb4699 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -808,8 +808,6 @@ struct netns_ipvs { > struct list_head rs_table[IP_VS_RTAB_SIZE]; > /* ip_vs_app */ > struct list_head app_list; > - /* ip_vs_ftp */ > - struct ip_vs_app *ftp_app; > /* ip_vs_proto */ > #define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */ > struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE]; > @@ -1179,7 +1177,8 @@ extern void ip_vs_service_net_cleanup(struct net *net); > * (from ip_vs_app.c) > */ > #define IP_VS_APP_MAX_PORTS 8 > -extern int register_ip_vs_app(struct net *net, struct ip_vs_app *app); > +extern struct ip_vs_app *register_ip_vs_app(struct net *net, > + struct ip_vs_app *app); > extern void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app); > extern int ip_vs_bind_app(struct ip_vs_conn *cp, struct ip_vs_protocol *pp); > extern void ip_vs_unbind_app(struct ip_vs_conn *cp); > diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c > index 64f9e8f..11caaea 100644 > --- a/net/netfilter/ipvs/ip_vs_app.c > +++ b/net/netfilter/ipvs/ip_vs_app.c > @@ -180,22 +180,41 @@ register_ip_vs_app_inc(struct net *net, struct ip_vs_app *app, __u16 proto, > } > > > -/* > - * ip_vs_app registration routine > - */ > -int register_ip_vs_app(struct net *net, struct ip_vs_app *app) > +/* Register application for netns */ > +struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app) > { > struct netns_ipvs *ipvs = net_ipvs(net); > - /* increase the module use count */ > - ip_vs_use_count_inc(); > + struct ip_vs_app *a; > + int err = 0; > + > + if (!ipvs) > + return ERR_PTR(-ENOENT); > > mutex_lock(&__ip_vs_app_mutex); > > - list_add(&app->a_list, &ipvs->app_list); > + list_for_each_entry(a, &ipvs->app_list, a_list) { > + if (!strcmp(app->name, a->name)) { > + err = -EEXIST; > + break; > + } > + } > + if (!err) { > + a = kmemdup(app, sizeof(*app), GFP_KERNEL); > + if (!a) > + err = -ENOMEM; > + } > + if (!err) { > + INIT_LIST_HEAD(&a->incs_list); > + list_add(&a->a_list, &ipvs->app_list); > + /* increase the module use count */ > + ip_vs_use_count_inc(); > + } I think this code will look better if you use something like: + if (!strcmp(app->name, a->name)) { + err = -EEXIST; + goto err_unlock; + } err_unlock: mutex_unlock(...) > > mutex_unlock(&__ip_vs_app_mutex); > > - return 0; > + if (err) > + return ERR_PTR(err); > + return a; For this three lines above, you can use: return err ? return ERR_PTR(err) : a; > } > > > @@ -205,20 +224,29 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app) > */ > void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app) > { > - struct ip_vs_app *inc, *nxt; > + struct netns_ipvs *ipvs = net_ipvs(net); > + struct ip_vs_app *a, *anxt, *inc, *nxt; > + > + if (!ipvs) > + return; > > mutex_lock(&__ip_vs_app_mutex); > > - list_for_each_entry_safe(inc, nxt, &app->incs_list, a_list) { > - ip_vs_app_inc_release(net, inc); > - } > + list_for_each_entry_safe(a, anxt, &ipvs->app_list, a_list) { > + if (app && strcmp(app->name, a->name)) > + continue; > + list_for_each_entry_safe(inc, nxt, &a->incs_list, a_list) { > + ip_vs_app_inc_release(net, inc); > + } > > - list_del(&app->a_list); > + list_del(&a->a_list); > + kfree(a); > > - mutex_unlock(&__ip_vs_app_mutex); > + /* decrease the module use count */ > + ip_vs_use_count_dec(); > + } > > - /* decrease the module use count */ > - ip_vs_use_count_dec(); > + mutex_unlock(&__ip_vs_app_mutex); > } > > > @@ -586,5 +614,6 @@ int __net_init ip_vs_app_net_init(struct net *net) > > void __net_exit ip_vs_app_net_cleanup(struct net *net) > { > + unregister_ip_vs_app(net, NULL /* all */); > proc_net_remove(net, "ip_vs_app"); > } > diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c > index b20b29c..ad70b7e 100644 > --- a/net/netfilter/ipvs/ip_vs_ftp.c > +++ b/net/netfilter/ipvs/ip_vs_ftp.c > @@ -441,16 +441,10 @@ static int __net_init __ip_vs_ftp_init(struct net *net) > > if (!ipvs) > return -ENOENT; > - app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL); > - if (!app) > - return -ENOMEM; > - INIT_LIST_HEAD(&app->a_list); > - INIT_LIST_HEAD(&app->incs_list); > - ipvs->ftp_app = app; > > - ret = register_ip_vs_app(net, app); > - if (ret) > - goto err_exit; > + app = register_ip_vs_app(net, &ip_vs_ftp); > + if (IS_ERR(app)) > + return PTR_ERR(app); > > for (i = 0; i < ports_count; i++) { > if (!ports[i]) > @@ -464,9 +458,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net) > return 0; > > err_unreg: > - unregister_ip_vs_app(net, app); > -err_exit: > - kfree(ipvs->ftp_app); > + unregister_ip_vs_app(net, &ip_vs_ftp); > return ret; > } > /* > @@ -474,10 +466,7 @@ err_exit: > */ > static void __ip_vs_ftp_exit(struct net *net) > { > - struct netns_ipvs *ipvs = net_ipvs(net); > - > - unregister_ip_vs_app(net, ipvs->ftp_app); > - kfree(ipvs->ftp_app); > + unregister_ip_vs_app(net, &ip_vs_ftp); > } > > static struct pernet_operations ip_vs_ftp_ops = { > -- > 1.7.10.2.484.gcd07cc5 > > -- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ipvs: generalize app registration in netns 2012-07-12 16:22 ` Pablo Neira Ayuso @ 2012-07-12 20:04 ` Julian Anastasov 2012-07-12 20:06 ` [PATCH v2] " Julian Anastasov 1 sibling, 0 replies; 12+ messages in thread From: Julian Anastasov @ 2012-07-12 20:04 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer Hello, On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote: > > +struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app) > > { > > struct netns_ipvs *ipvs = net_ipvs(net); > > - /* increase the module use count */ > > - ip_vs_use_count_inc(); > > + struct ip_vs_app *a; > > + int err = 0; > > + > > + if (!ipvs) > > + return ERR_PTR(-ENOENT); > > > > mutex_lock(&__ip_vs_app_mutex); > > > > - list_add(&app->a_list, &ipvs->app_list); > > + list_for_each_entry(a, &ipvs->app_list, a_list) { > > + if (!strcmp(app->name, a->name)) { > > + err = -EEXIST; > > + break; > > + } > > + } > > + if (!err) { > > + a = kmemdup(app, sizeof(*app), GFP_KERNEL); > > + if (!a) > > + err = -ENOMEM; > > + } > > + if (!err) { > > + INIT_LIST_HEAD(&a->incs_list); > > + list_add(&a->a_list, &ipvs->app_list); > > + /* increase the module use count */ > > + ip_vs_use_count_inc(); > > + } > > I think this code will look better if you use something like: > > + if (!strcmp(app->name, a->name)) { > + err = -EEXIST; > + goto err_unlock; > + } > > err_unlock: > mutex_unlock(...) > > > > > mutex_unlock(&__ip_vs_app_mutex); > > > > - return 0; > > + if (err) > > + return ERR_PTR(err); > > + return a; > > For this three lines above, you can use: > > return err ? return ERR_PTR(err) : a; Good point, sending v2 ... Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] ipvs: generalize app registration in netns 2012-07-12 16:22 ` Pablo Neira Ayuso 2012-07-12 20:04 ` Julian Anastasov @ 2012-07-12 20:06 ` Julian Anastasov 2012-07-13 2:59 ` Simon Horman 1 sibling, 1 reply; 12+ messages in thread From: Julian Anastasov @ 2012-07-12 20:06 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer Get rid of the ftp_app pointer and allow applications to be registered without adding fields in the netns_ipvs structure. v2: fix coding style as suggested by Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Julian Anastasov <ja@ssi.bg> --- include/net/ip_vs.h | 5 +-- net/netfilter/ipvs/ip_vs_app.c | 58 +++++++++++++++++++++++++++++----------- net/netfilter/ipvs/ip_vs_ftp.c | 21 +++----------- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index d6146b4..6cb4699 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -808,8 +808,6 @@ struct netns_ipvs { struct list_head rs_table[IP_VS_RTAB_SIZE]; /* ip_vs_app */ struct list_head app_list; - /* ip_vs_ftp */ - struct ip_vs_app *ftp_app; /* ip_vs_proto */ #define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */ struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE]; @@ -1179,7 +1177,8 @@ extern void ip_vs_service_net_cleanup(struct net *net); * (from ip_vs_app.c) */ #define IP_VS_APP_MAX_PORTS 8 -extern int register_ip_vs_app(struct net *net, struct ip_vs_app *app); +extern struct ip_vs_app *register_ip_vs_app(struct net *net, + struct ip_vs_app *app); extern void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app); extern int ip_vs_bind_app(struct ip_vs_conn *cp, struct ip_vs_protocol *pp); extern void ip_vs_unbind_app(struct ip_vs_conn *cp); diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c index 64f9e8f..9713e6e 100644 --- a/net/netfilter/ipvs/ip_vs_app.c +++ b/net/netfilter/ipvs/ip_vs_app.c @@ -180,22 +180,38 @@ register_ip_vs_app_inc(struct net *net, struct ip_vs_app *app, __u16 proto, } -/* - * ip_vs_app registration routine - */ -int register_ip_vs_app(struct net *net, struct ip_vs_app *app) +/* Register application for netns */ +struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app) { struct netns_ipvs *ipvs = net_ipvs(net); - /* increase the module use count */ - ip_vs_use_count_inc(); + struct ip_vs_app *a; + int err = 0; + + if (!ipvs) + return ERR_PTR(-ENOENT); mutex_lock(&__ip_vs_app_mutex); - list_add(&app->a_list, &ipvs->app_list); + list_for_each_entry(a, &ipvs->app_list, a_list) { + if (!strcmp(app->name, a->name)) { + err = -EEXIST; + goto out_unlock; + } + } + a = kmemdup(app, sizeof(*app), GFP_KERNEL); + if (!a) { + err = -ENOMEM; + goto out_unlock; + } + INIT_LIST_HEAD(&a->incs_list); + list_add(&a->a_list, &ipvs->app_list); + /* increase the module use count */ + ip_vs_use_count_inc(); +out_unlock: mutex_unlock(&__ip_vs_app_mutex); - return 0; + return err ? ERR_PTR(err) : a; } @@ -205,20 +221,29 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app) */ void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app) { - struct ip_vs_app *inc, *nxt; + struct netns_ipvs *ipvs = net_ipvs(net); + struct ip_vs_app *a, *anxt, *inc, *nxt; + + if (!ipvs) + return; mutex_lock(&__ip_vs_app_mutex); - list_for_each_entry_safe(inc, nxt, &app->incs_list, a_list) { - ip_vs_app_inc_release(net, inc); - } + list_for_each_entry_safe(a, anxt, &ipvs->app_list, a_list) { + if (app && strcmp(app->name, a->name)) + continue; + list_for_each_entry_safe(inc, nxt, &a->incs_list, a_list) { + ip_vs_app_inc_release(net, inc); + } - list_del(&app->a_list); + list_del(&a->a_list); + kfree(a); - mutex_unlock(&__ip_vs_app_mutex); + /* decrease the module use count */ + ip_vs_use_count_dec(); + } - /* decrease the module use count */ - ip_vs_use_count_dec(); + mutex_unlock(&__ip_vs_app_mutex); } @@ -586,5 +611,6 @@ int __net_init ip_vs_app_net_init(struct net *net) void __net_exit ip_vs_app_net_cleanup(struct net *net) { + unregister_ip_vs_app(net, NULL /* all */); proc_net_remove(net, "ip_vs_app"); } diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index b20b29c..ad70b7e 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -441,16 +441,10 @@ static int __net_init __ip_vs_ftp_init(struct net *net) if (!ipvs) return -ENOENT; - app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL); - if (!app) - return -ENOMEM; - INIT_LIST_HEAD(&app->a_list); - INIT_LIST_HEAD(&app->incs_list); - ipvs->ftp_app = app; - ret = register_ip_vs_app(net, app); - if (ret) - goto err_exit; + app = register_ip_vs_app(net, &ip_vs_ftp); + if (IS_ERR(app)) + return PTR_ERR(app); for (i = 0; i < ports_count; i++) { if (!ports[i]) @@ -464,9 +458,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net) return 0; err_unreg: - unregister_ip_vs_app(net, app); -err_exit: - kfree(ipvs->ftp_app); + unregister_ip_vs_app(net, &ip_vs_ftp); return ret; } /* @@ -474,10 +466,7 @@ err_exit: */ static void __ip_vs_ftp_exit(struct net *net) { - struct netns_ipvs *ipvs = net_ipvs(net); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ipvs: generalize app registration in netns 2012-07-12 20:06 ` [PATCH v2] " Julian Anastasov @ 2012-07-13 2:59 ` Simon Horman 0 siblings, 0 replies; 12+ messages in thread From: Simon Horman @ 2012-07-13 2:59 UTC (permalink / raw) To: Julian Anastasov Cc: Pablo Neira Ayuso, lvs-devel, netdev, netfilter-devel, Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer On Thu, Jul 12, 2012 at 11:06:20PM +0300, Julian Anastasov wrote: > > Get rid of the ftp_app pointer and allow applications > to be registered without adding fields in the netns_ipvs structure. > > v2: fix coding style as suggested by Pablo Neira Ayuso <pablo@netfilter.org> Thanks, I have (forcibly) pushed this into the ipvs-next tree in place of v1. I will want for consensus on the "ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper" change before sending a fresh pull request to Pablo. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-07-23 23:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-11 0:25 [GIT PULL nf-next] IPVS Simon Horman 2012-07-11 0:25 ` [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper Simon Horman 2012-07-12 15:39 ` Pablo Neira Ayuso 2012-07-12 19:43 ` Julian Anastasov 2012-07-23 6:48 ` Simon Horman 2012-07-23 17:39 ` Pablo Neira Ayuso 2012-07-23 23:11 ` Simon Horman 2012-07-11 0:25 ` [PATCH 2/2] ipvs: generalize app registration in netns Simon Horman 2012-07-12 16:22 ` Pablo Neira Ayuso 2012-07-12 20:04 ` Julian Anastasov 2012-07-12 20:06 ` [PATCH v2] " Julian Anastasov 2012-07-13 2:59 ` Simon Horman
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.