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 > networking-discuss at opensolaris.org
