Peter Memishian writes: > A number of your comments relate to the symlink/lintlib/headers for > libipmp. The libipmp library is Contracted Consolidation Private to the > Sun Cluster consolidation, which is historically why we've provided this > stuff; changing this seemed unrelated to Clearview IPMP and thus we've > continued to provide those things, albeit in /lib instead of /usr/lib > (because we had to move several IPMP programs into the root filesystem). > Yes, we could provide a separate internal package; we could also do that > in Nevada -- again, it seems like a separate bit of work and I'd rather > not do it with this wad.
OK; that seems reasonable to me. It sounds like a separate RFE. > > The RT_AWARE name seems a little vague ... RTS_ADDRTYPES or > > RTS_IFTYPES or some such might be more accurate. > > It's intentionally general, in that it's expected to generally convey the > consumer's "awareness" with respect to routing socket events. It could be > expanded in the future to cover other LIFC_* flag cases. It looked to me like it might select interest in particular kinds of addresses. Otherwise, ok, the current name is fine. > > usr/src/cmd/rcm_daemon/Makefile.com > > > > 123: what does '-L$(ROOT)/lib' do here? Doesn't the right thing > > happen by default? > > It doesn't; LDLIBS_MODULES doesn't automatically include the library > search path. I don't know why they did it this way. Weird. OK. > > usr/src/pkgdefs/SUNWcsr/prototype_com > > > > 405: nit: I'm not sure if it makes sense to have this in the > > administrator's search path, as there's nothing the administrator > > ought to be doing with it. "/lib/inet/in.mpathd" would be slightly > > better. > > It's been in the administrator's search path since S8. Further, > unfortunately, third-party software is aware of this location > (and the new location) (e.g., to restart it by hand). I think it's hard to work up sympathy for someone picking bits out of /usr/lib/inet ... but it's not so important that I'd object to it. > > 169: nit: wput doesn't have to return anything. (Yeah, I realize > > that's part of your text-line-conserving return-for-error code.) > > Yes; I considered both and prefer the way it is. OK. > > usr/src/uts/common/inet/ip/rts.c > > > > 560,562,711,713: nit: I don't think these locks do much of anything. > > Yeah it's a little weird, but status quo for conn_lock and options -- > e.g., see ip_opt_set(). Sigh ... ok. > > usr/src/uts/common/net/if.h > > > > 174: instead of "traditional interface," I suggest saying "any > > interface." (The former makes it sound as if there are interfaces > > on which the user can change some of the IFF_CANTCHANGE flags from > > user space ... and that's not true.) > > Except that they can before SIOCSLIFNAME is set, but that's another matter > :-O I'm not sure that using SIOCSLIFNAME is the key that makes it a 'traditional' interface. The reason I suggested the wording change was that the text here is confusing. The bits are additive; no interface can change these, and IPMP ones have an additional set of reserved bits. "Traditional" seems to exclude something. > > 601: is this equivalent to "gi_nv4 != 0"? > > 602: and is to "gi_nv6 != 0"? > > No. Perhaps the comments are too terse but, gi_v4 and gi_v6 represent > whether the IPMP group interface is plumbed for IPv4/IPv6, whereas the > gi_nv4 and gi_nv6 counters represent the number of underlying IPv4 and > IPv6 interfaces. For example, if one does "ifconfig ipmp0 ipmp", then > gi_v4 will be B_TRUE, but gi_nv4 will be zero. Oh, ok. So you could not reasonably have gi_v4 set to B_FALSE when gi_nv4 is greater than zero, but an empty set with gi_v4 set to B_TRUE and gi_nv4 equal to zero *is* supported. I get it now. > > usr/src/uts/common/net/route.h > > > > 265-266: I don't think I agree with using the RTA_.* name space > > (which is currently used for the rtm_addr bit mask values) for this > > new purpose. It'll get confusing in applications, and invites > > error. Please come up with a new name (RTAA_.*?). > > I went with RTAW_ Sounds good. > > usr/src/uts/common/sys/sockio.h > > > > 238: curious ... if we can remove them safely, why can we not reuse > > them? > > Because we'd like any lingering consumers to fail, rather than silently > take on new behavior. Actually, since the data size is part of the ioctl number, we could reuse them in at least some cases ... but ok. My point here was that in order to break those lingering consumers, I think we really had to judge that there simply weren't any to speak of, because we try hard not to break working binaries. That makes eventual reuse less of a problem. -- James Carlson, Solaris Networking <james.d.carlson at sun.com> Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
