[GIT PULL nf-next] IPVS

[GIT PULL nf-next] IPVS

[Simon Horman]

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(-)

[PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[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

[PATCH 2/2] ipvs: generalize app registration in netns

[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);

Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[Pablo Neira Ayuso]

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.

Re: [PATCH 2/2] ipvs: generalize app registration in netns

[Pablo Neira Ayuso]

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

Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[Julian Anastasov]

	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>

Re: [PATCH 2/2] ipvs: generalize app registration in netns

[Julian Anastasov]

	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>

[PATCH v2] ipvs: generalize app registration in netns

[Julian Anastasov]

	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);

Re: [PATCH v2] ipvs: generalize app registration in netns

[Simon Horman]

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.

Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[Simon Horman]

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?

Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[Pablo Neira Ayuso]

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.

Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper

[Simon Horman]

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.