Thanks for looking at this.

On Tue, 2008-12-02 at 16:58 -0500, Peter Memishian wrote:
>  > http://cr.opensolaris.org/~seb/webrev.6774464/
>  > 
> 
> The error messages on 311 and 317 say they're printing the stack instance
> not the net instance -- though I'm not sure either one is really clear to
> the administrator as compared to e.g. the zone name.

Indeed.  I've rewritten these warnings to include the zoneid instead.

> As per our discussion yesterday, it's a shame there's no way to find out
> why net_protocol_lookup() failed, but I suppose the risk of it failing
> for another reason is slim.

That's right.  There's no way to glean why net_protocol_lookup() failed
from its returning NULL.

> Finally, it seems like net_protocol_lookup() grabs a reference.  So if
> e.g., the ips_ndv4 lookup succeeds but the ips_ndv6 lookup fails, we'll
> end up leaking a reference to ips_ndv4.  Seems like we need to call
> net_protocol_release().

The net_protocol_release() of each net handle is done in
ipnet_stack_fini() if the appropriate handle is non-NULL, so we're not
leaking any handles in this function.

Looking at ipnet_stack_fini() though, it looks like we do have a related
bug, which is that we VERIFY() that net_hook_unregister() always
succeeds.  If the call to net_hook_register() in
ipnet_register_netihook() failed or we never attempted to call
net_hook_register() due to one of the net_protocol_lookup() functions
failing, then the net_hook_unregister() can't possibly succeed.  The
code in ipnet_register_netihook() needs to be restructured to make sure
that the net handle is only non-NULL if we were able to do both the
net_protocol_lookup() and the net_hook_register().

I've re-spun the webrev.

Thanks,
-Seb



Reply via email to