Re: [IPV4] LVS: Allow to send ICMP unreachable responses when real-servers are removed

[Balazs Scheidler <bazsi@balabit.hu>]

Hi,

Is there a chance that this, or a patch with similar spirit (e.g. a way
to send packets from non-local IP addresses) could be merged?

On Fri, 2007-06-01 at 02:18 +0300, Julian Anastasov wrote:
> 	Hello,
> 
> On Thu, 31 May 2007, KOVACS Krisztian wrote:
> 
> >   So what about this one?
> 
> 	May be we can try with better coding style. Also, this version
> adds undefined behavior for using FLOWI_FLAG_ANYSRC with multicast
> oldflp->fl4_dst
> 
> > Loosen source address check on IPv4 output
> > 
> > From: KOVACS Krisztian <hidden@balabit.hu>
> > 
> > ip_route_output() contains a check to make sure that no flows with
> > non-local source IP addresses are routed. This obviously makes using
> > such addresses impossible.
> > 
> > This patch introduces a flowi flag which makes omitting this check
> > possible. The new flag provides a way of handling transparent and
> > non-transparent connections differently.
> > 
> > Signed-off-by: KOVACS Krisztian <hidden@balabit.hu>
> > ---
> > 
> >  include/net/flow.h |    1 +
> >  net/ipv4/route.c   |   47 +++++++++++++++++++++++++----------------------
> >  2 files changed, 26 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/net/flow.h b/include/net/flow.h
> > index f3cc1f8..1bfc0dc 100644
> > --- a/include/net/flow.h
> > +++ b/include/net/flow.h
> > @@ -49,6 +49,7 @@ struct flowi {
> >  	__u8	proto;
> >  	__u8	flags;
> >  #define FLOWI_FLAG_MULTIPATHOLDROUTE 0x01
> > +#define FLOWI_FLAG_ANYSRC 0x02
> >  	union {
> >  		struct {
> >  			__be16	sport;
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 8603cfb..88d0a79 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -2396,7 +2396,7 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
> >  
> >  		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
> >  		dev_out = ip_dev_find(oldflp->fl4_src);
> > -		if (dev_out == NULL)
> > +		if (dev_out == NULL && !(oldflp->flags & FLOWI_FLAG_ANYSRC))
> >  			goto out;
> >  
> >  		/* I removed check for oif == dev_out->oif here.
> > @@ -2407,29 +2407,32 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
> >  		      of another iface. --ANK
> >  		 */
> >  
> > -		if (oldflp->oif == 0
> > -		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> > -			/* Special hack: user can direct multicasts
> > -			   and limited broadcast via necessary interface
> > -			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> > -			   This hack is not just for fun, it allows
> > -			   vic,vat and friends to work.
> > -			   They bind socket to loopback, set ttl to zero
> > -			   and expect that it will work.
> > -			   From the viewpoint of routing cache they are broken,
> > -			   because we are not allowed to build multicast path
> > -			   with loopback source addr (look, routing cache
> > -			   cannot know, that ttl is zero, so that packet
> > -			   will not leave this host and route is valid).
> > -			   Luckily, this hack is good workaround.
> > -			 */
> > +		if (dev_out) {
> > +			if (oldflp->oif == 0
> > +			    && (MULTICAST(oldflp->fl4_dst)
> > +				|| oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> > +				/* Special hack: user can direct multicasts
> > +				   and limited broadcast via necessary interface
> > +				   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> > +				   This hack is not just for fun, it allows
> > +				   vic,vat and friends to work.
> > +				   They bind socket to loopback, set ttl to zero
> > +				   and expect that it will work.
> > +				   From the viewpoint of routing cache they are broken,
> > +				   because we are not allowed to build multicast path
> > +				   with loopback source addr (look, routing cache
> > +				   cannot know, that ttl is zero, so that packet
> > +				   will not leave this host and route is valid).
> > +				   Luckily, this hack is good workaround.
> > +				*/
> > +
> > +				fl.oif = dev_out->ifindex;
> > +				goto make_route;
> > +			}
> >  
> > -			fl.oif = dev_out->ifindex;
> > -			goto make_route;
> > -		}
> > -		if (dev_out)
> >  			dev_put(dev_out);
> > -		dev_out = NULL;
> > +			dev_out = NULL;
> > +		}
> >  	}
> 
> 	What about something like this, it even reduces checks
> in the fast path. You can post new version if the following change
> looks good to you and to other developers. If additional sign line is 
> needed here it is:
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> 
> @@ -2396,8 +2396,6 @@ static int ip_route_output_slow(struct r
>  
>  		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
>  		dev_out = ip_dev_find(oldflp->fl4_src);
> -		if (dev_out == NULL)
> -			goto out;
>  
>  		/* I removed check for oif == dev_out->oif here.
>  		   It was wrong for two reasons:
> @@ -2409,6 +2407,8 @@ static int ip_route_output_slow(struct r
>  
>  		if (oldflp->oif == 0
>  		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> +			if (dev_out == NULL)
> +				goto out;
>  			/* Special hack: user can direct multicasts
>  			   and limited broadcast via necessary interface
>  			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> @@ -2427,9 +2427,11 @@ static int ip_route_output_slow(struct r
>  			fl.oif = dev_out->ifindex;
>  			goto make_route;
>  		}
> -		if (dev_out)
> +		if (dev_out) {
>  			dev_put(dev_out);
> -		dev_out = NULL;
> +			dev_out = NULL;
> +		} else if (!(oldflp->flags & FLOWI_FLAG_ANYSRC))
> +			goto out;
>  	}
>  
> 
> 
> 	Or we can go further and to avoid ip_dev_find? For me, this
> second variant is preferred because calling ip_dev_find() is useless for
> FLOWI_FLAG_ANYSRC.
> 
> @@ -2394,11 +2394,6 @@ static int ip_route_output_slow(struct r
>  		    ZERONET(oldflp->fl4_src))
>  			goto out;
>  
> -		/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
> -		dev_out = ip_dev_find(oldflp->fl4_src);
> -		if (dev_out == NULL)
> -			goto out;
> -
>  		/* I removed check for oif == dev_out->oif here.
>  		   It was wrong for two reasons:
>  		   1. ip_dev_find(saddr) can return wrong iface, if saddr is
> @@ -2409,6 +2404,11 @@ static int ip_route_output_slow(struct r
>  
>  		if (oldflp->oif == 0
>  		    && (MULTICAST(oldflp->fl4_dst) || oldflp->fl4_dst == htonl(0xFFFFFFFF))) {
> +			/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
> +			dev_out = ip_dev_find(oldflp->fl4_src);
> +			if (dev_out == NULL)
> +				goto out;
> +
>  			/* Special hack: user can direct multicasts
>  			   and limited broadcast via necessary interface
>  			   without fiddling with IP_MULTICAST_IF or IP_PKTINFO.
> @@ -2427,9 +2427,14 @@ static int ip_route_output_slow(struct r
>  			fl.oif = dev_out->ifindex;
>  			goto make_route;
>  		}
> -		if (dev_out)
> +		if (!(oldflp->flags & FLOWI_FLAG_ANYSRC)) {
> +			/* It is equivalent to inet_addr_type(saddr) == RTN_LOCAL */
> +			dev_out = ip_dev_find(oldflp->fl4_src);
> +			if (dev_out == NULL)
> +				goto out;
>  			dev_put(dev_out);
> -		dev_out = NULL;
> +			dev_out = NULL;
> +		}
>  	}
>  
> 
> -
> 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
> 
-- 
Bazsi