Looking over this, a few thoughts. 1) As you've discovered, some duplication exists with KSTATs. It strikes me that perhaps a better way to achieve this is to avoid the whole separate KSTAT mess altogether. By that, I mean Brussels properties could be obtained from the getprop() interface, and a default implementation for getstat() can be supplied by Brussels for the benefit of Brussels drivers. (Having some properties in KSTATs, and some in Brussels, seems, um, weird.)
2) DLD_NDD_READ|DLD_NDD_WRITE -- it would be clearer to have a DLD_NDD_RW macro. 3) MAC_KSTAT. Assuming you don't do anything about point 1above, why not just have DLD_NDD_KSTAT which can be in the same flags field as DLD_NDD_READ, DLD_NDD_WRITE. It would imply read-only access, of course. Generally, this is a big step in the right direction. Once this (or something like it) gets in, I'll get aggressive about eradicating ndd hackery from various GLDv3 drivers. More below: sowmini.varadhan at sun.com wrote: > Ok, so I've spent the last week trying out the idea of having > mac map everything into GLDV3 calls, so that the driver does not have > to register anything or muck with ndd in any way (see "Previous > Discussion" below). The webrev is at > > http://cr.opensolaris.org/~sowmini/nddcompat/ > > For those inside SWAN, webrev and cscope are at > http://zhadum.east/export/ws/sowmini/brussels/nddcompat > /net/zhadum.east/export/ws/sowmini/brussels/nddcompat/usr/src[/uts] > > and is doubtless cleaner, in that the driver is now swill-free. > There are still some XXX's that I need to get to, but meanwhile > there are some lingering Interface related questions: > > 1. The "outliers" (properties like "link_tx_pause" that do not map to > something that is known to mac_ndd.c) are implemented as private > properties in the driver. Previously (see > http://mail.opensolaris.org/pipermail/brussels-dev/2007-August/000388.html) > we had decided that private properties MUST start with "_", so we can > either > > 1.1 make the driver implement the property as "_link_tx_pause" and > have mac_ndd.c prefix the "_" when calling the driver > > 1.2 go with my prototype, which would make the ndd outliers > inaccessible from dladm. Note that /sbin/dladm filters out > properties that don't start with "_" > > I personally have no personal preference between 1.1 and 1.2. > I don't have much preference either. I'm happy with 1.2. > 2. Currently, to provide the list of ndd names for the > "ndd -get /dev/foo \?" command, the mac_ndd_get_names first goes > through the known mappings in mac_ndd_mapping[], and then needs > to get the names of the outliers from the driver. > > In my prototype, I've made the choice of using a private-property > called "ndd_names" to pass this information up, but I'm not completely > happy with this (it seems better to have a clearly defined interface > for this). Here are the alternatives I can think of: > > 2.1 Create a public property that will print out the names of the > private properties. We could use the DLD_PROP_NDD_LEGACY prop num > for this property, and it's implementation would look mostly like > the code in my webrev for ndd_names. > > Cons: The output of "ndd -get .. \?" does not fit into the tabular > format that is used by dladm. To work around this, we could > make this property inaccessible from dladm/libdladm (i.e., > only to be used between mac and the driver for legacy ndd > support) > It really only belongs, IMO, in the ndd output. One can even question the value of having it there at all. I would not be adverse to an ARC case proposing that the '?' property simply be removed or not supported by Brussels based drivers. I think the backlash on this would be either very small or non-existent. The number of cases of scripts that parse this output is probably quite small -- I know that QE has some, but they should be converted to use dladm anyway. > 2.2 Require drivers to implement an ioctl that prints out the names > of private properties. An advantage of such an ioctl would be > that it can be used by user-apps (including dladm) to get the > names of private props that the app can then iterate over, to > get to the value for each property. > > Cons: it would be aesthetically better to channel everything > through GLDv3 interfaces. > Having a way to enumerate properties does seem generally useful (though not just for ndd), and thus perhaps a special GLDv3 interface can be used. > Thoughts? Votes? Alternate ideas? > Have you considered the idea of having device drivers "register" their list of private properties with GLDv3? -- Garrett > --Sowmini > > > Previous Discussion: > -------------------- > On (02/08/08 12:11), Garrett D'Amore wrote: > : > >> 12) The more and more I think about it, the more I dislike the >> mac_ndd_handle separation. These should just be Brussels properties. >> Period. Having a separate mechanism for drivers to expose different >> sets of properties via NDD versus Brussels seems to add complexity and >> confusion, and I see no merit in it. That does mean that non-public >> properties are only ever interpreted as a string, but that's how NDD >> works today. What do you think? >> >> >> sowmini.varadhan at sun.com wrote: >> > : > >>> There's an additional level of optimization possible, that >>> I'm toying with. Since most ndd properties (not counting >>> the outliers such as those related to flow-control, or >>> those that have - and _ interchanged) are now public properties. >>> So, the mac layer can map the ndd name passed with the ND_SET/ND_GET >>> ioctl to a public property number, and just invoke the public >>> property itself. I've demo-ed this in the webrev for the "adv_autoneg_cap" >>> property (leaving out the rest for the sake of readability). >>> >>> Doing this mapping is cleaner, because a well-behaved driver >>> (one that has no outliers) will not need to register anything, >>> and can just gut out any ndd code it has today. >>> >>> But doing this mapping is also a little evil- if I were to write >>> a clean driver "foo" today, that only uses Brussels interfaces, >>> I would find that I could use both ndd and dladm to set the >>> MII props even though I have no intention of supporting ndd >>> configuration. In other words, this silently perpetuates the >>> usage of ndd where none is intended. So is this serious enough >>> to disqualify the ndd <-> "public property" mapping optimization? >>> >>> --Sowmini >>> >>> _______________________________________________ >>> brussels-dev mailing list >>> brussels-dev at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/brussels-dev >>> >>>
