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
