Thanks for the review!
On (01/31/08 13:09), Cathy Zhou wrote:
> dladm.c:
>
> - line 1458: should be "VLAN" instead of "LAN".
ok.
> - Line 1701: why this is needed?
Otherwise (as a result of line 1683) we'd end up with output like
"-- bge0 e1000g0" etc.
> - 1721-1722: these are not needed when (i == ginfo.lg_nports - 1).
Ok, will make this conditional.
> linkprop.c:
>
> - Does "speed" apply for all media type or only ETHER and WIFI? If it is
> the former, I think in the do_get_rate_prop() function, the code would be:
>
> if (media == DL_WIFI)
> return (do_get_rate_common(pd, linkid, prop_val, val_cnt,
> WL_DESIRED_RATES));
>
> return (dld_speed_get(pd, linkid, prop_val, val_cnt));
>
The goal is that speed applies to all media types (even though we
don't have brussels support in all drivers). I'll be sending out
a new webrev today based on discussions yesterday, but Seb and I
agreed that this made the most sense (until DL_WIFI exports
mc_setprop/mc_getprop interfaces for its properties):
static dladm_status_t
do_get_rate_prop(struct prop_desc *pd, datalink_id_t linkid,
char **prop_val, uint_t *val_cnt, datalink_media_t media)
{
if (media != DL_WIFI)
return (dld_speed_get(pd, linkid, prop_val, val_cnt));
return (do_get_rate_common(pd, linkid, prop_val, val_cnt,
WL_DESIRED_RATES));
}
Do you agree?
> - Line 1277: better to return DLADM_STATUS_PROPRDONLY.
Ok.
--Sowmini
