On 18 July 2014 05:19, Ben Pfaff <b...@nicira.com> wrote: > > > 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. >
I see. This case doesn't seem problematic to me, though. My broader aim with this code is to build a function that mimics the behaviour of translating into the sw_flow and back, without having to actually reconstruct a full key and mask. If userspace provides a shorter mask, then that shouldn't matter, so long as this function treats it the same as the sw_flow translation would. > - 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. Hmm. The README says that field ordering is not significant. I'm not sure about the latter concern, but it sounds like there are many aspects to this which I haven't considered yet. 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? > By the same metric, roughly 15% increase in flows supported. This is locking to write the cache back to the flow, but zero contention on that lock (I used an independent lock and a single revalidator thread for the test). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
