All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch resend nf] ipvs: fix AF assignment in ip_vs_conn_new()
@ 2014-01-29 14:54 Michal Kubecek
  2014-01-29 21:24 ` Julian Anastasov
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Kubecek @ 2014-01-29 14:54 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, lvs-devel, Wensong Zhang, Simon Horman, Julian Anastasov,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller

If a fwmark is passed to ip_vs_conn_new(), it is passed in
vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
vaddr assignment (like we do in ip_vs_ct_in_get()).

Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
(first time it didn't reach all recipients due to a malformed header)
---
 net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 59a1a85..282b39b 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
 	cp->protocol	   = p->protocol;
 	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
 	cp->cport	   = p->cport;
-	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
-	cp->vport	   = p->vport;
-	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
+	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
 	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
-		       &cp->daddr, daddr);
+		       &cp->vaddr, vaddr);
+	cp->vport	   = p->vport;
+	ip_vs_addr_set(p->af, &cp->daddr, p->daddr);
 	cp->dport          = dport;
 	cp->flags	   = flags;
 	cp->fwmark         = fwmark;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [Patch resend nf] ipvs: fix AF assignment in ip_vs_conn_new()
  2014-01-29 14:54 [Patch resend nf] ipvs: fix AF assignment in ip_vs_conn_new() Michal Kubecek
@ 2014-01-29 21:24 ` Julian Anastasov
  2014-01-30  7:39   ` Michal Kubecek
  2014-01-30  7:50   ` [PATCH nf v2] " Michal Kubecek
  0 siblings, 2 replies; 6+ messages in thread
From: Julian Anastasov @ 2014-01-29 21:24 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netfilter-devel, netdev, lvs-devel, Wensong Zhang, Simon Horman,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller


	Hello,

On Wed, 29 Jan 2014, Michal Kubecek wrote:

> If a fwmark is passed to ip_vs_conn_new(), it is passed in
> vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
> vaddr assignment (like we do in ip_vs_ct_in_get()).
> 
> Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
> (first time it didn't reach all recipients due to a malformed header)
> ---
>  net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 59a1a85..282b39b 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
>  	cp->protocol	   = p->protocol;
>  	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
>  	cp->cport	   = p->cport;
> -	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
> -	cp->vport	   = p->vport;
> -	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
> +	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
>  	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
> -		       &cp->daddr, daddr);
> +		       &cp->vaddr, vaddr);

	Patch does not compile due to vaddr and p->daddr
usage but you are in the right direction. Such change should
fix a problem where connection templates don't get full
IPv6 address for the real server, only the first 4 bytes
are copied and as result it works only for IPv4.

> +	cp->vport	   = p->vport;
> +	ip_vs_addr_set(p->af, &cp->daddr, p->daddr);
>  	cp->dport          = dport;
>  	cp->flags	   = flags;
>  	cp->fwmark         = fwmark;
> -- 
> 1.8.1.4

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Patch resend nf] ipvs: fix AF assignment in ip_vs_conn_new()
  2014-01-29 21:24 ` Julian Anastasov
@ 2014-01-30  7:39   ` Michal Kubecek
  2014-01-30  7:50   ` [PATCH nf v2] " Michal Kubecek
  1 sibling, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2014-01-30  7:39 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netfilter-devel, netdev, lvs-devel, Wensong Zhang, Simon Horman,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller

On Wed, Jan 29, 2014 at 11:24:17PM +0200, Julian Anastasov wrote:
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 59a1a85..282b39b 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
> >  	cp->protocol	   = p->protocol;
> >  	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
> >  	cp->cport	   = p->cport;
> > -	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
> > -	cp->vport	   = p->vport;
> > -	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
> > +	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
> >  	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
> > -		       &cp->daddr, daddr);
> > +		       &cp->vaddr, vaddr);
> 
> 	Patch does not compile due to vaddr and p->daddr
> usage but you are in the right direction. Such change should
> fix a problem where connection templates don't get full
> IPv6 address for the real server, only the first 4 bytes
> are copied and as result it works only for IPv4.

Sorry for that, looks like I ran the test build after adapting to
current code with a config which didn't actually compile this file.
I'll send a v2 after testing a fixed version and I'll also extend the
commit message to describe the outcome.

                                                      Michal Kubecek


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH nf v2] ipvs: fix AF assignment in ip_vs_conn_new()
  2014-01-29 21:24 ` Julian Anastasov
  2014-01-30  7:39   ` Michal Kubecek
@ 2014-01-30  7:50   ` Michal Kubecek
  2014-01-30  8:29     ` Julian Anastasov
  1 sibling, 1 reply; 6+ messages in thread
From: Michal Kubecek @ 2014-01-30  7:50 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, lvs-devel, Wensong Zhang, Simon Horman, Julian Anastasov,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller

If a fwmark is passed to ip_vs_conn_new(), it is passed in
vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
vaddr assignment (like we do in ip_vs_ct_in_get()), otherwise we
may copy only first 4 bytes of an IPv6 address into cp->daddr.

v2: fix messed up {d,v}addr and p->{d,v}addr, add info about the
outcome (thanks to Julian Anastasov)

Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 59a1a85..a8eb0a8 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
 	cp->protocol	   = p->protocol;
 	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
 	cp->cport	   = p->cport;
-	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
-	cp->vport	   = p->vport;
-	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
+	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
 	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
-		       &cp->daddr, daddr);
+		       &cp->vaddr, p->vaddr);
+	cp->vport	   = p->vport;
+	ip_vs_addr_set(p->af, &cp->daddr, daddr);
 	cp->dport          = dport;
 	cp->flags	   = flags;
 	cp->fwmark         = fwmark;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH nf v2] ipvs: fix AF assignment in ip_vs_conn_new()
  2014-01-30  7:50   ` [PATCH nf v2] " Michal Kubecek
