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. (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). >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. - 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? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev