[brussels-dev] Re: [driver-discuss] Re: [networking-discuss] Brussels high-level design document

[Raymond LI - Sun Microsystems - Beijing China]

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