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.
"link_cksum_offload" controls tx/rx checksum enabling as well. Framework
seems not having this as tunable so far.
* 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.
* 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?
Again, thanks for considering all this, and for your hard work. We're
getting much closer to convergence than we were at the start of your
work, and I'm quite pleased by the progress.
-- Garrett
_______________________________________________
networking-discuss mailing list
[email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
