[ovs-dev] [PATCH v2 2/2] ofproto-dpif-upcall: Use flow_wildcards_has_extra().

[Jarno Rajahalme]

Update the comment in ukey_revalidate() to reflect the fact that the
mask in ukey is not the datapath mask, but the originally translated
flow wildcards.

Use flow_wildcards_has_extra() instead of open coding equivalent (but
different) functionality.  The old form and the code in
flow_wildcards_has_extra() ((dp | wc != dp) and (dp & wc != wc),
respecively) give the same result:

dp   wc    (dp | wc != dp)        (dp & wc != wc)
-------------------------------------------------------
0    0      (0 | 0 != 0) (false)   (0 & 0 != 0) (false)
0    1      (0 | 1 != 0) (true)    (0 & 1 != 1) (true)
1    0      (1 | 0 != 1) (false)   (1 & 0 != 0) (false)
1    1      (1 | 1 != 1) (false)   (1 & 1 != 1) (false)

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 8a43bbf..428258a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1769,15 +1769,13 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     struct netflow *netflow;
     struct ofproto_dpif *ofproto;
     struct dpif_flow_stats push;
-    struct flow flow, dp_mask;
-    struct flow_wildcards wc;
+    struct flow flow;
+    struct flow_wildcards dp_mask, wc;
     enum reval_result result;
-    uint64_t *dp64, *xout64;
     ofp_port_t ofp_in_port;
     struct xlate_in xin;
     long long int last_used;
     int error;
-    size_t i;
     bool need_revalidate;
 
     result = UKEY_DELETE;
@@ -1854,21 +1852,18 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     }
 
     if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, ukey->key,
-                             ukey->key_len, &dp_mask, &flow) == ODP_FIT_ERROR) {
+                             ukey->key_len, &dp_mask.masks, &flow)
+        == ODP_FIT_ERROR) {
         goto exit;
     }
 
-    /* Since the kernel is free to ignore wildcarded bits in the mask, we can't
-     * directly check that the masks are the same.  Instead we check that the
-     * mask in the kernel is more specific i.e. less wildcarded, than what
-     * we've calculated here.  This guarantees we don't catch any packets we
-     * shouldn't with the megaflow. */
-    dp64 = (uint64_t *) &dp_mask;
-    xout64 = (uint64_t *) &wc.masks;
-    for (i = 0; i < FLOW_U64S; i++) {
-        if ((dp64[i] | xout64[i]) != dp64[i]) {
-            goto exit;
-        }
+    /* Do not modify if any bit is wildcarded by the installed datapath flow,
+     * but not the newly revalidated wildcard mask (wc), i.e., if revalidation
+     * tells that the datapath flow is now too generic and must be narrowed
+     * down.  Note that we do not know if the datapath has ignored any of the
+     * wildcarded bits, so we may be overtly conservative here. */
+    if (flow_wildcards_has_extra(&dp_mask, &wc)) {
+        goto exit;
     }
 
     if (!ofpbuf_equal(odp_actions,
-- 
2.1.4