Peter Memishian writes: > http://cr.opensolaris.org/~meem/dhcp-sock
You'll need the PSARC case in your putback comments. The code looks good, though I did catch a couple of bugs in it. cmd/cmd-inet/sbin/dhcpagent/Makefile It looks like udp_sum.c is used only by stand/lib/inet now. It could possibly be moved out of $SRC/common. (I don't care too much, but reducing the odd things in $SRC/common seems like a worthwhile fix.) cmd/cmd-inet/sbin/dhcpagent/interface.c 157,170,195: there's a double-close-and-free path here that could lead to a core dump. I think you should set dh back to NULL after line 157. 154-155: not your problem, but please bring this inside the if-statement above; we shouldn't be doing memcpy when pif_hwlen is zero, even if that "does nothing." 1257,1275: I find the use of host-byte-order for addresses to be pretty confusing because it's contrary to all other networking practice, so if this could be limited or removed, that'd be nice. 1315: is this needed? 1370: the design of this doesn't look entirely safe to me. If dhcpagent were to go down on some fatal signal, then it appears that we'd leave the SIOCSLIFDHCPINIT flag set on the LIF in the kernel. I realize that we set IFF_DHCPRUNNING and can leave it set in some cases, but this looks potentially more harmful. Is there any way to make it less harmful, such as perhaps associating the flag with a socket (which will then automatically close on exit)? Or, maybe, I shouldn't worry so much. cmd/cmd-inet/usr.sbin/in.routed/output.c 197: buf could be const. 198: I avoid the name 'sin' for variables, as some versions of gcc treat that as a built-in name (even when <math.h> isn't included), and will freak out if they see it in any other context. Suggest 'sinp' instead. cmd/cmd-inet/usr.sbin/in.routed/rdisc.c 1059,1078: this path looks broken to me. In the previous code, we relied on the fact that IP_XMIT_IF was sticky, so if rdisc_sock_interface was still set to ifp, then no new setsockopt was needed. The new code doesn't work right because it fetches ifindex only when rdisc_sock_interface is set to something other than ifp (in other words, the first time through), so it'll be zero most of the time, and the sendtoif won't do what's expected. To fix this, move the ifindex-fetching code (lines 1057-1058) outside of the if-statement. uts/common/inet/ip/ip.c 20258: can xmit_ill be non-NULL here, and thus lose a reference by this assignment? Line 20274 seems to say this can happen. 20281: unclear what "standard path" means here, as it looks like we're vectoring away from the standard (fall-through) path for these cases. 20303: this comment isn't true; it can't be a local ire here anymore. 21152: shouldn't this be testing conn_outgoing_ill for NULL, so that IP_BOUND_IF takes precedence over SO_DONTROUTE? (Maybe it's not necessary because you can't get here in that case ... if so, then an assert might help document that.) uts/common/inet/ip_if.h 463: nit: what's the intended grouping (based on whitespace) here? -- James Carlson, Solaris Networking <james.d.carlson at sun.com> Sun Microsystems / 1 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
