Joost Mulders wrote: > Hi Sowimi, > > Thanks for your response. I have a few remarks inline. > > On Sep 12, 2008, at 3:12 PM, Sowmini.Varadhan at Sun.COM wrote: > > >> On (09/12/08 00:55), Joost Mulders wrote: >> >>> There are 2 sets of properties named MAC_PROP_ADV_ and MAC_PROP_EN_. >>> There is a 3rd set, the defaults, that is implemented as a bit named >>> >> Well, the MAC_PROP_DEFAULT flag is not specific to the adv/en >> versions- >> it applies to all properties, and is a flag asking the driver to >> return the default value for that property. I think that your >> suggestion >> of adding a parallel set of properties to return default would result >> in propery space bloat. >>
Possibly, but this is a really sort a design detail where the bloat impact isn't clear -- code has to be supplied in either case, so the amount of text difference is probably minimal between the two approaches. Using separate properties would consume more space in the property namespace, but I've not viewed that as a limited resource. >> >>> MAC_PROP_DEFAULT in pr_flags. It was unclear to me for which >>> properties >>> defaults are actually retrieved. It might be cleaner to spell out all >>> properties for which defaults are required as MAC_PROP_DEFAULT_. >>> >> dladm makes the decision based on whether the default value is >> known globally for all drivers, or if the default >> value depends on the driver implementation/hardware/link-type >> etc. In the latter case, the driver will be contacted. >> > > Thus drivers must return defaults for all properties. > Yes. Or at least for all properties that they implement. dladm nor the framework needs to know where the defaults come from. And if you try to make a decision that the framework should supply defaults for some drivers, you will inevitably wind up with a new hardware which needs a different default from everything else, in which case you might have painted yourself into a corner. > >> if the concern is that you don't know which ones to implement, then >> you could safely implement return values for all of them that are >> supported. >> > > And thus the bloat? (is that http://en.wikipedia.org/wiki/Bloat) of > properties in the driver. If dladm spells out exactly for what > Ethernet properties defaults are required, drivers don't need to > implement say a default for > say MAC_PROP_ADV_1000FDX_CAP and MAC_PROP_EN_1000FDX_CAP. > Ah, but that's a good example where the default value may be very different for different kinds of hardware! 10GbE CX4 will have that default to off (and probably won't allow any other setting), but 10G-BaseT would have it enabled. Put shortly, this is a case where I'd rather not endow the framework too many brains. > > >>> When adv_autoneg_cap is 0, a link's speed and duplex should be >>> writable >>> properties. Currently, these are readonly, which results in funny >>> implementations. e1000g for example, selects the "best" speed and >>> duplex >>> from the enabled set (MAC_PROP_EN_*_CAP) when autoneg is turned >>> off. I >>> found this confusing. IMHO, it would be better to have speed duplex >>> readonly when autoneg is on and writable when autoneg is off. >>> >> There was a long discussion around this in the early stages >> of Brussels. See >> http://mail.opensolaris.org/pipermail/brussels-dev/2007-June/000192.html >> My understanding is that speed/duplex can only be strictly set >> as pairs, hence the 10fdx, 10hdx, 100fdx etc. hooks. >> > > I don't understand this requirement. I don's see why speed and duplex > can be independantly writable when autoeg is off, but I don't think > it's important to make fuss about it :-) > Auto negotiation actually locks them together -- if you look at the 802.3u spec it basically negotiates for the best "mode", which is a combination of link speed and duplex (that's not quite perfectly true, because 100T4 is also a "mode"). If you disable autoneg, you had better specify *both*, because otherwise its a crap shoot whether the configuration chosen by the device will work. -- Garrett
