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

Reply via email to