Hi Nicolas,

On Fri, 2009-08-14 at 21:10 -0700, Nicolas Droux wrote:
> One goal is to prevent multiple clients to simultaneously update the  
> MTU. The perimeter should be sufficient to do this.

I agree.  This should be no different than updating any other property,
really, which simply involves holding the perimeter.

> There are also  
> drivers which cannot change their MTU while they are started, but on  
> the other hand some drivers may support this, and therefore we could  
> depend on the drivers to do test for this condition instead of doing  
> this from mac.

Yes, I've tested this with a few drivers and have inspected all drivers
in ON, and drivers respond with ENOTSUP if they don't handle setting the
"mtu" property in their property set callbacks.

> I don't remember issues related to re-entry with aggr from the top of  
> my head. If there is one I agree that this should not require a  
> workaround like this in mac. BTW, it's a bit silly to require a driver  
> to call mac_maxsdu_update() while updating its MTU. If the framework  
> is calling the driver to update the MTU it should know that the MTU is  
> changing.

I totally agree, but the semantics should be well-defined either way.
For example, if indeed the mac module is modified to change mi_sdu_max
and send notifications itself, then the (upcoming) driver API
documentation should make it clear that the mac_maxsdu_update() would
only need to be called when the MTU changes in all cases except for the
"mtu" link property.

> But this is really not even address the real issue.

I agree, my motivation was to split this small bit of code-trivia out of
the CR I mentioned, which is still a problem, and fix it along with IP
tunneling.

> There's still the  
> risk of having mac_perim_enter_by_macname() fail when it's called from  
> dls_devnet_close() if there's any operation holding the perimeter in  
> progress. dls_devnet_close() should be able to handle the case where  
> there are pending MAC clients accessing the MAC instance and wait  
> until all references to the mac_impl_t are dropped, or we should  
> ensure that dls_devnet_close() is called only after these references  
> are dropped, or we should change dls_devnet_close() to handle the case  
> when it cannot enter the perimeter.

If dls_devnet_rename() is the only remaining consumer of this exclusive
flag, then perhaps we need to rethink this.  If dls_devnet_close() is
being called, then certainly there is a reference on the dls_devnet_t
that was held on open, and dls_devnet_rename() will return EBUSY and
won't get to set the exclusive flag.  So I think if rename is the only
consumer, then dls_devnet_close() _can_ assert that
mac_perim_enter_by_macname() won't return EBUSY.

The problem is then, do we want dls_devnet_rename() to be the only
consumer of this exclusive interface in the future to ensure that this
fragile bit of code keeps working?  Or do we need a more complex
mechanism to allow dls_devnet_close() to work in the face of any
possible future consumer of mac_mark_exclusive()?

-Seb



Reply via email to