On Tue, 2008-10-07 at 19:05 -0400, Peter Memishian wrote: > First, thanks to all who came to the kick-off meeting. For those unable > to attend, everything you need is linked from the code review webrev at: > > http://cr.opensolaris.org/~meem/clearview-ipmp-cr/
Here are some comments on in.mpathd and IPv6 source address selection. We've already addressed some of the in.mpathd issues in person. I also reviewed the in.ndpd and in.routed changes and had no comments. usr/src/cmd/cmd-inet/usr.lib/in.mpathd/Makefile * 45: Does "-lc" really need to be explicitly included in LDLIBS? usr/src/cmd/cmd-inet/usr.lib/in.mpathd/mpd_main.c * 362: We don't care about failure here? * 362: I find it odd that in.mpathd keeps a linked-list of all of the local addresses on the system for the purpose of verifying if a probe target is a local address. The comment above own_address() in mpd_probe.c actually suggests a (IMO) better method, which is to simply call SIOCTMYADDR instead. * 1872: I think there is potential to grab an IRE_INTERFACE for an interface that's not part of the same group as the output interface of the "rp" route, since the next-hop is link-local, and every interface on the system will have a link-local IRE_INTERFACE route. Somehow, this needs to check that the rp1 is associated with the same group as rp. * 1872: On a related note, I'm not sure that this prefix_equal() call here does anything particularly useful given that the test addresses are always link-local and the probe targets also need to be link-local. As a result, any link-local next-hop will be good enough as a target, and any link-local IRE_INTERFACE will work. * 2483: This is simply !IN6_IS_ADDR_LOOPBACK(), and you don't need "loopback_addr". usr/src/uts/common/inet/ip/ip6_if.c * 2361: I can try and guess why you've decided to restrict the candidate set to the IPMP ill; is it so that you don't look at underlying ill's? This code leaves out other valid ill's for IPv6 source address selection. The idea of the algorithm is that addresses on all IPv6 ill's on the system are in the candidate set of IPv6 source addresses, but that addresses on the output ill are preferred over those that are not (by virtue of one of the selection rules). If the intent is to leave out underlying ill's from the candidate set, then I'd be more comfortable with skipping them in the ill-walking loop. -Seb
