Cathy,
> 1. Can you think of any non-physical link driver (or even softmac) could
> call mac_prop_init()s? I ask this question because it is impossible for
> pseudo links (aggr, VNIC, or even softmac) to provide the instance
> argument. That number is assigned by the GLDv3 framework.
This was discussed before and during ARC review and we feel that right
now mac_prop_init/get will be used by physical drivers only. The need
for drivers to actively retrieve properties is currently very limited,
mostly due to some drivers' design defects/peculiarities. Most drivers
should be able to live without these functions, relying on mc_setprop
callbacks only.
> 2. Assume the code path
> mac_start()->mac_prop_load()->dls_mgmt_linkprop_init()->
> dlmgmt_upcall_linkprop_init()->dlmgmt_getlink_by_dev() ...
>
> Note that dlmgmt_getlink_by_dev() can only be used for physical links,
> and even for physical links, the device name is not the same as the mac
> name (for example, softmacs). Therefore, when dlmgmt_getlink_by_dev() is
> called with mac_name, it will return ENOENT for softmacs and other
> non-physical links.
>
> The similar problem exists in dld_set_public_prop(). It assumes that mac
> name is the same as the device name.
Yes, I think this also chimes with Meem's comment about passing linkid
instead of name. It would be cleaner. I need to do some research. One
aspect that bothers me is interaction of dlmgmtd with partially attached
devices - i.e. the attach(9E)->mac_prop_init->mac_prop_load->... chain.
Do you know of any existing kernel functions to translate macnames
into devnames and vice versa?
> 3. I assume that you know that mac_prop_init() could fail when a new
> network interface is plugged in or you fresh-install a system. I guess
> since mac_prop_init() is used to apply the persistent link property
> configuration, those failures do not matter. But is a console message
> needed here if that fails (the bge_attach() function, line 3091)?
I personally don't see the need for console spam here. There's no
messages when ddi_prop_get_int() doesn't find the property, why should
mac_prop_get() be different. I asked Ted to review, maybe he can provide
his opinion also.
> 4. I don't see dladm init-linkprop in network/physical service anymore.
> How link properties like autopush can be applied after your changes?
Well, this what this RFE is all about: to remove the need to follow each
ifconfig plumb with dladm init-linkprop. When a link is plumbed, MAC
start will trigger dladm_init_linkprop() and all these properties,
including autopush, will be automatically sent to the kernel. I tested
autopush, among others - it works.
Now, as Sowmini noted, we still need dladm init-linkprop to push
properties _after plumb_ for wireless drivers. So I'm currently adding
an option to initialize wifi properties only, what you'll see in
network/physical is:
dladm init-linkprop -w
> mac.c
>
> - It does not look right to call mac_prop_update() for the ENOTSUP case
> (line 2952).
Will fix.
> - mac_get_prop() does not seem to be symmetric to mac_set_prop(). I
> guess the reason that DLD_PROP_FLAG_MAC_ONLY only applies to
> mac_set_prop() is because in that case, the property is already
> initialized in the underlying driver. But the current flag name is
> misleading because it seems that this property only needs to be known by
> the mac layer. Can we change the name of DLD_PROP_FLAG_MAC_ONLY to
> something like DLD_PROP_FLAG_DRIVER_IS_SET?
I feel DLD_PROP_FLAG_DRIVER_IS_SET is opaque. Naming is subjective, it's
a matter of perception. Luckily, we don't have to rely on just name to
derive its meaning: we can go to definition and read the comment. I
think it's clear, but I could make it clearer.
> linkprop.c
>
> - In dladm_init_linkprop() function, it does not seem right to take
> (flags | DLADM_OPT_PERSIST) as the last argument. Note the possible
> flags value here is either DLADM_OPT_PROP_MAC_ONLY or
> DLADM_OPT_PROP_MAC_NAME.
You're right, I should pass the flags via 'argp' argument instead.
thanks,
-Artem
_______________________________________________
networking-discuss mailing list
[email protected]
