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. > >> 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. > > > 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. > >> 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 :-) > > >> The -p option for dladm's set-linkprop and show-linkprop doesn't add >> value for me. Today, dladm's syntax is "dladm command -p prop=val >> link". >> I think it would easier to just have "dladm command prop=val link". >> Especially because -p is used elsewhere in dladm for producing >> parseable >> output. > > I too have frequently forgotten the "-p" in my usage and run into > this, but Meem's already addressed the motivation for this elsewhere. > >> There is a property called adv_autoneg_cap. I think it's cleaner to >> rename this one to "autoneg" or "linknegotiation". There's no such >> thing as an advertised autoneg capability. > > adv_autoneg_cap comes directly from the ieee802.3(5) definitions. If > we are going to rename this, we should be careful not to create > conflicts with other MII params like cap_autoneg. > > --Sowmini > > _______________________________________________ > brussels-dev mailing list > brussels-dev at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/brussels-dev -- Joost Mulders + email: joost.mulders at sun.com Technical Specialist + phone: +31-33-45-15701 Client Solutions + fax: +31-33-45-15734 Sun Microsystems + mobile: +31-6-5198-7268 -= Anything not done right, has to be done again =-
