[brussels-dev] [clearview-discuss] 2008/056 Brussels addendum
Sebastien Roy
Sebastien.Roy at Sun.COM
Wed Jan 30 10:20:35 PST 2008
Sowmini.Varadhan at Sun.COM wrote:
> On (01/29/08 09:27), Sebastien Roy wrote:
>> I'm sponsoring this case for Sowmini Varadhan. It is comprised of a
>> small number of fairly obvious and minor changes to the Brussels project
>> (which integrated into ON last week), and is thus closed approved automatic.
>
> I have the changes related to this (and other misc merge-fixes) at
>
> http://cr.opensolaris.org/~sowmini/rfe/
>
> Could you please review? I'll fix up the CRs after all the review
> comments come in.
Looks good. Only a couple of comments:
usr/src/lib/libdladm/common/linkprop.c
192: The alignment of one of the columns is off.
204: The description is not quite accurate. The term "frame" is specific
to Ethernet, and the MTU property isn't the "default", but the "current"
link MTU. "current link MTU" would be more accurate IMO.
1182-1229: The checks for DL_ETHER make the code inherently not scalable.
I think the only two cases are DL_WIFI, and not DL_WIFI. In other
words, WiFi is the only special case, and every other media type is the
norm. I believe you've started to codify this at line 311 by making the
property applicable to DATALINK_ANY_MEDIATYPE, but stopped there. A
related question is, how does DL_IB treat speed?
1189,1209,1268: The caller of these functions already knew the media type
so that it could compare it against pd_dmedia. It's inefficient to go
the door call a second time to get the same information that we already
had a moment ago. A better approach might be to simply pass in the media
type as an argument to these functions.
usr/src/uts/common/io/bge/bge_main2.c
1274: cstyle: err is not a boolean.
-Seb
More information about the brussels-dev
mailing list