Anurag S. Maskey wrote:
> Code review for
>
> 11103 - 'ncu:ip' should not go into 'online' at first, instead of
> 'offline*'
>
> http://zhadum.east/export/ws/am223141/temp/nwam1-work/webrev/
>
> The fix examines the NCU state when DHCP times out and only changes to
> offline*, timed out if the NCU is not in the online state.
So the reasoning here is that if we're already online, another
address has been assigned on the link, right?
> Also, an interface with 0.0.0.0 is disregarded when checking if an
> interface is managed by DHCP.
ncu_ip.c: nit here, and maybe it's just me but would it
be better to get rid of the "goto done_with_interface"
stuff and just do "icfg_close(ifh);continue;" at line
518, 528 and for the logic where we break
out of the loop, just do something like:
if (icfg_get_flags(ifh, &flags) == ICFG_SUCCESS &&
(flags & IFF_DHCPRUNNING) == IFF_DHCPRUNNING) {
/*
* If we get here we have an address that has the DHCP flag
* set and isn't an expected static address.
*/
icfg_close(ifh);
rv = B_TRUE;
break;
}
It just feels a bit strangely constructed right
now. I guess it's a bit repetitive using the icfg_close(ifh),
but it would probably scan better - it took a little
while to figure out the logic here. Not a big deal though.
>
> The webrev also changes the syslog level of message when state change
> events are enqueued.
>
> Finally, I noticed that sometimes multiple LINK UP events are received
> and processed by nwamd. I added code to ignore LINK UP state event if
> the link NCU is already in an online state.
>
Good catch!
Alan
