[brussels-dev] [networking-discuss] code review Brussels persistence

[Cathy Zhou]

Artem Kachitchkine wrote:
> 
>> But note mac_start() will be moved to be part of DL_BIND_REQ 
>> processing soon (as part of the Clearview softmac fast-path work). In 
>> my opinion, mac_start() should have been called as the result of 
>> DL_BIND_REQ, as only from then on, the driver is allowed to transfer 
>> data.
>>
>> So my question changes to: Is it possible to move mac_prop_load() call 
>> from mac_start() to mac_open()?
> 
> I tried this and I'm getting a deadlock between the two threads:
> 
> dls_devnet_open
>   ...
>     softmac_create
>       mac_open
>         mac_prop_load
>           upcall to dlmgmtd
> 
> dlmgmtd
>   ...
>     dld`drv_ioc_phys_attr
>       softmac_hold_device
>         mutex_enter(smac_mutex)
> 
> 
> softmac_create calls mac_open with smac_mutex held, and when upcall 
> handler tries to map linkid to devname, it deadlocks. It seems that 
> mac_open might be a little too early for mac_prop_load.
> 
Before we get to that, I still have problem of the general design here. Note 
that dls_mgmt_linkprop_init() takes mac name, but once this mac name is passed 
to dlmgmtd, dlmgmtd cannot be able to identify either the datalink name or 
linkid because it doesn't have the mac name information. Although for GLDv3 
phhysical links, mac name is the same as its device instance name, I would 
think this code path is needed for aggregations, or legacy physical links (and 
other classes of links) to automatically apply their link properties, such as 
autopush.

I understand yesterday that we talked about mac_prop_init() only works for 
GLDv3 physical links, but I think mac_prop_load() is different, because 
mac_start()->mac_prop_load() should be used for all links.

Speaking of the "calling mac_prop_load() by softmac" idea, I don't think that 
would work because the same reason: softmac only knows physical links, and 
mac_prop_load() should work for all classes of links.

Thanks
- Cathy