On Mon, Feb 18, 2013 at 12:13 AM, Rajahalme, Jarno (NSN - FI/Espoo) <jarno.rajaha...@nsn.com> wrote: > > On Feb 16, 2013, at 1:19 , ext Jesse Gross wrote: > >> On Mon, Feb 11, 2013 at 6:46 AM, Jarno Rajahalme >> <jarno.rajaha...@nsn.com> wrote: >>> Store key_len and hash fields at the end of struct sw_flow_key, past the >>> area being hashed. >>> Rename functions operating on keys from "_flow_" to "_flow_key_". >>> Shift the responsibility for key hashing from lookup and insert to key >>> construction side, which helps avaiding unnecessary hash computations. >>> >>> Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com> >> >> This seems like three independent changes, so it really should be >> broken up into different commits. However, that being said, it also >> needs more justification since I don't really see the benefit at the >> moment. In particular, how does moving the location of hash >> computation reduce unnecessary work? > > > I'd be happy to break it apart. > > In datapath.c ovs_flow_cmd_build_info(), there is first a call to > ovs_flow_tbl_lookup(), which currently computes the hash. In the case of a > new flow this is followed by ovs_flow_tbl_insert(), which currently > recomputes the same hash. This can be avoided if hash is made a part of the > key itself.
OK, I see now. In the numbers that you provided before, the test did not stress the kernel flow setup path (although the performance still seemed to improve after this patch, which seems odd to me). Do you know if this helps independently (and also how such a situation might arise)? > In a later patch (which may be more controversial), I have proposed to expose > the kernel computed key hash to the userspace (think "cookie"), which is > given back to the kernel only when the key is passed back as-is. In this case > additional key hash computations can be avoided in kernel flow set-up. My hope was that if this had a benefit on it's own then it could be applied independently. Otherwise, we'll have to wait to see what conclusion we come to on the rest of the patches. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
