From: Ben Pfaff <blp@nicira.com>
Date: Sat, 2 Aug 2014 00:22:20 +0000 (-0700)
Subject: ofproto-dpif: Use DPIF_FP_CREATE but not DPIF_FP_MODIFY.
X-Git-Tag: v2.3~5
X-Git-Url: http://git.cascardo.info/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=c54a83828e5f9798301e7ce01d46920f706a127c

ofproto-dpif: Use DPIF_FP_CREATE but not DPIF_FP_MODIFY.

A dpif reports EEXIST if a flow put operation that should create a new flow
instead attempts to modify an existing flow, or ENOENT if a flow put would
create a flow that overlaps some existing flow.  The latter does not always
indicate a bug in OVS userspace, because it can also mean that two
userspace OVS packet handler threads received packets that generated
the same megaflow for different microflows.  Until now, userspace has
logged this, which confuses users by making them suspect a bug.  We could
simply not log ENOENT in userspace, but that would suppress logging for
genuine bugs too.  Instead, this commit drops DPIF_FP_MODIFY from flow
put operations in ofproto-dpif, which transforms this particular
problem into EEXIST, which userspace already logs at "debug" level (see
flow_message_log_level()), effectively suppressing the logging in normal
circumstances.

It appears that in practice ofproto-dpif doesn't actually ever need to
modify flows in the datapath, only create and delete them, so this
shouldn't cause problems.

This backport squashes together commit a7d1bbdcfe4 (ofproto-dpif: Use
DPIF_FP_CREATE but not DPIF_FP_MODIFY.) and commit dc71c5dd0455
(ofproto-dpif: Fix tests broken by previous commit.) from master, with
resolved conflicts.

Suggested-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
---

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 9b8e896a0..8577b0e79 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -798,7 +798,7 @@ read_upcalls(struct handler *handler,
                  * in the kernel. */
                 VLOG_INFO_RL(&rl, "received packet on unassociated datapath "
                              "port %"PRIu32, odp_in_port);
-                dpif_flow_put(udpif->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+                dpif_flow_put(udpif->dpif, DPIF_FP_CREATE,
                               dupcall->key, dupcall->key_len, NULL, 0, NULL, 0,
                               NULL);
             }
@@ -1010,7 +1010,7 @@ handle_upcalls(struct handler *handler, struct hmap *misses,
 
             op = &ops[n_ops++];
             op->type = DPIF_OP_FLOW_PUT;
-            op->u.flow_put.flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
+            op->u.flow_put.flags = DPIF_FP_CREATE;
             op->u.flow_put.key = miss->key;
             op->u.flow_put.key_len = miss->key_len;
             op->u.flow_put.mask = ofpbuf_data(&mask);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d26aaf78c..46595f869 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -960,7 +960,7 @@ check_recirc(struct dpif_backer *backer)
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
-    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
                           ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
                           0, NULL);
     if (error && error != EEXIST) {
@@ -1088,7 +1088,7 @@ check_max_mpls_depth(struct dpif_backer *backer)
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
-        error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+        error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
                               ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL, 0, NULL);
         if (error && error != EEXIST) {
             if (error != EINVAL) {
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6b2b5e721..1fcd93773 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -13,7 +13,7 @@ m4_define([STRIP_XOUT], [[sed '
 ' | sort]])
 m4_define([FILTER_FLOW_INSTALL], [[
 grep ' put' | sed '
-    s/.*put\[create\]\[modify\] //
+    s/.*put\[create\] //
 ' | sort | uniq]])
 m4_define([FILTER_FLOW_DUMP], [[
 grep 'flow_dump ' | sed '