I think something like this could work:
int
dls_devnet_destroy(mac_handle_t mh, datalink_id_t *idp)
{
int err;
mac_perim_handle_t mph;
mac_perim_enter_by_mh(mh, &mph);
*idp = DATALINK_INVALID_LINKID;
err = dls_devnet_unset(mac_name(mh), idp, B_FALSE);
if (err != 0) {
mac_perim_exit(mph);
return (err);
}
err = dls_link_rele_by_name(mac_name(mh));
ASSERT(err == 0);
mac_perim_exit(mph);
return (err);
}
static int
dls_devnet_unset(const char *macname, datalink_id_t *id, boolean_t wait)
{
dls_devnet_t *ddp;
int err;
mod_hash_val_t val;
rw_enter(&i_dls_devnet_lock, RW_WRITER);
if ((err = mod_hash_find(i_dls_devnet_hash,
(mod_hash_key_t)macname, (mod_hash_val_t *)&ddp)) != 0) {
ASSERT(err == MH_ERR_NOTFOUND);
rw_exit(&i_dls_devnet_lock);
return (ENOENT);
}
if ((err = dls_link_is_busy(macname)) != 0) {
rw_exit(&i_dls_devnet_lock);
return (err);
}
mutex_enter(&ddp->dd_mutex);
....
}
int
dls_link_is_busy(const char *name)
{
dls_link_t *dlp;
if (mod_hash_find(i_dls_link_hash, (mod_hash_key_t)name,
(mod_hash_val_t *)&dlp) != 0)
return (ENOENT);
ASSERT(MAC_PERIM_HELD(dlp->dl_mh));
/*
* Must fail detach if mac client is busy.
*/
ASSERT(dlp->dl_ref > 0 && dlp->dl_mch != NULL);
if (mac_link_has_flows(dlp->dl_mch))
return (ENOTEMPTY);
return (0);
}
notes:
-the DD_CONDEMNED flag has the effect of disabling the dls_devnet_t
and ensures that subsequent calls to dls_devnet_hold_tmp() would
fail (indirectly called by mac_link_flow_add())
-we do not wait in dls_devnet_unset() because this could deadlock
if someone is holding a tmp reference. softmac_destroy() already
does this so it should be ok to change aggr/vnic to behave the
same way.
let me know if this makes sense.
eric
On Fri, Jan 16, 2009 at 03:17:34PM -0800, Cathy Zhou wrote:
> On 2009?01?16? 14:34, Sebastien Roy wrote:
> > On Fri, 2009-01-16 at 13:33 -0800, Cathy Zhou wrote:
> >> On 2009?01?16? 13:16, Sebastien Roy wrote:
> >>> Hi Crossbow team,
> >>>
> >>> I have a question on the modifications made to dls_devnet_destroy():
> >>>
> >>> int
> >>> dls_devnet_destroy(mac_handle_t mh, datalink_id_t *idp, boolean_t wait)
> >>> {
> >>> int err;
> >>> mac_perim_handle_t mph;
> >>>
> >>> *idp = DATALINK_INVALID_LINKID;
> >>> err = dls_devnet_unset(mac_name(mh), idp, wait);
> >>> if (err != 0 && err != ENOENT)
> >>> return (err);
> >>>
> >>> mac_perim_enter_by_mh(mh, &mph);
> >>> err = dls_link_rele_by_name(mac_name(mh));
> >>> mac_perim_exit(mph);
> >>>
> >>> if (err != 0)
> >>> (void) dls_devnet_set(mac_name(mh), *idp, NULL);
> >>> return (err);
> >>> }
> >>>
> >>> Once the call to dls_devnet_unset() has succeeded, is it possible for
> >>> dls_link_rele_by_name() to fail? I ask because the subsequent call to
> >>> dls_devnet_set() upon dls_link_rele_by_name() failure is problematic.
> >>> It this dls_devnet_set() call fails, the system is rendered hosed until
> >>> rebooted.
> >>>
> >> It looks possible. As long as there are still subflows added on this link,
> >> it would fail.
> >
> > Is there a way to fix this? There has to be some way to freeze the
> > state of things once it's been verified that teardown is safe (sort of
> > like what mac_disable() was intended to do). That way, one can assert
> > that the various steps involved in teardown cannot fail.
> >
> Yes. I believe it is possible. Can we add a dls_devnet_disable() function,
> which checks whether we can destroy dls_devnet_t, dls_link_t (or even
> mac_impl_t?) and set the disabled flag on each of them. The disabled flag
> will be used to prevent further usage of those structures.
>
> Thanks
> - Cathy
> _______________________________________________
> crossbow-discuss mailing list
> crossbow-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
