> The webrev for the changes is here:
>
> http://zhadum.east/ws/pk34663/ipobs-only-hg/webrev
Do we really need IPNETIF_CLOSING, or would the same thing be accomplished
by doing an ipnetif_refhold() in ipnet_create_if() and an
ipnetif_refrele() ipnet_remove_if()? That is, could the same thing be
accomplished in a simpler manner by merely regarding the global visibility
of the ipnetif_t in the AVL trees as itself being a reference?
I'd also still like to understand why we need to have sdev_ipnetops.c (or,
moreover, anything outside of ipnet.c) interacting with ipnetif_t objects.
It just makes things more complex. Is there a reason why my earlier
suggestion of introducing and ipnet_getdev() function (which would just
return a dev_t) won't work for sdev_ipnetops.c? This would allow us to
remove ipnet_if_getby_name() (since the only consumer in sdev_ipnetops.c).
In the current code, I also don't see how the ipnet_populate_if()
reference counting stuff works -- i.e., if ipnet_if_getby_index() returns
non-NULL, then we hold a reference to an ipnetif_t, otherwise we call
ipnet_create_if() and we don't hold a reference (though this would be
addressed if ipnet_create_if() acquired a reference as suggested above).
Speaking of ipnet_populate_if() (and ipnet_addrchg_ev()): if the kernel
can handle the stack usage (which I think should be OK since it's only 416
bytes more than your current stack utilization), it be much cleaner to
revise the definition for ipnet_create_ifaddr() and declare addresses as
sockaddr_storage -- e.g.:
struct sockaddr_storage addrs[2];
net_ifaddr_t type[2];
phy_if_t phyif;
lif_if_t lif;
ipnetif_addr_t *ifaddr;
ipnetif_t *ipnetif;
zoneid_t zone;
char name[LIFNAMSIZ];
type[0] = NA_ADDRESS;
type[1] = NA_BROADCAST;
for (phyif = net_phygetnext(nd, 0); phyif != 0;
phyif = net_phygetnext(nd, phyif)) {
/*
* If net_getifname() fails, then there's no name for the
* interface, in which case we're best off failing returning
* an error and failing the attach.
*/
if (net_getifname(nd, phyif, name, LIFNAMSIZ) != 0)
return (DDI_FAILURE);
if ((ipnetif = ipnet_if_getby_index(phyif, ips)) != NULL) {
ipnetif = ipnet_create_if(name, phyif, ips);
if (ipnetif == NULL) {
cmn_err(CE_WARN, "ipnet_populate_if:"
"ipnet_create_if() failed");
return (DDI_FAILURE);
}
}
for (lif = net_lifgetnext(nd, phyif, 0); lif != 0;
lif = net_lifgetnext(nd, phyif, lif)) {
if (net_getlifzone(nd, phyif, lif, &zone) < 0 ||
net_getlifaddr(nd, phyif, lif, 2, type, addrs) < 0)
continue;
ifaddr = ipnet_match_lif(ipnetif, lif, family);
if (ifaddr == NULL) {
ifaddr = ipnet_create_ifaddr(addrs, zone,
lif, family);
ipnet_if_add_ifaddr(ipnetif, ifaddr, family);
}
}
}
--
meem
