* Bhavesh Davda ([email protected]) 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 _______________________________________________ Virtualization mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/virtualization
