Cathy Zhou wrote:
> 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.
For example:
If in the future all the speed props of wifi link are supported which
means we can control the "current value" and "current desired values"
seprately,we need multiple values setting to "desired values list".But
since we can not make clear when or whether this feature would be
available by now.
Hmmm, meem and eric:
how do you think about this issue?
>
> 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().
If there would be one prop which is suppored by both nic and wifi, we
may need another structure to keep all the information of every prop and
it's type.Let's wait for feedback of meem and eric ;-) .
Thanks,
Tony
>
> Thanks
> - Cathy
