ofproto: Add extra comments and checking for expiring a pending rule.

A given rule may only have one pending operation at a time, so when an
operation is pending we must not allow a flow expiration to be started on
that rule.

This doesn't fix a user-visible bug in ofproto-dpif because ofproto-dpif
always completes operations immediately, that is, no operations will be
pending when expiration runs.  (Technically there is a bug if the user
runs "ovs-appctl ofproto/clog", but that feature is for debugging only and
there is no reason for a user to ever run it.)

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2451d44d7d8c3ad3cb6e329b5e803e9733e43301..5265d7bc6e8c005ccf4514013c34a5daec4ac59c 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3562,6 +3562,11 @@ rule_expire(struct rule_dpif *rule)
     long long int now;
     uint8_t reason;
 
+    if (rule->up.pending) {
+        /* We'll have to expire it later. */
+        return;
+    }
+
     /* Has 'rule' expired? */
     now = time_msec();
     if (rule->up.hard_timeout

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index afd17a6019fa1e1e9733f0066fd5f9f3143ede15..f22c9f61da021e67f76f374a3442adc448e34ffd 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -388,6 +388,9 @@ struct ofproto_class {
      *   - Call ofproto_rule_expire() for each OpenFlow flow that has reached
      *     its hard_timeout or idle_timeout, to expire the flow.
      *
+     *     (But rules that are part of a pending operation, e.g. rules for
+     *     which ->pending is true, may not expire.)
+     *
      * Returns 0 if successful, otherwise a positive errno value. */
     int (*run)(struct ofproto *ofproto);
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 90cf343bb40c8c4f0af3a0eacc6b886dbd81fc8e..93401919e00d554a373fd4e1e20e1ea6704d9ff5 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3128,6 +3128,9 @@ ofproto_rule_update_used(struct rule *rule, long long int used)
  * OFPRR_HARD_TIMEOUT or OFPRR_IDLE_TIMEOUT), and then removes 'rule' from its
  * ofproto.
  *
+ * 'rule' must not have a pending operation (that is, 'rule->pending' must be
+ * NULL).
+ *
  * ofproto implementation ->run() functions should use this function to expire
  * OpenFlow flows. */
 void