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

[Cathy Zhou]

Artem Kachitchkine wrote:
> First I'd like to thank you for the interest in our work and timely 
> responses; the Brussels team really appreciates this. I've posted the 
> latest webrev:
> 
> 	http://cr.opensolaris.org/~artem/pers/
> 
> Summary of change:
> 
> - Following a productive discussion with Cathy, I moved 
> mac_prop_load/unload invocations from MAC to DLS. Properties are loaded 
> when a network device is first opened. They are unloaded lazily when the 
> devnet structure goes away (rather than in dls_devnet_close, which would 
> be way too frequent). I wasn't sure about the level of concurrency in 
> dls_devnet_open_by_dev(), so I tried to be on the safe side.
> 

As we discussed offline, calling mac_prop_load() in dls_devnet_open_by_dev() 
is fine, but one thing to note is ddp->dd_vlanid could be 
DATALINK_INVALID_LINKID at this point, if this is a /dev node which is plumbed 
when dlmgmtd is not started yet. At that point, its linkid is not assigned.

Further, as we discussed, adding mac_prop_load() in dls_devnet_open_xxx() code 
path will not be able to initialize the link properties for physical links if 
they are used by some MAC clients (for example, when bge1 is added into an 
aggregation). Alternatively, I am wondering whether it is possible to call 
mac_prop_load/unload() in dls_devnet_set/unset(), so that persistent link 
properties would be populated whenever the link exists.

Thirdly, like I mentioned to you, can we make mac_proplist_t to be based on 
linkid instead of spa and move proplist to be part of the dls module instead 
of the mac module? Doing that, I think we could simplify the code a bit.

Let me know if the above is doable. If not, please also let me know and I will 
continue the review.

Thanks
- Cathy

> - As a result, linkid is now the "unit of currency" for the door upcall. 
> We only use macname (aka devname for physical devices) for the 
> mac_prop_init/MAC_ONLY case. Property lists are now keyed by macname/vid 
> (aka spa). We now handle vlan and aggregation properties more correctly 
> than before.
> 
> - dladm init-linkprop is back in net-physical, with the -w[ireless] option.
> 
> - Incorporated other comments from Sowmini and Cathy.
> 
> Old webrev is still there, as well as new-vs-old webrev:
> 
> 	http://cr.opensolaris.org/~artem/pers-0421/
> 	http://cr.opensolaris.org/~artem/pers-27vs21/
> 
> Cscoped ws is still /net/aja.sfbay/export0/brussels/br-pers/
> 
> -Artem
> 
> _______________________________________________
> networking-discuss mailing list
> networking-discuss at opensolaris.org