Re: [PATCH net-next-2.6] ipv6: adding ip_nonlocal_bind option from ipv4

[Michal Humpula <michal.humpula@web4u.cz>]

On Monday 05 of July 2010 04:03:36 Simon Horman wrote:
> On Sat, Jul 03, 2010 at 10:38:28PM +0200, Michal Humpula wrote:
> > Adds ability to bind non-local IPv6 address the same way as for IPv4
> > 
> > Signed-off-by: Michal Humpula <michal.humpula@web4u.cz>
> > 
> > diff --git a/Documentation/networking/ip-sysctl.txt
> > b/Documentation/networking/ip-sysctl.txt index f350c69..27fa09a 100644
> > --- a/Documentation/networking/ip-sysctl.txt
> > +++ b/Documentation/networking/ip-sysctl.txt
> > @@ -962,6 +962,10 @@ bindv6only - BOOLEAN
> > 
> >  		FALSE: enable IPv4-mapped address feature
> >  	
> >  	Default: FALSE (as specified in RFC2553bis)
> 
> I think a blank line here would be nice.
> 
> > +ipv6_nonlocal_bind - BOOLEAN
> > +	If set, allows processes to bind() to non-local IPv6 addresses,
> > +	which can be quite useful - but may break some applications.
> > +	Default: 0
> > 
> >  IPv6 Fragmentation:
> > 
> > diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> > index e830cd4..55b3552 100644
> > --- a/net/ipv6/af_inet6.c
> > +++ b/net/ipv6/af_inet6.c
> > 
> > @@ -345,8 +347,10 @@ int inet6_bind(struct socket *sock, struct sockaddr
> > *uaddr, int addr_len)
> > 
> >  			if (!(addr_type & IPV6_ADDR_MULTICAST))	{
> >  			
> >  				if (!ipv6_chk_addr(net, &addr->sin6_addr,
> >  				
> >  						   dev, 0)) {
> > 
> > -					err = -EADDRNOTAVAIL;
> > -					goto out_unlock;
> > +					if (!sysctl_ipv6_nonlocal_bind) {
> > +						err = -EADDRNOTAVAIL;
> > +						goto out_unlock;
> > +					}
> > 
> >  				}
> >  			
> >  			}
> >  			rcu_read_unlock();
> 
> Perhaps the inner two if statements could be combined to remove
> unnecessary nesting? And perhaps check for sysctl_ipv6_nonlocal_bind first,
> as presumably its a cheaper, though less likely to succeed check.
> 
> 			if (!(addr_type & IPV6_ADDR_MULTICAST))	{
> 				if (!sysctl_ipv6_nonlocal_bind &&
> 				    !ipv6_chk_addr(net, &addr->sin6_addr,
> 						   dev, 0)) {
> 					err = -EADDRNOTAVAIL;
> 					goto out_unlock;
> 				}
> 			}
> 
> Actually, I wonder if all three if statements could be combined.
> 
> 			if (!(addr_type & IPV6_ADDR_MULTICAST) &&
> 		            !sysctl_ipv6_nonlocal_bind &&
> 			    !ipv6_chk_addr(net, &addr->sin6_addr, dev, 0)) {
> 				err = -EADDRNOTAVAIL;
> 				goto out_unlock;
> 			}
> 

Thanks you for review, I agree with the objections. I'll send a corrected version.