Thanks for the review. Please see below: On 12/15/08 13:02, Nicholas Solter wrote: > Prasanna Kunisetty wrote: >> Hi, >> >> Solaris has simplified the IPMP architecture with clearview project >> and here are the changes needed for >> the integration of this feature into Colorado: >> >> http://cr.opensolaris.org/~kunisett/colorado-clearview/ >> >> Please review this and let me know your comments. >> > > Prasanna, > > usr/src/cmd/pnm/common/pnmd_network_solaris_ipmp.c > > Since the new code in nw_lognet_get() is not #ifdefed, I assume it > works fine on S10. Is that correct? The IFF_IPMP is only available with clearview, so I believe the code should be fine for S10. Basically for S10, the ioctl should not give any IPMP interface in the list. > > I'm not following this code: > > 1171 #ifdef LIFC_ALLZONES > 1172 if (alladdrs) > 1173 lifn.lifn_flags |= LIFC_ALLZONES; > 1174 #else > 1175 #ifdef LIFC_UNDER_IPMP > 1176 lifn.lifn_flags |= LIFC_ALLZONES | LIFC_UNDER_IPMP; > 1177 #endif > 1178 #endif > > > The else/if clause ors in LIFC_ALLZONES, but I think is only executed > when LIFC_ALLZONES is not defined. Hmm, I guess the if/else is reversed. If the LIFC_UNDER_IPMP is available, then the flags should be LIFC_ALLZONES | LIFC_UNDER_IPMP, else it should be just LIFC_ALL_ZONES.
Will change the code. Thanks, Prasanna > > The rest looks fine. > > Thanks, > Nick
