Raymond LI - Sun Microsystems - Beijing China wrote:
Garrett, pls see my comments inline.
Garrett D'Amore wrote:
Sowmini,
Thanks for updating this proposal, I think we're nearing convergence
on what I'd like to see. ;-)
A few general thoughts:
* we still need to work out the details on a few items, including
kstat snapshot collection (you indicated it needs consideration), and
property registration.
* mac_list_private_props ... maybe exposing this as a property might
not be the best choice. its not clear. i'd love to see this idea
more fully explored though.
* I'd avoid making notes/comments about the STABILITY level of
individual properties at this point in the games. We should do that
just before the case goes to ARC.
* link_interrupt_coalescing, link_cksum_offload. As a writable
value, I don't think this makes sense, at least not initially. The
framework is supposed to auto-tune these for the user going forward,
so I'd prefer not to expose them. (As observable values, they may
have some merit, however.) (Right now a lot of drivers don't make
use of this, but this will change with crossbow, etc.)
For "link_interrupt_coalescing", though the GLD framework will
automaticly switch interrupt rates, we still have some tunables in
driver side. For example, bge uses "bge_rx_ticks_norm" and
"bge_rx_count_norm" as parameters when registering blank func to GLD,
which is now tunable. Another example is e1000g, which has
"intr_throttling_rate" tunable for customer. ixgb has "coalesce-num"
as a tunable.
But I find it is confusing and non-uniform across drivers to set this
tunable. bge has only "bge_rx_[ticks|count]_norm" as tunable. e1000g
has much more tunables for this feature. We might want Brussel to do
the job of a uniform interface across drivers.
No, we want to remove the tunables altogether. The only reason that
these are here, is because the framework support for adjusting the
throttling rate is inadequate, and the drivers have not all been
converted to use the coalescing feature of the framework. (In other
words, the uniform interface needs to be between the driver and the
framework. End-users should not be exposed to it.)
In the near term, if you want to expose something, I guess that's okay.
But I'd advise against documenting them too strongly... we will want to
remove them in the future, and end-users should not be relying on this
kind of tuning.
"link_cksum_offload" controls tx/rx checksum enabling as well.
Framework seems not having this as tunable so far.
Its because it doesn't need to be tunable. The framework negotiates
this feature with the drivers... it always enables the feature if the
driver can support it. There is never any reason to disable it.
* link_speed and link_duplex. It appears that you're
overriding/removing autonegotiation here. I'm uncomfortable with
this approach. First off, at 1G and higher, autonegotiation is
required by the standard. (You can however choose not to advertise
certain link/duplex combinations.) I'd prefer to have the
adv_cap_xxx and lp_cap_xxx values. (This also is very useful for
debug, because it lets one see what the link partner is doing.)
Agree. I think we should add that in.
I would consider gutting the ability to disable autonegotiation
altogether (I.e. you have to autoneg., but you can refuse to autoneg.
to anything other than a fixed speed/duplex setting... :-) This
requires coordination from all the interested parties though, because
some sites still refuse to support autonegotiation in their
infrastructure. (IMO, this is a sign of incompetence on the part of
the network administrators, but that is another rant...)
Having link_speed and link_duplex as *observable* values (i.e. the
result of autoneg.) is however useful. Please retain that
functionality.
* i'd like to see more public tunables. E.g. the lance_mode, ipg0,
ipg1, ipg2 values exposed by all NSN drivers.
I wonder if we could add more public properties in 2nd phase's scope.
In the first stage, it focuses on bge/e1000g/nge, which don't have
those tunables supported yet. It will be a problem for us to test
those "stub" func then.
You might also look at eri, which is now a GLDv3 driver. :-) I'd not
worry too much about nge, it has other issues. bge and e1000g are good
candidates though.
* we need to have observables for all the current kstats.
* for the near term, we might want to expose bcopy thresholds. A lot
of drivers use bcopy thresholds to determine when to switch between
D(V)MA and just plain copying. (Separate rx and tx thresholds are
needed, really.) Eventually I hope to move this logic to the common
Nemo layer, but that is a larger, and unfunded, project.
Agree. We could have this added.
* what about WLAN properties? It would be nice to consider the
current, separate WIFI configuration mechanism to see if we can
integrate it into Brussels. Maybe not initially, but certainly in
the mid- term. (Think SSID, link keys, etc.)
* what about loopback modes? I've mentioned this a few times. It
seems like setting/changing the loopback mode (used by SunVTS) could
easily be treated as another property for use with Brussels. I'd
like to eliminate the separate ioctls that are currently used to
handle this. Perhaps mention this for phase 2 or somesuch?
Would user set loopback mode for any application other than SunVTS?
Not that I'm aware of, though maybe another test application. But I'm
not entirely sure how relevant that question is ... I'm interested in
reducing/eliminating redundancy in the drivers. (I'd be perfectly happy
with a situation where the loopback tunables were not exposed to
userland applications through dladm/ndd, but were restricted to an ioctl
call. I just think that Nemo can do the ioctl remapping in just the
same way that it will for the ndd ioctls.
-- Garrett
_______________________________________________
networking-discuss mailing list
[email protected]
