On Tue, Mar 17, 2015 at 03:59:59PM -0700, Ben Pfaff wrote: > On Mon, Mar 09, 2015 at 10:11:03AM +0900, Simon Horman wrote: > > This is intended as a usable demonstration of how > > the NTR selection method extension might may be used. > > > > NTR selection method > > Signed-off-by: Simon Horman <simon.hor...@netronome.com> > > I think that xlate_hash_fields_select_group() should only add a field > to the flow if the field is to be hashed *and* its prerequisites are > satisfied. (Probably should add the prerequisites to the hash too?)
Yes, I think that is reasonable (and the bracketed bit too). I'll see about making it so. > I think that xlate_hash_fields_select_group() probably wants to start > with an all-zeros flow? Currently it seems to start from the whole > input flow and then re-set all the fields back into it? But it seems > like it could just hash a field at a time for that matter; why does it > hash a whole flow? Again, yes on all counts. I think starting with a non-zero flow was a think-o on my part. And while hashing the entire flow may have made more sense in an earlier version of this patch it does not now. I changed things around to hash individual fields, entirely removing the need for a local flow, and it seems quite clean. > (The test could check for this, by hashing only one field, then > checking that only flows that vary on that field go to different > buckets.) Nice idea. > It's probably better if xlate_hash_fields_select_group() doesn't > discard the high 64 bits of the selection_method_param. Indeed that would be better. I have made it so. > I'd tend to use hash_bytes() instead of jhash_bytes(); any particular > reason for the latter? No reason. I have changed the code to use hash_bytes() as you suggest. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