@ 2014-01-30  8:29     ` Julian Anastasov
  2014-02-04  9:08       ` Simon Horman
  0 siblings, 1 reply; 6+ messages in thread
From: Julian Anastasov @ 2014-01-30  8:29 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netfilter-devel, netdev, lvs-devel, Wensong Zhang, Simon Horman,
	Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
	David S. Miller


	Hello,

On Thu, 30 Jan 2014, Michal Kubecek wrote:

> If a fwmark is passed to ip_vs_conn_new(), it is passed in
> vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
> vaddr assignment (like we do in ip_vs_ct_in_get()), otherwise we
> may copy only first 4 bytes of an IPv6 address into cp->daddr.
> 
> v2: fix messed up {d,v}addr and p->{d,v}addr, add info about the
> outcome (thanks to Julian Anastasov)
> 
> Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Acked-by: Julian Anastasov <ja@ssi.bg>

	Looks like problem comes from
commit be8be9eccbf2d908a7e56b3f7a71105cd88da06b
"ipvs: Fix IPv4 FWMARK virtual services" (2.6.30).

> ---
>  net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 59a1a85..a8eb0a8 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
>  	cp->protocol	   = p->protocol;
>  	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
>  	cp->cport	   = p->cport;
> -	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
> -	cp->vport	   = p->vport;
> -	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
> +	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
>  	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
> -		       &cp->daddr, daddr);
> +		       &cp->vaddr, p->vaddr);
> +	cp->vport	   = p->vport;
> +	ip_vs_addr_set(p->af, &cp->daddr, daddr);
>  	cp->dport          = dport;
>  	cp->flags	   = flags;
>  	cp->fwmark         = fwmark;
> -- 
> 1.8.1.4

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH nf v2] ipvs: fix AF assignment in ip_vs_conn_new()
  2014-01-30  8:29     ` Julian Anastasov
@ 2014-02-04  9:08       ` Simon Horman
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2014-02-04  9:08 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Michal Kubecek, netfilter-devel, netdev, lvs-devel,
	Wensong Zhang, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller

On Thu, Jan 30, 2014 at 10:29:22AM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 30 Jan 2014, Michal Kubecek wrote:
> 
> > If a fwmark is passed to ip_vs_conn_new(), it is passed in
> > vaddr, not daddr. Therefore we should set AF to AF_UNSPEC in
> > vaddr assignment (like we do in ip_vs_ct_in_get()), otherwise we
> > may copy only first 4 bytes of an IPv6 address into cp->daddr.
> > 
> > v2: fix messed up {d,v}addr and p->{d,v}addr, add info about the
> > outcome (thanks to Julian Anastasov)
> > 
> > Signed-off-by: Bogdano Arendartchuk <barendartchuk@suse.com>
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>
> 
> 	Looks like problem comes from
> commit be8be9eccbf2d908a7e56b3f7a71105cd88da06b
> "ipvs: Fix IPv4 FWMARK virtual services" (2.6.30).

Thanks, I'll queue this up.

> 
> > ---
> >  net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 59a1a85..a8eb0a8 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -871,11 +871,11 @@ ip_vs_conn_new(const struct ip_vs_conn_param *p,
> >  	cp->protocol	   = p->protocol;
> >  	ip_vs_addr_set(p->af, &cp->caddr, p->caddr);
> >  	cp->cport	   = p->cport;
> > -	ip_vs_addr_set(p->af, &cp->vaddr, p->vaddr);
> > -	cp->vport	   = p->vport;
> > -	/* proto should only be IPPROTO_IP if d_addr is a fwmark */
> > +	/* proto should only be IPPROTO_IP if p->vaddr is a fwmark */
> >  	ip_vs_addr_set(p->protocol == IPPROTO_IP ? AF_UNSPEC : p->af,
> > -		       &cp->daddr, daddr);
> > +		       &cp->vaddr, p->vaddr);
> > +	cp->vport	   = p->vport;
> > +	ip_vs_addr_set(p->af, &cp->daddr, daddr);
> >  	cp->dport          = dport;
> >  	cp->flags	   = flags;
> >  	cp->fwmark         = fwmark;
> > -- 
> > 1.8.1.4
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-04  9:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-29 14:54 [Patch resend nf] ipvs: fix AF assignment in ip_vs_conn_new() Michal Kubecek
2014-01-29 21:24 ` Julian Anastasov
2014-01-30  7:39   ` Michal Kubecek
2014-01-30  7:50   ` [PATCH nf v2] " Michal Kubecek
2014-01-30  8:29     ` Julian Anastasov
2014-02-04  9:08       ` 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.