On Tue, Aug 07, 2012 at 11:36:56PM -0400, Ed Maste wrote: > The original developer of the BSD port (Gaetano Catalli) also implemented a > threaded version of the userspace datapath to increase performance. As > with the original port, the threading work was done against Open vSwitch > 1.1.0pre2. Giuseppe Lettieri and I have ported it forward, and it's > functional and generally passes unit tests (with a few caveats). The > threaded datapath has shown about 10x improvement in throughput in some > tests, when compared to the current userspace datapath. > > There are still a few issues to work through, but I wanted to present the > patch now for discussion of the general approach. The threaded patches are > available in the bsd-port-threaded branch in this repo: > https://github.com/emaste/openvswitch/tree/bsd-port-threaded > > Or as a diff: > http://people.freebsd.org/~emaste/openvswitch/threaded.diff > > The patch introduces a configure option to enable threaded mode: > --enable-threaded, and the changes are largely confined to #ifdef THREADED > blocks. Two mutexes are added to dp_netdev: one for the flow table, and > one for the port list. (I've also added a mutex over most of lib/vlog.c, > although a less conservative approach may be possible.) > > Significant outstanding issues are: > 1. Buffer ownership and management isn't really correct right now, > apparently done due to a desire to avoid extra copies. This seems to be a > consequence of the standard BPF / libpcap interface. > 2. The patch needs a better method of ensuring flows aren't deleted while > there may be a reference in the datapath thread. > 3. The datapath thread waits in poll() with each port's file descriptor in > the fd array. However there's currently no fd to signal a bridge > reconfiguration, so the poll() has to timeout before the thread will pick > up the new config on the next time through its loop. > > In any case, I wanted to post the current patch for discussion and review > while working on the remaining issues. I'm very interested in your > thoughts and comments.
I always expected that we'd eventually need to add some threading to OVS, but I didn't expect to get it as an external contribution! I'm curious about the performance improvement. You mentioned a 10x performance improvement. How much did CPU usage increase as part of that? We already have users who complain when CPU usage spikes to 100%; I'm not sure that users will be happy if CPU usage can spike to 1000% (!). Oh I see, it looks like there is only one thread that does all packet processing? I had a notion that there would be many threads, for example on a per-datapath or per-port basis. It's very impressive getting 10x improvement with only a single additional thread. I looked at the repository and then at the diff, both very briefly. The repository looks more like development notes than something mergeable; the diff was easier to read. One early reaction to the code is that there are too many #ifdefs (mainly for locking and unlocking mutexes). I think that we should be able to remove many of them with a few well-chosen macros. Some of the code has style issues, but this isn't the stage to point those out. Some OVS library functions have thread safety issues. Either the datapath threads need to be carefully written to avoid those issues (I didn't check whether that's already true), or we need to audit the OVS library to find and fix those. OVS recently got a library, lib/worker.c, for implementing code in a "worker process". So far, we're using that in a single-worker-process model, mostly for asynchronous logging, but we could generalize it so that there could be a second worker process used for datapath implementation. If this would achieve acceptable performance, then it would avoid the difficulties of threads. I guess the question there is how much communication a process model would require versus a thread model. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
