[ovs-dev] [PATCH 1/3] OXM: Allow masking of IPv6 Flow Label

[Simon Horman]

On Thu, Jul 19, 2012 at 10:08:50AM +0900, Simon Horman wrote:
> On Wed, Jul 18, 2012 at 10:21:50AM -0700, Ben Pfaff wrote:
> > On Wed, Jul 18, 2012 at 12:02:19PM +0900, Simon Horman wrote:
> > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > 
> > When I apply this to master (currently 333be161abe78), I get the
> > following test failure.  It isn't immediately obvious to me why this
> > change should have an effect on dl_vlan_pcp, so I'll leave it to you
> > to take a look and let me know.
> 
> Thanks. I thought I had checked that all the tests pass.
> I'll look into this.

After much confusion on my part I believe that I have found the problem.

Expanding struct flow_wildcards causes the value hash returned
by flow_wildcards_hash() to change. This in turn causes the order
of flows in a NXST_FLOW_MONITOR reply (and probably elsewhere) to
be changed.

I applied the _hack_ shown at the end of this email to test this theory: it
should return the same hash as before the ipv6_label_mask element was sdded
to struct flow_wildcards. The result was that the test in question passes.

So witht his in mond I propose adding a hunk to this patch to
correct the test for the new hash in a new version of this patch.

Alternateively, perhaps the test could be reverted to the state
it was in before I recently expanded it. Perhaps that would lead to
a stable test.


diff --git a/lib/flow.c b/lib/flow.c
index 76f6b27..ef0d2f9 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -751,8 +751,11 @@ flow_wildcards_hash(const struct flow_wildcards *wc, uint32_t basis)
     /* If you change struct flow_wildcards and thereby trigger this
      * assertion, please check that the new struct flow_wildcards has no holes
      * in it before you update the assertion. */
+    uint8_t tmp[128];
     BUILD_ASSERT_DECL(sizeof *wc == 104 + FLOW_N_REGS * 4);
-    return hash_bytes(wc, sizeof *wc, basis);
+    memcpy(tmp, wc, 108);
+    memcpy((uint8_t *)tmp + 108, (uint8_t *)wc + 112, 20);
+    return hash_bytes(tmp, sizeof tmp, basis);
 }
 
 /* Returns true if 'a' and 'b' represent the same wildcards, false if they are