On Thu, Jul 17, 2014 at 11:04:55AM +1200, Joe Stringer wrote: > On 17 July 2014 04:48, Ben Pfaff <b...@nicira.com> wrote: > > > On Wed, Jul 16, 2014 at 02:50:12PM +1200, Joe Stringer wrote: > > > Converting the flow key and mask back into netlink format during flow > > > dump is fairly expensive. By caching the userspace-provided versions of > > > these during flow setup, and copying the memory directly during flow > > > dump, we are able to support up to 33% more flows in the datapath. > > > > Caching the userspace-provided flow key and mask could violate the > > guarantee given at the end of datapath/README: > > > > - When the kernel sends a given flow key to userspace, it always > > composes it the same way. This allows userspace to hash and > > compare entire flow keys that it may not be able to fully > > interpret. > > Hmm. The way I read this is that the netlink format of a particular flow > key should always be the same, no matter how it's serialised in the kernel.
Right... > (For completeness, I mention that the case where userspace and the kernel > agrees is uninteresting, because the netlink format of the key should be > the same). Yes. > From datapath/README: > > - If the kernel's flow key includes more fields than the userspace > version of the flow key, for example if the kernel decoded IPv6 > headers but userspace stopped at the Ethernet type (because it > does not understand IPv6), then again nothing special is > necessary. Userspace can still set up a flow in the usual way, > as long as it uses the kernel-provided flow key to do it. > > In this case, we would expect to see that the flow key in the flow_put > message is the same as the key composed during the upcall process. Right--if userspace takes this advice. This advice predates megaflows, though. I don't know what we do these days in such a case with megaflows. Maybe we generate a flow match in userspace that only includes the fields that userspace cares about. > - If the userspace flow key includes more fields than the > kernel's, for example if userspace decoded an IPv6 header but > the kernel stopped at the Ethernet type, then userspace can > forward the packet manually, without setting up a flow in the > kernel. This case is bad for performance because every packet > that the kernel considers part of the flow must go to userspace, > but the forwarding behavior is correct. (If userspace can > determine that the values of the extra fields would not affect > forwarding behavior, then it could set up a flow anyway.) > > Looking further in the README, the patch doesn't handle this case > appropriately. If the interpretation of that last part of the README is > valid, then we could look at ways to fix this. For instance, after copying > the key, do a pass over it looking for attributes we don't understand, and > remove them. Alternatively we could skip caching in this case. > > > Does this address your concerns adequately? Not entirely. It still means that kernel-generated and userspace-generated flow keys could, for example, order the fields differently, or accidentally fill some ignored padding fields with different bytes. Basically I'd be much more comfortable with the kernel generating its own flow key the first time it dumps a flow, then caching that. How much overhead does that have versus caching the userspace-provided flow key? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
