Date: Fri, 9 Jan 2009 18:02:09 +0100 From: Max Laier <max@love2party.net> To: Adrian Chadd <adrian@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r186955 - in head/sys: conf netinet Message-ID: <200901091802.10287.max@love2party.net> In-Reply-To: <200901091602.n09G2Jj1061164@svn.freebsd.org> References: <200901091602.n09G2Jj1061164@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 09 January 2009 17:02:19 Adrian Chadd wrote: > Author: adrian > Date: Fri Jan 9 16:02:19 2009 > New Revision: 186955 > URL: http://svn.freebsd.org/changeset/base/186955 > > Log: > Implement a new IP option (not compiled/enabled by default) to allow > applications to specify a non-local IP address when bind()'ing a socket > to a local endpoint. That's a *socket* option ... you had me very worried there for a moment ;) I don't quite see why you'd hide these under a build time option - having the sysctl defaulting to off under CTLFLAG_SECURE seems good enough - if people disagree - make it a boot time tuneable, but I certainly don't see why you should have to rebuild the kernel for a minor thing like this. It certainly isn't performance critical. Some nit picking below ... > Modified: head/sys/netinet/in_pcb.c > @@ -346,7 +347,11 @@ in_pcbbind_setup(struct inpcb *inp, stru > } else if (sin->sin_addr.s_addr != INADDR_ANY) { > sin->sin_port = 0; /* yech... */ > bzero(&sin->sin_zero, sizeof(sin->sin_zero)); > - if (ifa_ifwithaddr((struct sockaddr *)sin) == 0) > + if ( > +#if defined(IP_NONLOCALBIND) > + ((inp->inp_flags & INP_NONLOCALOK) == 0) && > +#endif > + (ifa_ifwithaddr((struct sockaddr *)sin) == 0)) > return (EADDRNOTAVAIL); > } > laddr = sin->sin_addr; This logic is really hard to get a first glance. Esp. the not NON...OK part. Maybe a comment is called for here - or is this just me being confused? > Modified: head/sys/netinet/ip_output.c > @@ -866,6 +873,13 @@ ip_ctloutput(struct socket *so, struct s > return (error); > } > > +#if defined(IP_NONLOCALBIND) > + case IP_NONLOCALOK: > + if (! ip_nonlocalok) { > + error = ENOPROTOOPT; > + break; > + } > +#endif Indentation is off here. And you should add a /* FALLTHROUGH */ comment to make it clear that this is intended. > case IP_TOS: > case IP_TTL: > case IP_MINTTL: -- /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901091802.10287.max>