Re: [Pv-drivers] [PATCH 2.6.31-rc9] net: VMware virtual Ethernet NIC driver: vmxnet3

[Chris Wright <chrisw@sous-sol.org>]

* Bhavesh Davda (bhavesh@vmware.com) wrote:
> > > Thanks a bunch for your really thorough review! I'll answer some of
> > your questions here. Shreyas can respond to your comments about some of
> > the coding style/comments/etc. in a separate mail.
> > 
> > The style is less important at this stage, but certainly eases review
> > to make it more consistent w/ Linux code.  The StudlyCaps, extra macros
> > (screaming caps) and inconistent space/tabs are visual distractions,
> > that's all.
> 
> Agreed, but we'll definitely address all the style issues in our subsequent patch posts. Actually Shreyas showed me his raw patch and it had tabs and not spaces, so we're trying to figure out if either Outlook (corporate blessed) or our Exchange server is converting those tabs to spaces or something.

Ah, that's always fun.  You can check by mailing to yourself and looking
at the outcome.

> > > We do have an internal prototype of a Linux vmxnet3 driver with 4 Tx
> > queues and 4 Rx queues, using 9 MSI-X vectors, but it needs some work
> > before calling it production ready.
> > 
> > I'd expect once you switch to alloc_etherdev_mq(), make napi work per
> > rx queue, and fix MSI-X allocation (all needed for 4/4), you should
> > have enough to support the max of 16/8 (IOW, 4/4 still sounds like an
> > aritificial limitation).
> 
> Absolutely: 4/4 was simply a prototype to see if it helps with performance any with certain benchmarks. So far it looks like there's a small performance gain with microbenchmarks like netperf, but we're hoping having multiple queues with multiple vectors might have some promise with macro benchmarks like SPECjbb. If it pans out, we'll most likely make it a module_param with some reasonable defaults, possibly just 1/1 by default.

Most physical devices that do MSI-X will do queue per cpu (or some
grouping if large number of cpus compared to queues).  Probably
reasonable default here too.

> > > > How about GRO conversion?
> > >
> > > Looks attractive, and we'll work on that in a subsequent patch.
> > Again, when we first wrote the driver, the NETIF_F_GRO stuff didn't
> > exist in Linux.
> > 
> > OK, shouldn't be too much work.
> > 
> > Another thing I forgot to mention is that net_device now has
> > net_device_stats in it.  So you shouldn't need net_device_stats in
> > vmxnet3_adapter.
> 
> Cool. Will do.
> 
> > > > > +#define UPT1_MAX_TX_QUEUES  64
> > > > > +#define UPT1_MAX_RX_QUEUES  64
> > > >
> > > > This is different than the 16/8 described above (and seemingly all
> > moot
> > > > since it becomes a single queue device).
> > >
> > > Nice catch! Those are not even used and are from the earliest days of
> > our driver development. We'll nuke those.
> > 
> > Could you describe the UPT layer a bit?  There were a number of
> > constants that didn't appear to be used.
> 
> UPT stands for Uniform Pass Thru, a spec/framework VMware developed with its IHV partners to implement the fast path (Tx/Rx) features of vmxnet3 in silicon. Some of these #defines that appear not to be used are based on this initial spec that VMware shared with its IHV partners.
> 
> We divided the emulated vmxnet3 PCIe device's registers into two sets on two separate BARs: BAR 0 for the UPT registers we asked IHV partners to implement that we emulate in our hypervisor if no physical device compliant with the UPT spec is available to pass thru from a virtual machine, and BAR 1 for registers we always emulate for slow path/control operations like setting the MAC address, or activating/quiescing/resetting the device, etc.

Interesting.  Sounds like part of NetQueue and also something that virtio
has looked into to support, e.g. VMDq.  Do you have a more complete
spec?

thanks,
-chris