xiaodong zhang - Sun Microsystems - Beijing China wrote:
> Cathy Zhou wrote:
>>>> To change dladm autopush to dladm ***-linkprop, I'd like to move
>>>> some of general linkprop logic from libwladm to libdladm
>>>> (specifically, linkprop.c), because link properties are no longer
>>>> wifi specific anymore. For example, the following logic should be
>>>> general enough: the list of possible values for each property and
>>>> do_check_prop() function; the default value of specific property;
>>>> read only property, etc., and maybe the pd_getmod() logic.
>>>>
>>>> Does this sound reasonable?
>>>>
>>>> I am also reading the libwladm code. Why the allocation of wldp_t
>>>> (gbuf) cannot be the internal implementation of the pd_get(),
>>>> pd_set(), pd_getmod() callback? So that the second argument of these
>>>> callbacks is not needed?
> All this callback would call the corresponding do_set(get)_* functions
> at last.All the do_set(get)_* functions have same prototype with a gbuf
> parameter, and we do this because most the do_set(get)_functions are
> used in do_connect which called by wladm_connect in which do_connect may
> be called several times.So we can malloc a gbuf and pass it to
> do_connect(..) in which several do_set_* functions are called and pass
> the gbuf to all these functions.From this view I thinkt that it's some
> better to malloc gbuf out of do_set(get)_* functions.
>
> Ofc, for the wrapper of these do_set(get)_* functions such as *_prop
> functions used to set(get) linkprop of one link, we may do mallocing of
> gbuf in these wrappers before calling to the do_set(get)_* functions.How
> to handle this issue may need more discusss.
>
Well. Like I said, I'd like to move some general logic of set/get_linkprop
from libwladm to libdladm. As the part of this work, do_set(get)_prop will
be moved to libdladm too. I found it is very hard to understand why
do_set(get)_prop() takes an gbuf argument.
>>> Oh, forgot one question. For which property, val_cnt can be greater
>>> than 1 in wladm_set_prop()?
>>>
>> I talked to Tony and he told me that we used to allow multiple values
>> to be set for the "speed" property but it is not true anymore. Do we
>> still want to keep this possibility open? If not, the code can be
>> simplified.
> It's true that by now all the linkprops of wifi can only have one value
> to set.Since we need identify the reset operation from set operation,so
> at least one boolean_t parameter is needed ,I think. So I think it's
> better to keep the current interface which can support both single and
> multiple values setting if we want to support multiple setting in
> future.Is this right :-\ ?
>>
What I want to understand is that whether we see any possible use of this
(setting multiple value for one property) in the future. If not, I don't why
we keep it and only complex the code.
Plus, in the wifi PSARC material, it is said:
Implementation Notes:
dladm_set_prop() is meant to be a wrapper for media-specific set_prop()
interfaces. Its internal structure is similar to:
dladm_set_prop(...)
{
...
if (wladm_is_valid(link)) {
s = wladm_set_prop(link, ...);
....
goto save;
}
if (ether_is_valid(link)) {
s = ether_set_prop(link, ...);
....
goto save;
}
...
save:
if ((flags & DLADM_OPT_TEMP) == 0)
save_config(link, ...);
...
}
This is not true. which function to call is not dependent on the type of the
link, but the property. For example, "autopush" is a property for all links
so that it might call dladm_gen_set_prop(), and "speed" is a property of
wifi, so that it call wifi_set_prop().
Thanks
- Cathy
