Merge branch 'mainstream'
authorGiuseppe Lettieri <g.lettieri@iet.unipi.it>
Sun, 15 Sep 2013 07:54:08 +0000 (09:54 +0200)
committerGiuseppe Lettieri <g.lettieri@iet.unipi.it>
Sun, 15 Sep 2013 07:54:08 +0000 (09:54 +0200)
40 files changed:
AUTHORS
FAQ
NEWS
acinclude.m4
datapath/linux/compat/include/linux/skbuff.h
include/openflow/openflow-1.2.h
lib/automake.mk
lib/classifier.c
lib/classifier.h
lib/dpif-linux.c
lib/dynamic-string.c
lib/dynamic-string.h
lib/guarded-list.c [new file with mode: 0644]
lib/guarded-list.h [new file with mode: 0644]
lib/ofp-actions.c
lib/ofp-actions.h
lib/ofp-parse.c
lib/ofp-util.c
lib/table.c
lib/timeval.c
lib/timeval.h
lib/vlog.c
lib/vlog.h
ofproto/connmgr.c
ofproto/connmgr.h
ofproto/fail-open.c
ofproto/ofproto-dpif-upcall.c
ofproto/ofproto-dpif-upcall.h
ofproto/ofproto-dpif-xlate.c
ofproto/ofproto-dpif.c
ofproto/ofproto-dpif.h
ofproto/ofproto-provider.h
ofproto/ofproto.c
ovsdb/ovsdb-tool.c
python/ovs/vlog.py
tests/ofproto.at
tests/vlog.at
utilities/ovs-appctl.8.in
utilities/ovs-ofctl.c
vswitchd/bridge.c

diff --git a/AUTHORS b/AUTHORS
index 7642aee..63c1ef8 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -215,6 +215,7 @@ Stephen Hemminger       shemminger@vyatta.com
 Takayuki HAMA           t-hama@cb.jp.nec.com
 Teemu Koponen           koponen@nicira.com
 Timothy Chen            tchen@nicira.com
+Torbjorn Tornkvist      kruskakli@gmail.com
 Valentin Bud            valentin@hackaserver.com
 Vishal Swarankar        vishal.swarnkar@gmail.com
 Vjekoslav Brajkovic     balkan@cs.washington.edu
@@ -223,6 +224,7 @@ YAMAMOTO Takashi        yamamoto@valinux.co.jp
 Yeming Zhao             zhaoyeming@gmail.com
 Ying Chen               yingchen@vmware.com
 Yongqiang Liu           liuyq7809@gmail.com
+ankur dwivedi           ankurengg2003@gmail.com
 kk yap                  yapkke@stanford.edu
 likunyun                kunyunli@hotmail.com
 rahim entezari          rahim.entezari@gmail.com
diff --git a/FAQ b/FAQ
index 20a3e3b..5744d5a 100644 (file)
--- a/FAQ
+++ b/FAQ
@@ -148,7 +148,7 @@ A: The following table lists the Linux kernel versions against which the
        1.9.x      2.6.18 to 3.8
        1.10.x     2.6.18 to 3.8
        1.11.x     2.6.18 to 3.8
-       2.x        2.6.32 to 3.10
+       2.0.x      2.6.32 to 3.10
 
    Open vSwitch userspace should also work with the Linux kernel module
    built into Linux 3.3 and later.
diff --git a/NEWS b/NEWS
index 09c98eb..4dd9568 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v2.0.0
 ---------------------
+    - Log files now report times with millisecond resolution.  (Previous
+      versions only reported whole seconds.)
 
 
 v2.0.0 - xx xxx xxxx
index 071fe54..c293d33 100644 (file)
@@ -240,8 +240,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   # quoting rules.
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [[[^@]]proto_data_valid],
                   [OVS_DEFINE([HAVE_PROTO_DATA_VALID])])
-  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [raw],
-                  [OVS_DEFINE([HAVE_MAC_RAW])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_dst(],
                   [OVS_DEFINE([HAVE_SKB_DST_ACCESSOR_FUNCS])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], 
index a486096..9868a98 100644 (file)
@@ -95,12 +95,6 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
 #define CHECKSUM_COMPLETE CHECKSUM_HW
 #endif
 
-#ifdef HAVE_MAC_RAW
-#define mac_header mac.raw
-#define network_header nh.raw
-#define transport_header h.raw
-#endif
-
 #ifndef HAVE_SKBUFF_HEADER_HELPERS
 static inline unsigned char *skb_transport_header(const struct sk_buff *skb)
 {
index d1e42a4..541b143 100644 (file)
@@ -109,18 +109,16 @@ enum oxm12_ofb_match_fields {
     OFPXMT12_OFB_IPV6_ND_TLL,    /* Target link-layer for ND. */
     OFPXMT12_OFB_MPLS_LABEL,     /* MPLS label. */
     OFPXMT12_OFB_MPLS_TC,        /* MPLS TC. */
-    /* Following added in OpenFlow 1.3 */
-    OFPXMT12_OFB_MPLS_BOS,       /* MPLS BoS bit. */
-    OFPXMT12_OFB_PBB_ISID,       /* PBB I-SID. */
-    OFPXMT12_OFB_TUNNEL_ID,      /* Logical Port Metadata */
-    OFPXMT12_OFB_IPV6_EXTHDR,    /* IPv6 Extension Header pseudo-field */
+#define OFPXMT12_MASK ((1ULL << (OFPXMT12_OFB_MPLS_TC + 1)) - 1)
 
-    /* End Marker */
-    OFPXMT12_OFB_MAX,
+    /* Following added in OpenFlow 1.3 */
+    OFPXMT13_OFB_MPLS_BOS,       /* MPLS BoS bit. */
+    OFPXMT13_OFB_PBB_ISID,       /* PBB I-SID. */
+    OFPXMT13_OFB_TUNNEL_ID,      /* Logical Port Metadata */
+    OFPXMT13_OFB_IPV6_EXTHDR,    /* IPv6 Extension Header pseudo-field */
+#define OFPXMT13_MASK ((1ULL << (OFPXMT13_OFB_IPV6_EXTHDR + 1)) - 1)
 };
 
-#define OFPXMT12_MASK ((1ULL << OFPXMT12_OFB_MAX) - 1)
-
 /* OXM implementation makes use of NXM as they are the same format
  * with different field definitions
  */
@@ -180,13 +178,13 @@ enum oxm12_ofb_match_fields {
 #define OXM_OF_IPV6_ND_TLL    OXM_HEADER   (OFPXMT12_OFB_IPV6_ND_TLL, 6)
 #define OXM_OF_MPLS_LABEL     OXM_HEADER   (OFPXMT12_OFB_MPLS_LABEL, 4)
 #define OXM_OF_MPLS_TC        OXM_HEADER   (OFPXMT12_OFB_MPLS_TC, 1)
-#define OXM_OF_MPLS_BOS       OXM_HEADER   (OFPXMT12_OFB_MPLS_BOS, 1)
+#define OXM_OF_MPLS_BOS       OXM_HEADER   (OFPXMT13_OFB_MPLS_BOS, 1)
 #define OXM_OF_PBB_ISID       OXM_HEADER   (OFPXMT12_OFB_PBB_ISID, 4)
 #define OXM_OF_PBB_ISID_W     OXM_HEADER_W (OFPXMT12_OFB_PBB_ISID, 4)
-#define OXM_OF_TUNNEL_ID      OXM_HEADER   (OFPXMT12_OFB_TUNNEL_ID, 8)
-#define OXM_OF_TUNNEL_ID_W    OXM_HEADER_W (OFPXMT12_OFB_TUNNEL_ID, 8)
-#define OXM_OF_IPV6_EXTHDR    OXM_HEADER   (OFPXMT12_OFB_IPV6_EXTHDR, 2)
-#define OXM_OF_IPV6_EXTHDR_W  OXM_HEADER_W (OFPXMT12_OFB_IPV6_EXTHDR, 2)
+#define OXM_OF_TUNNEL_ID      OXM_HEADER   (OFPXMT13_OFB_TUNNEL_ID, 8)
+#define OXM_OF_TUNNEL_ID_W    OXM_HEADER_W (OFPXMT13_OFB_TUNNEL_ID, 8)
+#define OXM_OF_IPV6_EXTHDR    OXM_HEADER   (OFPXMT13_OFB_IPV6_EXTHDR, 2)
+#define OXM_OF_IPV6_EXTHDR_W  OXM_HEADER_W (OFPXMT13_OFB_IPV6_EXTHDR, 2)
 
 /* The VLAN id is 12-bits, so we can use the entire 16 bits to indicate
  * special conditions.
index fce8423..b2d6dc2 100644 (file)
@@ -58,6 +58,8 @@ lib_libopenvswitch_a_SOURCES = \
        lib/fatal-signal.h \
        lib/flow.c \
        lib/flow.h \
+       lib/guarded-list.c \
+       lib/guarded-list.h \
        lib/hash.c \
        lib/hash.h \
        lib/hindex.c \
index 93ee977..36eb1f0 100644 (file)
@@ -500,8 +500,9 @@ cls_cursor_first(struct cls_cursor *cursor)
 /* Returns the next matching cls_rule in 'cursor''s iteration, or a null
  * pointer if there are no more matches. */
 struct cls_rule *
-cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *rule)
+cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *rule_)
 {
+    struct cls_rule *rule = CONST_CAST(struct cls_rule *, rule_);
     const struct cls_table *table;
     struct cls_rule *next;
 
index 5a45458..a795b4a 100644 (file)
 extern "C" {
 #endif
 
+/* Needed only for the lock annotation in struct classifier. */
+extern struct ovs_mutex ofproto_mutex;
+
 /* A flow classifier. */
 struct classifier {
     int n_rules;                /* Total number of rules. */
     struct hmap tables;         /* Contains "struct cls_table"s.  */
     struct list tables_priority; /* Tables in descending priority order */
-    struct ovs_rwlock rwlock;
+    struct ovs_rwlock rwlock OVS_ACQ_AFTER(ofproto_mutex);
 };
 
 /* A set of rules that all have the same fields wildcarded. */
@@ -140,7 +143,7 @@ struct cls_cursor {
 void cls_cursor_init(struct cls_cursor *cursor, const struct classifier *cls,
                      const struct cls_rule *match) OVS_REQ_RDLOCK(cls->rwlock);
 struct cls_rule *cls_cursor_first(struct cls_cursor *cursor);
-struct cls_rule *cls_cursor_next(struct cls_cursor *cursor, struct cls_rule *);
+struct cls_rule *cls_cursor_next(struct cls_cursor *cursor, const struct cls_rule *);
 
 #define CLS_CURSOR_FOR_EACH(RULE, MEMBER, CURSOR)                       \
     for (ASSIGN_CONTAINER(RULE, cls_cursor_first(CURSOR), MEMBER);      \
index e2300d6..6f75f57 100644 (file)
@@ -706,7 +706,7 @@ dpif_linux_port_dump_start(const struct dpif *dpif_, void **statep)
     *statep = state = xmalloc(sizeof *state);
 
     dpif_linux_vport_init(&request);
-    request.cmd = OVS_DP_CMD_GET;
+    request.cmd = OVS_VPORT_CMD_GET;
     request.dp_ifindex = dpif->dp_ifindex;
 
     buf = ofpbuf_new(1024);
@@ -963,7 +963,7 @@ dpif_linux_flow_dump_start(const struct dpif *dpif_, void **statep)
     *statep = state = xmalloc(sizeof *state);
 
     dpif_linux_flow_init(&request);
-    request.cmd = OVS_DP_CMD_GET;
+    request.cmd = OVS_FLOW_CMD_GET;
     request.dp_ifindex = dpif->dp_ifindex;
 
     buf = ofpbuf_new(1024);
index 9b3e7ba..5137d9f 100644 (file)
@@ -183,21 +183,24 @@ ds_put_printable(struct ds *ds, const char *s, size_t n)
     }
 }
 
-/* Writes time 'when' to 'string' based on 'template', in local time or UTC
- * based on 'utc'. */
+/* Writes the current time with optional millisecond resolution to 'string'
+ * based on 'template'.
+ * The current time is either localtime or UTC based on 'utc'. */
 void
-ds_put_strftime(struct ds *ds, const char *template, time_t when, bool utc)
+ds_put_strftime_msec(struct ds *ds, const char *template, long long int when,
+                     bool utc)
 {
-    struct tm tm;
+    struct tm_msec tm;
     if (utc) {
-        gmtime_r(&when, &tm);
+        gmtime_msec(when, &tm);
     } else {
-        localtime_r(&when, &tm);
+        localtime_msec(when, &tm);
     }
 
     for (;;) {
         size_t avail = ds->string ? ds->allocated - ds->length + 1 : 0;
-        size_t used = strftime(&ds->string[ds->length], avail, template, &tm);
+        size_t used = strftime_msec(&ds->string[ds->length], avail, template,
+                                    &tm);
         if (used) {
             ds->length += used;
             return;
@@ -209,12 +212,12 @@ ds_put_strftime(struct ds *ds, const char *template, time_t when, bool utc)
 /* Returns a malloc()'d string for time 'when' based on 'template', in local
  * time or UTC based on 'utc'. */
 char *
-xastrftime(const char *template, time_t when, bool utc)
+xastrftime_msec(const char *template, long long int when, bool utc)
 {
     struct ds s;
 
     ds_init(&s);
-    ds_put_strftime(&s, template, when, utc);
+    ds_put_strftime_msec(&s, template, when, utc);
     return s.string;
 }
 
index c069586..2272343 100644 (file)
@@ -61,10 +61,9 @@ int ds_get_line(struct ds *, FILE *);
 int ds_get_preprocessed_line(struct ds *, FILE *, int *line_number);
 int ds_get_test_line(struct ds *, FILE *);
 
-void ds_put_strftime(struct ds *, const char *template, time_t when, bool utc)
-    STRFTIME_FORMAT(2);
-char *xastrftime(const char *template, time_t when, bool utc)
-    STRFTIME_FORMAT(1);
+void ds_put_strftime_msec(struct ds *, const char *template, long long int when,
+                         bool utc);
+char *xastrftime_msec(const char *template, long long int when, bool utc);
 
 char *ds_cstr(struct ds *);
 const char *ds_cstr_ro(const struct ds *);
diff --git a/lib/guarded-list.c b/lib/guarded-list.c
new file mode 100644 (file)
index 0000000..cbb2030
--- /dev/null
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2013 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "guarded-list.h"
+
+void
+guarded_list_init(struct guarded_list *list)
+{
+    ovs_mutex_init(&list->mutex);
+    list_init(&list->list);
+    list->n = 0;
+}
+
+void
+guarded_list_destroy(struct guarded_list *list)
+{
+    ovs_mutex_destroy(&list->mutex);
+}
+
+bool
+guarded_list_is_empty(const struct guarded_list *list)
+{
+    bool empty;
+
+    ovs_mutex_lock(&list->mutex);
+    empty = list->n == 0;
+    ovs_mutex_unlock(&list->mutex);
+
+    return empty;
+}
+
+/* If 'list' has fewer than 'max' elements, adds 'node' at the end of the list
+ * and returns the number of elements now on the list.
+ *
+ * If 'list' already has at least 'max' elements, returns 0 without modifying
+ * the list. */
+size_t
+guarded_list_push_back(struct guarded_list *list,
+                       struct list *node, size_t max)
+{
+    size_t retval = 0;
+
+    ovs_mutex_lock(&list->mutex);
+    if (list->n < max) {
+        list_push_back(&list->list, node);
+        retval = ++list->n;
+    }
+    ovs_mutex_unlock(&list->mutex);
+
+    return retval;
+}
+
+struct list *
+guarded_list_pop_front(struct guarded_list *list)
+{
+    struct list *node = NULL;
+
+    ovs_mutex_lock(&list->mutex);
+    if (list->n) {
+        node = list_pop_front(&list->list);
+        list->n--;
+    }
+    ovs_mutex_unlock(&list->mutex);
+
+    return node;
+}
+
+size_t
+guarded_list_pop_all(struct guarded_list *list, struct list *elements)
+{
+    size_t n;
+
+    ovs_mutex_lock(&list->mutex);
+    list_move(elements, &list->list);
+    n = list->n;
+
+    list_init(&list->list);
+    list->n = 0;
+    ovs_mutex_unlock(&list->mutex);
+
+    return n;
+}
diff --git a/lib/guarded-list.h b/lib/guarded-list.h
new file mode 100644 (file)
index 0000000..625865d
--- /dev/null
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2013 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef GUARDED_LIST_H
+#define GUARDED_LIST_H 1
+
+#include <stddef.h>
+#include "compiler.h"
+#include "list.h"
+#include "ovs-thread.h"
+
+struct guarded_list {
+    struct ovs_mutex mutex;
+    struct list list;
+    size_t n;
+};
+
+void guarded_list_init(struct guarded_list *);
+void guarded_list_destroy(struct guarded_list *);
+
+bool guarded_list_is_empty(const struct guarded_list *);
+
+size_t guarded_list_push_back(struct guarded_list *, struct list *,
+                              size_t max);
+struct list *guarded_list_pop_front(struct guarded_list *);
+size_t guarded_list_pop_all(struct guarded_list *, struct list *);
+
+#endif /* guarded-list.h */
index 77aa69c..54df17f 100644 (file)
@@ -2117,6 +2117,30 @@ ofpacts_equal(const struct ofpact *a, size_t a_len,
 {
     return a_len == b_len && !memcmp(a, b, a_len);
 }
+
+/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
+ * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
+ *
+ * This function relies on the order of 'ofpacts' being correct (as checked by
+ * ofpacts_verify()). */
+uint32_t
+ofpacts_get_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
+{
+    const struct ofpact *a;
+
+    OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+        enum ovs_instruction_type inst;
+
+        inst = ovs_instruction_type_from_ofpact_type(a->type);
+        if (a->type == OFPACT_METER) {
+            return ofpact_get_METER(a)->meter_id;
+        } else if (inst > OVSINST_OFPIT13_METER) {
+            break;
+        }
+    }
+
+    return 0;
+}
 \f
 /* Formatting ofpacts. */
 
index a3fb60f..0876ed7 100644 (file)
@@ -530,6 +530,7 @@ bool ofpacts_output_to_group(const struct ofpact[], size_t ofpacts_len,
                              uint32_t group_id);
 bool ofpacts_equal(const struct ofpact a[], size_t a_len,
                    const struct ofpact b[], size_t b_len);
+uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len);
 
 /* Formatting ofpacts.
  *
index a61beb9..e28bf02 100644 (file)
@@ -1370,7 +1370,7 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
     char *name;
 
     /* Meters require at least OF 1.3. */
-    *usable_protocols &= OFPUTIL_P_OF13_UP;
+    *usable_protocols = OFPUTIL_P_OF13_UP;
 
     switch (command) {
     case -1:
index 23c7136..9edfe9e 100644 (file)
@@ -4010,6 +4010,19 @@ ofputil_put_ofp11_table_stats(const struct ofp12_table_stats *in,
     out->matched_count = in->matched_count;
 }
 
+static void
+ofputil_put_ofp12_table_stats(const struct ofp12_table_stats *in,
+                              struct ofpbuf *buf)
+{
+    struct ofp12_table_stats *out = ofpbuf_put(buf, in, sizeof *in);
+
+    /* Trim off OF1.3-only capabilities. */
+    out->match &= htonll(OFPXMT12_MASK);
+    out->wildcards &= htonll(OFPXMT12_MASK);
+    out->write_setfields &= htonll(OFPXMT12_MASK);
+    out->apply_setfields &= htonll(OFPXMT12_MASK);
+}
+
 static void
 ofputil_put_ofp13_table_stats(const struct ofp12_table_stats *in,
                               struct ofpbuf *buf)
@@ -4035,31 +4048,27 @@ ofputil_encode_table_stats_reply(const struct ofp12_table_stats stats[], int n,
 
     reply = ofpraw_alloc_stats_reply(request, n * sizeof *stats);
 
-    switch ((enum ofp_version) request->version) {
-    case OFP10_VERSION:
-        for (i = 0; i < n; i++) {
+    for (i = 0; i < n; i++) {
+        switch ((enum ofp_version) request->version) {
+        case OFP10_VERSION:
             ofputil_put_ofp10_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    case OFP11_VERSION:
-        for (i = 0; i < n; i++) {
+        case OFP11_VERSION:
             ofputil_put_ofp11_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    case OFP12_VERSION:
-        ofpbuf_put(reply, stats, n * sizeof *stats);
-        break;
+        case OFP12_VERSION:
+            ofputil_put_ofp12_table_stats(&stats[i], reply);
+            break;
 
-    case OFP13_VERSION:
-        for (i = 0; i < n; i++) {
+        case OFP13_VERSION:
             ofputil_put_ofp13_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    default:
-        NOT_REACHED();
+        default:
+            NOT_REACHED();
+        }
     }
 
     return reply;
index 21f4905..4628170 100644 (file)
@@ -221,7 +221,7 @@ table_print_table_line__(struct ds *line)
 static char *
 table_format_timestamp__(void)
 {
-    return xastrftime("%Y-%m-%d %H:%M:%S", time_wall(), true);
+    return xastrftime_msec("%Y-%m-%d %H:%M:%S.###", time_wall_msec(), true);
 }
 
 static void
index 3262397..223ed30 100644 (file)
@@ -42,12 +42,12 @@ VLOG_DEFINE_THIS_MODULE(timeval);
 struct clock {
     clockid_t id;               /* CLOCK_MONOTONIC or CLOCK_REALTIME. */
 
-    /* Features for use by unit tests.  Protected by 'rwlock'. */
-    struct ovs_rwlock rwlock;
-    struct timespec warp;       /* Offset added for unit tests. */
-    bool stopped;               /* Disables real-time updates if true.  */
-
-    struct timespec cache;      /* Last time read from kernel. */
+    /* Features for use by unit tests.  Protected by 'mutex'. */
+    struct ovs_mutex mutex;
+    atomic_bool slow_path;             /* True if warped or stopped. */
+    struct timespec warp OVS_GUARDED;  /* Offset added for unit tests. */
+    bool stopped OVS_GUARDED;          /* Disable real-time updates if true. */
+    struct timespec cache OVS_GUARDED; /* Last time read from kernel. */
 };
 
 /* Our clocks. */
@@ -76,7 +76,8 @@ init_clock(struct clock *c, clockid_t id)
 {
     memset(c, 0, sizeof *c);
     c->id = id;
-    ovs_rwlock_init(&c->rwlock);
+    ovs_mutex_init(&c->mutex);
+    atomic_init(&c->slow_path, false);
     xclock_gettime(c->id, &c->cache);
 }
 
@@ -105,14 +106,28 @@ time_init(void)
 static void
 time_timespec__(struct clock *c, struct timespec *ts)
 {
+    bool slow_path;
+
     time_init();
 
-    if (!c->stopped) {
+    atomic_read_explicit(&c->slow_path, &slow_path, memory_order_relaxed);
+    if (!slow_path) {
         xclock_gettime(c->id, ts);
     } else {
-        ovs_rwlock_rdlock(&c->rwlock);
-        timespec_add(ts, &c->cache, &c->warp);
-        ovs_rwlock_unlock(&c->rwlock);
+        struct timespec warp;
+        struct timespec cache;
+        bool stopped;
+
+        ovs_mutex_lock(&c->mutex);
+        stopped = c->stopped;
+        warp = c->warp;
+        cache = c->cache;
+        ovs_mutex_unlock(&c->mutex);
+
+        if (!stopped) {
+            xclock_gettime(c->id, &cache);
+        }
+        timespec_add(ts, &cache, &warp);
     }
 }
 
@@ -320,14 +335,24 @@ timespec_add(struct timespec *sum,
     *sum = tmp;
 }
 
+static bool
+is_warped(const struct clock *c)
+{
+    bool warped;
+
+    ovs_mutex_lock(&c->mutex);
+    warped = monotonic_clock.warp.tv_sec || monotonic_clock.warp.tv_nsec;
+    ovs_mutex_unlock(&c->mutex);
+
+    return warped;
+}
+
 static void
 log_poll_interval(long long int last_wakeup)
 {
     long long int interval = time_msec() - last_wakeup;
 
-    if (interval >= 1000
-        && !monotonic_clock.warp.tv_sec
-        && !monotonic_clock.warp.tv_nsec) {
+    if (interval >= 1000 && !is_warped(&monotonic_clock)) {
         const struct rusage *last_rusage = get_recent_rusage();
         struct rusage rusage;
 
@@ -451,10 +476,11 @@ timeval_stop_cb(struct unixctl_conn *conn,
                  int argc OVS_UNUSED, const char *argv[] OVS_UNUSED,
                  void *aux OVS_UNUSED)
 {
-    ovs_rwlock_wrlock(&monotonic_clock.rwlock);
+    ovs_mutex_lock(&monotonic_clock.mutex);
+    atomic_store(&monotonic_clock.slow_path, true);
     monotonic_clock.stopped = true;
     xclock_gettime(monotonic_clock.id, &monotonic_clock.cache);
-    ovs_rwlock_unlock(&monotonic_clock.rwlock);
+    ovs_mutex_unlock(&monotonic_clock.mutex);
 
     unixctl_command_reply(conn, NULL);
 }
@@ -480,9 +506,10 @@ timeval_warp_cb(struct unixctl_conn *conn,
     ts.tv_sec = msecs / 1000;
     ts.tv_nsec = (msecs % 1000) * 1000 * 1000;
 
-    ovs_rwlock_wrlock(&monotonic_clock.rwlock);
+    ovs_mutex_lock(&monotonic_clock.mutex);
+    atomic_store(&monotonic_clock.slow_path, true);
     timespec_add(&monotonic_clock.warp, &monotonic_clock.warp, &ts);
-    ovs_rwlock_unlock(&monotonic_clock.rwlock);
+    ovs_mutex_unlock(&monotonic_clock.mutex);
 
     unixctl_command_reply(conn, "warped");
 }
@@ -494,3 +521,49 @@ timeval_dummy_register(void)
     unixctl_command_register("time/warp", "MSECS", 1, 1,
                              timeval_warp_cb, NULL);
 }
+
+
+
+/* strftime() with an extension for high-resolution timestamps.  Any '#'s in
+ * 'format' will be replaced by subseconds, e.g. use "%S.###" to obtain results
+ * like "01.123".  */
+size_t
+strftime_msec(char *s, size_t max, const char *format,
+              const struct tm_msec *tm)
+{
+    size_t n;
+
+    n = strftime(s, max, format, &tm->tm);
+    if (n) {
+        char decimals[4];
+        char *p;
+
+        sprintf(decimals, "%03d", tm->msec);
+        for (p = strchr(s, '#'); p; p = strchr(p, '#')) {
+            char *d = decimals;
+            while (*p == '#')  {
+                *p++ = *d ? *d++ : '0';
+            }
+        }
+    }
+
+    return n;
+}
+
+struct tm_msec *
+localtime_msec(long long int now, struct tm_msec *result)
+{
+  time_t now_sec = now / 1000;
+  localtime_r(&now_sec, &result->tm);
+  result->msec = now % 1000;
+  return result;
+}
+
+struct tm_msec *
+gmtime_msec(long long int now, struct tm_msec *result)
+{
+  time_t now_sec = now / 1000;
+  gmtime_r(&now_sec, &result->tm);
+  result->msec = now % 1000;
+  return result;
+}
index d0962ee..99b3af0 100644 (file)
@@ -40,6 +40,11 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t));
 #define TIME_MAX TYPE_MAXIMUM(time_t)
 #define TIME_MIN TYPE_MINIMUM(time_t)
 
+struct tm_msec {
+  struct tm tm;
+  int msec;
+};
+
 time_t time_now(void);
 time_t time_wall(void);
 long long int time_msec(void);
@@ -53,6 +58,10 @@ int time_poll(struct pollfd *, int n_pollfds, long long int timeout_when,
 long long int timespec_to_msec(const struct timespec *);
 long long int timeval_to_msec(const struct timeval *);
 
+struct tm_msec *localtime_msec(long long int now, struct tm_msec *result);
+struct tm_msec *gmtime_msec(long long int now, struct tm_msec *result);
+size_t strftime_msec(char *s, size_t max, const char *format,
+                     const struct tm_msec *);
 void xgettimeofday(struct timeval *);
 void xclock_gettime(clock_t, struct timespec *);
 
index a267112..37806b8 100644 (file)
@@ -583,7 +583,7 @@ static void
 vlog_init__(void)
 {
     static char *program_name_copy;
-    time_t now;
+    long long int now;
 
     /* openlog() is allowed to keep the pointer passed in, without making a
      * copy.  The daemonize code sometimes frees and replaces 'program_name',
@@ -593,10 +593,10 @@ vlog_init__(void)
     program_name_copy = program_name ? xstrdup(program_name) : NULL;
     openlog(program_name_copy, LOG_NDELAY, LOG_DAEMON);
 
-    now = time_wall();
+    now = time_wall_msec();
     if (now < 0) {
-        char *s = xastrftime("%a, %d %b %Y %H:%M:%S", now, true);
-        VLOG_ERR("current time is negative: %s (%ld)", s, (long int) now);
+        char *s = xastrftime_msec("%a, %d %b %Y %H:%M:%S", now, true);
+        VLOG_ERR("current time is negative: %s (%lld)", s, now);
         free(s);
     }
 
@@ -746,12 +746,12 @@ format_log_message(const struct vlog_module *module, enum vlog_level level,
             ds_put_cstr(s, vlog_get_module_name(module));
             break;
         case 'd':
-            p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp);
-            ds_put_strftime(s, tmp, time_wall(), false);
+            p = fetch_braces(p, "%Y-%m-%d %H:%M:%S.###", tmp, sizeof tmp);
+            ds_put_strftime_msec(s, tmp, time_wall_msec(), false);
             break;
         case 'D':
-            p = fetch_braces(p, "%Y-%m-%d %H:%M:%S", tmp, sizeof tmp);
-            ds_put_strftime(s, tmp, time_wall(), true);
+            p = fetch_braces(p, "%Y-%m-%d %H:%M:%S.###", tmp, sizeof tmp);
+            ds_put_strftime_msec(s, tmp, time_wall_msec(), true);
             break;
         case 'm':
             /* Format user-supplied log message and trim trailing new-lines. */
index d2c6679..d7d63bf 100644 (file)
@@ -64,7 +64,7 @@ enum vlog_level vlog_get_level_val(const char *name);
 #define VLOG_FACILITIES                                                 \
     VLOG_FACILITY(SYSLOG, "ovs|%05N|%c%T|%p|%m")                            \
     VLOG_FACILITY(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m")    \
-    VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m")
+    VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%S.###Z}|%05N|%c%T|%p|%m")
 enum vlog_facility {
 #define VLOG_FACILITY(NAME, PATTERN) VLF_##NAME,
     VLOG_FACILITIES
index 2f315e6..4a370eb 100644 (file)
@@ -271,6 +271,7 @@ void
 connmgr_run(struct connmgr *mgr,
             bool (*handle_openflow)(struct ofconn *,
                                     const struct ofpbuf *ofp_msg))
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofconn *ofconn, *next_ofconn;
     struct ofservice *ofservice;
@@ -1667,6 +1668,7 @@ connmgr_has_in_band(struct connmgr *mgr)
  * In-band control has more sophisticated code that manages flows itself. */
 void
 connmgr_flushed(struct connmgr *mgr)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     if (mgr->fail_open) {
         fail_open_flushed(mgr->fail_open);
@@ -1833,6 +1835,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
                  enum nx_flow_update_event event,
                  enum ofp_flow_removed_reason reason,
                  const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
+    OVS_REQUIRES(ofproto_mutex)
 {
     enum nx_flow_monitor_flags update;
     struct ofconn *ofconn;
@@ -1897,14 +1900,14 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
                 fu.match = &match;
                 fu.priority = rule->cr.priority;
 
-                ovs_mutex_lock(&rule->timeout_mutex);
+                ovs_mutex_lock(&rule->mutex);
                 fu.idle_timeout = rule->idle_timeout;
                 fu.hard_timeout = rule->hard_timeout;
-                ovs_mutex_unlock(&rule->timeout_mutex);
+                ovs_mutex_unlock(&rule->mutex);
 
                 if (flags & NXFMF_ACTIONS) {
-                    fu.ofpacts = rule->ofpacts;
-                    fu.ofpacts_len = rule->ofpacts_len;
+                    fu.ofpacts = rule->actions->ofpacts;
+                    fu.ofpacts_len = rule->actions->ofpacts_len;
                 } else {
                     fu.ofpacts = NULL;
                     fu.ofpacts_len = 0;
@@ -1951,12 +1954,12 @@ ofmonitor_flush(struct connmgr *mgr)
 static void
 ofmonitor_resume(struct ofconn *ofconn)
 {
+    struct rule_collection rules;
     struct ofpbuf *resumed;
     struct ofmonitor *m;
-    struct list rules;
     struct list msgs;
 
-    list_init(&rules);
+    rule_collection_init(&rules);
     HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) {
         ofmonitor_collect_resume_rules(m, ofconn->monitor_paused, &rules);
     }
index 72134b0..55d08a6 100644 (file)
@@ -187,11 +187,15 @@ void ofmonitor_destroy(struct ofmonitor *);
 
 void ofmonitor_report(struct connmgr *, struct rule *,
                       enum nx_flow_update_event, enum ofp_flow_removed_reason,
-                      const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid);
+                      const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
+    OVS_REQUIRES(ofproto_mutex);
 void ofmonitor_flush(struct connmgr *);
 
+
+struct rule_collection;
 void ofmonitor_collect_resume_rules(struct ofmonitor *, uint64_t seqno,
-                                    struct list *rules);
-void ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs);
+                                    struct rule_collection *);
+void ofmonitor_compose_refresh_updates(struct rule_collection *rules,
+                                       struct list *msgs);
 
 #endif /* connmgr.h */
index 2c0a8f3..4900c05 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -211,6 +211,7 @@ fail_open_wait(struct fail_open *fo)
 
 void
 fail_open_flushed(struct fail_open *fo)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     int disconn_secs = connmgr_failure_duration(fo->connmgr);
     bool open = disconn_secs >= trigger_duration(fo);
index 54f441b..bc1e884 100644 (file)
@@ -23,6 +23,7 @@
 #include "dynamic-string.h"
 #include "dpif.h"
 #include "fail-open.h"
+#include "guarded-list.h"
 #include "latch.h"
 #include "seq.h"
 #include "list.h"
@@ -41,6 +42,7 @@ COVERAGE_DEFINE(upcall_queue_overflow);
 COVERAGE_DEFINE(drop_queue_overflow);
 COVERAGE_DEFINE(miss_queue_overflow);
 COVERAGE_DEFINE(fmb_queue_overflow);
+COVERAGE_DEFINE(fmb_queue_revalidated);
 
 /* A thread that processes each upcall handed to it by the dispatcher thread,
  * forwards the upcall's packet, and then queues it to the main ofproto_dpif
@@ -79,26 +81,15 @@ struct udpif {
     struct handler *handlers;          /* Miss handlers. */
     size_t n_handlers;
 
-    /* Atomic queue of unprocessed drop keys. */
-    struct ovs_mutex drop_key_mutex;
-    struct list drop_keys OVS_GUARDED;
-    size_t n_drop_keys OVS_GUARDED;
-
-    /* Atomic queue of special upcalls for ofproto-dpif to process. */
-    struct ovs_mutex upcall_mutex;
-    struct list upcalls OVS_GUARDED;
-    size_t n_upcalls OVS_GUARDED;
-
-    /* Atomic queue of flow_miss_batches. */
-    struct ovs_mutex fmb_mutex;
-    struct list fmbs OVS_GUARDED;
-    size_t n_fmbs OVS_GUARDED;
+    /* Queues to pass up to ofproto-dpif. */
+    struct guarded_list drop_keys; /* "struct drop key"s. */
+    struct guarded_list upcalls;   /* "struct upcall"s. */
+    struct guarded_list fmbs;      /* "struct flow_miss_batch"es. */
 
     /* Number of times udpif_revalidate() has been called. */
     atomic_uint reval_seq;
 
     struct seq *wait_seq;
-    uint64_t last_seq;
 
     struct latch exit_latch; /* Tells child threads to exit. */
 };
@@ -121,13 +112,10 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
     udpif->secret = random_uint32();
     udpif->wait_seq = seq_create();
     latch_init(&udpif->exit_latch);
-    list_init(&udpif->drop_keys);
-    list_init(&udpif->upcalls);
-    list_init(&udpif->fmbs);
+    guarded_list_init(&udpif->drop_keys);
+    guarded_list_init(&udpif->upcalls);
+    guarded_list_init(&udpif->fmbs);
     atomic_init(&udpif->reval_seq, 0);
-    ovs_mutex_init(&udpif->drop_key_mutex);
-    ovs_mutex_init(&udpif->upcall_mutex);
-    ovs_mutex_init(&udpif->fmb_mutex);
 
     return udpif;
 }
@@ -153,9 +141,9 @@ udpif_destroy(struct udpif *udpif)
         flow_miss_batch_destroy(fmb);
     }
 
-    ovs_mutex_destroy(&udpif->drop_key_mutex);
-    ovs_mutex_destroy(&udpif->upcall_mutex);
-    ovs_mutex_destroy(&udpif->fmb_mutex);
+    guarded_list_destroy(&udpif->drop_keys);
+    guarded_list_destroy(&udpif->upcalls);
+    guarded_list_destroy(&udpif->fmbs);
     latch_destroy(&udpif->exit_latch);
     seq_destroy(udpif->wait_seq);
     free(udpif);
@@ -228,34 +216,17 @@ udpif_recv_set(struct udpif *udpif, size_t n_handlers, bool enable)
     }
 }
 
-void
-udpif_run(struct udpif *udpif)
-{
-    udpif->last_seq = seq_read(udpif->wait_seq);
-}
-
 void
 udpif_wait(struct udpif *udpif)
 {
-    ovs_mutex_lock(&udpif->drop_key_mutex);
-    if (udpif->n_drop_keys) {
-        poll_immediate_wake();
-    }
-    ovs_mutex_unlock(&udpif->drop_key_mutex);
-
-    ovs_mutex_lock(&udpif->upcall_mutex);
-    if (udpif->n_upcalls) {
-        poll_immediate_wake();
-    }
-    ovs_mutex_unlock(&udpif->upcall_mutex);
-
-    ovs_mutex_lock(&udpif->fmb_mutex);
-    if (udpif->n_fmbs) {
+    uint64_t seq = seq_read(udpif->wait_seq);
+    if (!guarded_list_is_empty(&udpif->drop_keys) ||
+        !guarded_list_is_empty(&udpif->upcalls) ||
+        !guarded_list_is_empty(&udpif->fmbs)) {
         poll_immediate_wake();
+    } else {
+        seq_wait(udpif->wait_seq, seq);
     }
-    ovs_mutex_unlock(&udpif->fmb_mutex);
-
-    seq_wait(udpif->wait_seq, udpif->last_seq);
 }
 
 /* Notifies 'udpif' that something changed which may render previous
@@ -265,20 +236,21 @@ udpif_revalidate(struct udpif *udpif)
 {
     struct flow_miss_batch *fmb, *next_fmb;
     unsigned int junk;
+    struct list fmbs;
 
     /* Since we remove each miss on revalidation, their statistics won't be
      * accounted to the appropriate 'facet's in the upper layer.  In most
      * cases, this is alright because we've already pushed the stats to the
      * relevant rules.  However, NetFlow requires absolute packet counts on
      * 'facet's which could now be incorrect. */
-    ovs_mutex_lock(&udpif->fmb_mutex);
     atomic_add(&udpif->reval_seq, 1, &junk);
-    LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &udpif->fmbs) {
+
+    guarded_list_pop_all(&udpif->fmbs, &fmbs);
+    LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &fmbs) {
         list_remove(&fmb->list_node);
         flow_miss_batch_destroy(fmb);
-        udpif->n_fmbs--;
     }
-    ovs_mutex_unlock(&udpif->fmb_mutex);
+
     udpif_drop_key_clear(udpif);
 }
 
@@ -288,16 +260,8 @@ udpif_revalidate(struct udpif *udpif)
 struct upcall *
 upcall_next(struct udpif *udpif)
 {
-    struct upcall *next = NULL;
-
-    ovs_mutex_lock(&udpif->upcall_mutex);
-    if (udpif->n_upcalls) {
-        udpif->n_upcalls--;
-        next = CONTAINER_OF(list_pop_front(&udpif->upcalls), struct upcall,
-                            list_node);
-    }
-    ovs_mutex_unlock(&udpif->upcall_mutex);
-    return next;
+    struct list *next = guarded_list_pop_front(&udpif->upcalls);
+    return next ? CONTAINER_OF(next, struct upcall, list_node) : NULL;
 }
 
 /* Destroys and deallocates 'upcall'. */
@@ -316,16 +280,28 @@ upcall_destroy(struct upcall *upcall)
 struct flow_miss_batch *
 flow_miss_batch_next(struct udpif *udpif)
 {
-    struct flow_miss_batch *next = NULL;
+    int i;
+
+    for (i = 0; i < 50; i++) {
+        struct flow_miss_batch *next;
+        unsigned int reval_seq;
+        struct list *next_node;
+
+        next_node = guarded_list_pop_front(&udpif->fmbs);
+        if (!next_node) {
+            break;
+        }
 
-    ovs_mutex_lock(&udpif->fmb_mutex);
-    if (udpif->n_fmbs) {
-        udpif->n_fmbs--;
-        next = CONTAINER_OF(list_pop_front(&udpif->fmbs),
-                            struct flow_miss_batch, list_node);
+        next = CONTAINER_OF(next_node, struct flow_miss_batch, list_node);
+        atomic_read(&udpif->reval_seq, &reval_seq);
+        if (next->reval_seq == reval_seq) {
+            return next;
+        }
+
+        flow_miss_batch_destroy(next);
     }
-    ovs_mutex_unlock(&udpif->fmb_mutex);
-    return next;
+
+    return NULL;
 }
 
 /* Destroys and deallocates 'fmb'. */
@@ -347,52 +323,13 @@ flow_miss_batch_destroy(struct flow_miss_batch *fmb)
     free(fmb);
 }
 
-/* Discards any flow miss batches queued up in 'udpif' for 'ofproto' (because
- * 'ofproto' is being destroyed).
- *
- * 'ofproto''s xports must already have been removed, otherwise new flow miss
- * batches could still end up getting queued. */
-void
-flow_miss_batch_ofproto_destroyed(struct udpif *udpif,
-                                  const struct ofproto_dpif *ofproto)
-{
-    struct flow_miss_batch *fmb, *next_fmb;
-
-    ovs_mutex_lock(&udpif->fmb_mutex);
-    LIST_FOR_EACH_SAFE (fmb, next_fmb, list_node, &udpif->fmbs) {
-        struct flow_miss *miss, *next_miss;
-
-        HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &fmb->misses) {
-            if (miss->ofproto == ofproto) {
-                hmap_remove(&fmb->misses, &miss->hmap_node);
-                miss_destroy(miss);
-            }
-        }
-
-        if (hmap_is_empty(&fmb->misses)) {
-            list_remove(&fmb->list_node);
-            flow_miss_batch_destroy(fmb);
-            udpif->n_fmbs--;
-        }
-    }
-    ovs_mutex_unlock(&udpif->fmb_mutex);
-}
-
 /* Retreives the next drop key which ofproto-dpif needs to process.  The caller
  * is responsible for destroying it with drop_key_destroy(). */
 struct drop_key *
 drop_key_next(struct udpif *udpif)
 {
-    struct drop_key *next = NULL;
-
-    ovs_mutex_lock(&udpif->drop_key_mutex);
-    if (udpif->n_drop_keys) {
-        udpif->n_drop_keys--;
-        next = CONTAINER_OF(list_pop_front(&udpif->drop_keys), struct drop_key,
-                            list_node);
-    }
-    ovs_mutex_unlock(&udpif->drop_key_mutex);
-    return next;
+    struct list *next = guarded_list_pop_front(&udpif->drop_keys);
+    return next ? CONTAINER_OF(next, struct drop_key, list_node) : NULL;
 }
 
 /* Destorys and deallocates 'drop_key'. */
@@ -410,14 +347,13 @@ void
 udpif_drop_key_clear(struct udpif *udpif)
 {
     struct drop_key *drop_key, *next;
+    struct list list;
 
-    ovs_mutex_lock(&udpif->drop_key_mutex);
-    LIST_FOR_EACH_SAFE (drop_key, next, list_node, &udpif->drop_keys) {
+    guarded_list_pop_all(&udpif->drop_keys, &list);
+    LIST_FOR_EACH_SAFE (drop_key, next, list_node, &list) {
         list_remove(&drop_key->list_node);
         drop_key_destroy(drop_key);
-        udpif->n_drop_keys--;
     }
-    ovs_mutex_unlock(&udpif->drop_key_mutex);
 }
 \f
 /* The dispatcher thread is responsible for receving upcalls from the kernel,
@@ -618,17 +554,16 @@ recv_upcalls(struct udpif *udpif)
                 upcall_destroy(upcall);
             }
         } else {
-            ovs_mutex_lock(&udpif->upcall_mutex);
-            if (udpif->n_upcalls < MAX_QUEUE_LENGTH) {
-                n_udpif_new_upcalls = ++udpif->n_upcalls;
-                list_push_back(&udpif->upcalls, &upcall->list_node);
-                ovs_mutex_unlock(&udpif->upcall_mutex);
+            size_t len;
 
+            len = guarded_list_push_back(&udpif->upcalls, &upcall->list_node,
+                                         MAX_QUEUE_LENGTH);
+            if (len > 0) {
+                n_udpif_new_upcalls = len;
                 if (n_udpif_new_upcalls >= FLOW_MISS_MAX_BATCH) {
                     seq_change(udpif->wait_seq);
                 }
             } else {
-                ovs_mutex_unlock(&udpif->upcall_mutex);
                 COVERAGE_INC(upcall_queue_overflow);
                 upcall_destroy(upcall);
             }
@@ -725,7 +660,7 @@ execute_flow_miss(struct flow_miss *miss, struct dpif_op *ops, size_t *n_ops)
             xlate_actions_for_side_effects(&xin);
         }
     }
-    rule_dpif_release(rule);
+    rule_dpif_unref(rule);
 
     if (miss->xout.odp_actions.size) {
         LIST_FOR_EACH (packet, list_node, &miss->packets) {
@@ -762,13 +697,11 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls)
 {
     struct dpif_op *opsp[FLOW_MISS_MAX_BATCH];
     struct dpif_op ops[FLOW_MISS_MAX_BATCH];
-    unsigned int old_reval_seq, new_reval_seq;
     struct upcall *upcall, *next;
     struct flow_miss_batch *fmb;
     size_t n_upcalls, n_ops, i;
     struct flow_miss *miss;
-
-    atomic_read(&udpif->reval_seq, &old_reval_seq);
+    unsigned int reval_seq;
 
     /* Construct the to-do list.
      *
@@ -776,6 +709,7 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls)
      * the packets that have the same flow in the same "flow_miss" structure so
      * that we can process them together. */
     fmb = xmalloc(sizeof *fmb);
+    atomic_read(&udpif->reval_seq, &fmb->reval_seq);
     hmap_init(&fmb->misses);
     n_upcalls = 0;
     LIST_FOR_EACH_SAFE (upcall, next, list_node, upcalls) {
@@ -808,14 +742,10 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls)
             drop_key->key = xmemdup(dupcall->key, dupcall->key_len);
             drop_key->key_len = dupcall->key_len;
 
-            ovs_mutex_lock(&udpif->drop_key_mutex);
-            if (udpif->n_drop_keys < MAX_QUEUE_LENGTH) {
-                udpif->n_drop_keys++;
-                list_push_back(&udpif->drop_keys, &drop_key->list_node);
-                ovs_mutex_unlock(&udpif->drop_key_mutex);
+            if (guarded_list_push_back(&udpif->drop_keys, &drop_key->list_node,
+                                       MAX_QUEUE_LENGTH)) {
                 seq_change(udpif->wait_seq);
             } else {
-                ovs_mutex_unlock(&udpif->drop_key_mutex);
                 COVERAGE_INC(drop_queue_overflow);
                 drop_key_destroy(drop_key);
             }
@@ -868,21 +798,15 @@ handle_miss_upcalls(struct udpif *udpif, struct list *upcalls)
     }
     dpif_operate(udpif->dpif, opsp, n_ops);
 
-    ovs_mutex_lock(&udpif->fmb_mutex);
-    atomic_read(&udpif->reval_seq, &new_reval_seq);
-    if (old_reval_seq != new_reval_seq) {
-        /* udpif_revalidate() was called as we were calculating the actions.
-         * To be safe, we need to assume all the misses need revalidation. */
-        ovs_mutex_unlock(&udpif->fmb_mutex);
+    atomic_read(&udpif->reval_seq, &reval_seq);
+    if (reval_seq != fmb->reval_seq) {
+        COVERAGE_INC(fmb_queue_revalidated);
         flow_miss_batch_destroy(fmb);
-    } else if (udpif->n_fmbs < MAX_QUEUE_LENGTH) {
-        udpif->n_fmbs++;
-        list_push_back(&udpif->fmbs, &fmb->list_node);
-        ovs_mutex_unlock(&udpif->fmb_mutex);
-        seq_change(udpif->wait_seq);
-    } else {
+    } else if (!guarded_list_push_back(&udpif->fmbs, &fmb->list_node,
+                                       MAX_QUEUE_LENGTH)) {
         COVERAGE_INC(fmb_queue_overflow);
-        ovs_mutex_unlock(&udpif->fmb_mutex);
         flow_miss_batch_destroy(fmb);
+    } else {
+        seq_change(udpif->wait_seq);
     }
 }
index 8e8264e..57d462d 100644 (file)
@@ -36,7 +36,6 @@ struct udpif *udpif_create(struct dpif_backer *, struct dpif *);
 void udpif_recv_set(struct udpif *, size_t n_workers, bool enable);
 void udpif_destroy(struct udpif *);
 
-void udpif_run(struct udpif *);
 void udpif_wait(struct udpif *);
 
 void udpif_revalidate(struct udpif *);
@@ -105,13 +104,12 @@ struct flow_miss_batch {
 
     struct flow_miss miss_buf[FLOW_MISS_MAX_BATCH];
     struct hmap misses;
+
+    unsigned int reval_seq;
 };
 
 struct flow_miss_batch *flow_miss_batch_next(struct udpif *);
 void flow_miss_batch_destroy(struct flow_miss_batch *);
-
-void flow_miss_batch_ofproto_destroyed(struct udpif *,
-                                       const struct ofproto_dpif *);
 \f
 /* Drop keys are odp flow keys which have drop flows installed in the kernel.
  * These are datapath flows which have no associated ofproto, if they did we
index e7cec14..a5b6814 100644 (file)
@@ -44,6 +44,7 @@
 #include "ofproto/ofproto-dpif-mirror.h"
 #include "ofproto/ofproto-dpif-sflow.h"
 #include "ofproto/ofproto-dpif.h"
+#include "ofproto/ofproto-provider.h"
 #include "tunnel.h"
 #include "vlog.h"
 
@@ -1668,11 +1669,9 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port)
 
 static void
 xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
-    OVS_RELEASES(rule)
 {
     struct rule_dpif *old_rule = ctx->rule;
-    const struct ofpact *ofpacts;
-    size_t ofpacts_len;
+    struct rule_actions *actions;
 
     if (ctx->xin->resubmit_stats) {
         rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats);
@@ -1680,12 +1679,11 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
 
     ctx->recurse++;
     ctx->rule = rule;
-    rule_dpif_get_actions(rule, &ofpacts, &ofpacts_len);
-    do_xlate_actions(ofpacts, ofpacts_len, ctx);
+    actions = rule_dpif_get_actions(rule);
+    do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
+    rule_actions_unref(actions);
     ctx->rule = old_rule;
     ctx->recurse--;
-
-    rule_dpif_release(rule);
 }
 
 static void
@@ -1696,7 +1694,6 @@ xlate_table_action(struct xlate_ctx *ctx,
         struct rule_dpif *rule;
         ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port;
         uint8_t old_table_id = ctx->table_id;
-        bool got_rule;
 
         ctx->table_id = table_id;
 
@@ -1704,18 +1701,16 @@ xlate_table_action(struct xlate_ctx *ctx,
          * original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will
          * have surprising behavior). */
         ctx->xin->flow.in_port.ofp_port = in_port;
-        got_rule = rule_dpif_lookup_in_table(ctx->xbridge->ofproto,
-                                             &ctx->xin->flow, &ctx->xout->wc,
-                                             table_id, &rule);
+        rule_dpif_lookup_in_table(ctx->xbridge->ofproto,
+                                  &ctx->xin->flow, &ctx->xout->wc,
+                                  table_id, &rule);
         ctx->xin->flow.in_port.ofp_port = old_in_port;
 
         if (ctx->xin->resubmit_hook) {
             ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
         }
 
-        if (got_rule) {
-            xlate_recursively(ctx, rule);
-        } else if (may_packet_in) {
+        if (!rule && may_packet_in) {
             struct xport *xport;
 
             /* XXX
@@ -1728,7 +1723,10 @@ xlate_table_action(struct xlate_ctx *ctx,
             choose_miss_rule(xport ? xport->config : 0,
                              ctx->xbridge->miss_rule,
                              ctx->xbridge->no_packet_in_rule, &rule);
+        }
+        if (rule) {
             xlate_recursively(ctx, rule);
+            rule_dpif_unref(rule);
         }
 
         ctx->table_id = old_table_id;
@@ -2098,7 +2096,8 @@ static void
 xlate_learn_action(struct xlate_ctx *ctx,
                    const struct ofpact_learn *learn)
 {
-    struct ofputil_flow_mod *fm;
+    uint64_t ofpacts_stub[1024 / 8];
+    struct ofputil_flow_mod fm;
     struct ofpbuf ofpacts;
 
     ctx->xout->has_learn = true;
@@ -2109,11 +2108,10 @@ xlate_learn_action(struct xlate_ctx *ctx,
         return;
     }
 
-    fm = xmalloc(sizeof *fm);
-    ofpbuf_init(&ofpacts, 0);
-    learn_execute(learn, &ctx->xin->flow, fm, &ofpacts);
-
-    ofproto_dpif_flow_mod(ctx->xbridge->ofproto, fm);
+    ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+    learn_execute(learn, &ctx->xin->flow, &fm, &ofpacts);
+    ofproto_dpif_flow_mod(ctx->xbridge->ofproto, &fm);
+    ofpbuf_uninit(&ofpacts);
 }
 
 static void
@@ -2528,6 +2526,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     struct flow_wildcards *wc = &xout->wc;
     struct flow *flow = &xin->flow;
 
+    struct rule_actions *actions = NULL;
     enum slow_path_reason special;
     const struct ofpact *ofpacts;
     struct xport *in_port;
@@ -2604,7 +2603,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         ofpacts = xin->ofpacts;
         ofpacts_len = xin->ofpacts_len;
     } else if (xin->rule) {
-        rule_dpif_get_actions(xin->rule, &ofpacts, &ofpacts_len);
+        actions = rule_dpif_get_actions(xin->rule);
+        ofpacts = actions->ofpacts;
+        ofpacts_len = actions->ofpacts_len;
     } else {
         NOT_REACHED();
     }
@@ -2690,4 +2691,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
 out:
     ovs_rwlock_unlock(&xlate_rwlock);
+
+    rule_actions_unref(actions);
 }
index 95fb874..b91b3df 100644 (file)
@@ -31,6 +31,7 @@
 #include "dpif.h"
 #include "dynamic-string.h"
 #include "fail-open.h"
+#include "guarded-list.h"
 #include "hmapx.h"
 #include "lacp.h"
 #include "learn.h"
@@ -484,9 +485,6 @@ struct ofproto_dpif {
     struct classifier facets;     /* Contains 'struct facet's. */
     long long int consistency_rl;
 
-    /* Support for debugging async flow mods. */
-    struct list completions;
-
     struct netdev_stats stats; /* To account packets generated and consumed in
                                 * userspace. */
 
@@ -510,19 +508,9 @@ struct ofproto_dpif {
     uint64_t n_missed;
 
     /* Work queues. */
-    struct ovs_mutex flow_mod_mutex;
-    struct list flow_mods OVS_GUARDED;
-    size_t n_flow_mods OVS_GUARDED;
-
-    struct ovs_mutex pin_mutex;
-    struct list pins OVS_GUARDED;
-    size_t n_pins OVS_GUARDED;
+    struct guarded_list pins;      /* Contains "struct ofputil_packet_in"s. */
 };
 
-/* Defer flow mod completion until "ovs-appctl ofproto/unclog"?  (Useful only
- * for debugging the asynchronous flow_mod implementation.) */
-static bool clogged;
-
 /* By default, flows in the datapath are wildcarded (megaflows).  They
  * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
 static bool enable_megaflows = true;
@@ -562,23 +550,13 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 /* Initial mappings of port to bridge mappings. */
 static struct shash init_ofp_ports = SHASH_INITIALIZER(&init_ofp_ports);
 
-/* Executes and takes ownership of 'fm'. */
+/* Executes 'fm'.  The caller retains ownership of 'fm' and everything in
+ * it. */
 void
 ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
                       struct ofputil_flow_mod *fm)
 {
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    if (ofproto->n_flow_mods > 1024) {
-        ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-        COVERAGE_INC(flow_mod_overflow);
-        free(fm->ofpacts);
-        free(fm);
-        return;
-    }
-
-    list_push_back(&ofproto->flow_mods, &fm->list_node);
-    ofproto->n_flow_mods++;
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
+    ofproto_flow_mod(&ofproto->up, fm);
 }
 
 /* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
@@ -587,18 +565,11 @@ void
 ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto,
                             struct ofputil_packet_in *pin)
 {
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    if (ofproto->n_pins > 1024) {
-        ovs_mutex_unlock(&ofproto->pin_mutex);
+    if (!guarded_list_push_back(&ofproto->pins, &pin->list_node, 1024)) {
         COVERAGE_INC(packet_in_overflow);
         free(CONST_CAST(void *, pin->packet));
         free(pin);
-        return;
     }
-
-    list_push_back(&ofproto->pins, &pin->list_node);
-    ofproto->n_pins++;
-    ovs_mutex_unlock(&ofproto->pin_mutex);
 }
 \f
 /* Factory functions. */
@@ -1031,7 +1002,6 @@ process_dpif_port_error(struct dpif_backer *backer, int error)
 static int
 dpif_backer_run_fast(struct dpif_backer *backer)
 {
-    udpif_run(backer->udpif);
     handle_upcalls(backer);
 
     return 0;
@@ -1293,19 +1263,7 @@ construct(struct ofproto *ofproto_)
     classifier_init(&ofproto->facets);
     ofproto->consistency_rl = LLONG_MIN;
 
-    list_init(&ofproto->completions);
-
-    ovs_mutex_init(&ofproto->flow_mod_mutex);
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    list_init(&ofproto->flow_mods);
-    ofproto->n_flow_mods = 0;
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-
-    ovs_mutex_init(&ofproto->pin_mutex);
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    list_init(&ofproto->pins);
-    ofproto->n_pins = 0;
-    ovs_mutex_unlock(&ofproto->pin_mutex);
+    guarded_list_init(&ofproto->pins);
 
     ofproto_dpif_unixctl_init();
 
@@ -1380,7 +1338,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
 
     if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL,
                                   rulep)) {
-        rule_dpif_release(*rulep);
+        rule_dpif_unref(*rulep);
     } else {
         NOT_REACHED();
     }
@@ -1423,28 +1381,16 @@ add_internal_flows(struct ofproto_dpif *ofproto)
     return error;
 }
 
-static void
-complete_operations(struct ofproto_dpif *ofproto)
-{
-    struct dpif_completion *c, *next;
-
-    LIST_FOR_EACH_SAFE (c, next, list_node, &ofproto->completions) {
-        ofoperation_complete(c->op, 0);
-        list_remove(&c->list_node);
-        free(c);
-    }
-}
-
 static void
 destruct(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct rule_dpif *rule, *next_rule;
     struct ofputil_packet_in *pin, *next_pin;
-    struct ofputil_flow_mod *fm, *next_fm;
     struct facet *facet, *next_facet;
     struct cls_cursor cursor;
     struct oftable *table;
+    struct list pins;
 
     ovs_rwlock_rdlock(&ofproto->facets.rwlock);
     cls_cursor_init(&cursor, &ofproto->facets, NULL);
@@ -1458,42 +1404,30 @@ destruct(struct ofproto *ofproto_)
     xlate_remove_ofproto(ofproto);
     ovs_rwlock_unlock(&xlate_rwlock);
 
-    flow_miss_batch_ofproto_destroyed(ofproto->backer->udpif, ofproto);
+    /* Discard any flow_miss_batches queued up for 'ofproto', avoiding a
+     * use-after-free error. */
+    udpif_revalidate(ofproto->backer->udpif);
 
     hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
-    complete_operations(ofproto);
 
     OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
         struct cls_cursor cursor;
 
-        ovs_rwlock_wrlock(&table->cls.rwlock);
+        ovs_rwlock_rdlock(&table->cls.rwlock);
         cls_cursor_init(&cursor, &table->cls, NULL);
+        ovs_rwlock_unlock(&table->cls.rwlock);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, up.cr, &cursor) {
-            ofproto_rule_delete(&ofproto->up, &table->cls, &rule->up);
+            ofproto_rule_delete(&ofproto->up, &rule->up);
         }
-        ovs_rwlock_unlock(&table->cls.rwlock);
-    }
-    complete_operations(ofproto);
-
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &ofproto->flow_mods) {
-        list_remove(&fm->list_node);
-        ofproto->n_flow_mods--;
-        free(fm->ofpacts);
-        free(fm);
     }
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-    ovs_mutex_destroy(&ofproto->flow_mod_mutex);
 
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &ofproto->pins) {
+    guarded_list_pop_all(&ofproto->pins, &pins);
+    LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         list_remove(&pin->list_node);
-        ofproto->n_pins--;
         free(CONST_CAST(void *, pin->packet));
         free(pin);
     }
-    ovs_mutex_unlock(&ofproto->pin_mutex);
-    ovs_mutex_destroy(&ofproto->pin_mutex);
+    guarded_list_destroy(&ofproto->pins);
 
     mbridge_unref(ofproto->mbridge);
 
@@ -1521,9 +1455,8 @@ run_fast(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct ofputil_packet_in *pin, *next_pin;
-    struct ofputil_flow_mod *fm, *next_fm;
-    struct list flow_mods, pins;
     struct ofport_dpif *ofport;
+    struct list pins;
 
     /* Do not perform any periodic activity required by 'ofproto' while
      * waiting for flow restore to complete. */
@@ -1531,30 +1464,7 @@ run_fast(struct ofproto *ofproto_)
         return 0;
     }
 
-    ovs_mutex_lock(&ofproto->flow_mod_mutex);
-    list_move(&flow_mods, &ofproto->flow_mods);
-    list_init(&ofproto->flow_mods);
-    ofproto->n_flow_mods = 0;
-    ovs_mutex_unlock(&ofproto->flow_mod_mutex);
-
-    LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &flow_mods) {
-        int error = ofproto_flow_mod(&ofproto->up, fm);
-        if (error && !VLOG_DROP_WARN(&rl)) {
-            VLOG_WARN("learning action failed to modify flow table (%s)",
-                      ofperr_get_name(error));
-        }
-
-        list_remove(&fm->list_node);
-        free(fm->ofpacts);
-        free(fm);
-    }
-
-    ovs_mutex_lock(&ofproto->pin_mutex);
-    list_move(&pins, &ofproto->pins);
-    list_init(&ofproto->pins);
-    ofproto->n_pins = 0;
-    ovs_mutex_unlock(&ofproto->pin_mutex);
-
+    guarded_list_pop_all(&ofproto->pins, &pins);
     LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         connmgr_send_packet_in(ofproto->up.connmgr, pin);
         list_remove(&pin->list_node);
@@ -1577,10 +1487,6 @@ run(struct ofproto *ofproto_)
     struct ofbundle *bundle;
     int error;
 
-    if (!clogged) {
-        complete_operations(ofproto);
-    }
-
     if (mbridge_need_revalidate(ofproto->mbridge)) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
         ovs_rwlock_wrlock(&ofproto->ml->rwlock);
@@ -1658,10 +1564,6 @@ wait(struct ofproto *ofproto_)
     struct ofport_dpif *ofport;
     struct ofbundle *bundle;
 
-    if (!clogged && !list_is_empty(&ofproto->completions)) {
-        poll_immediate_wake();
-    }
-
     if (ofproto_get_flow_restore_wait()) {
         return;
     }
@@ -3657,7 +3559,7 @@ handle_upcalls(struct dpif_backer *backer)
 
 static int subfacet_max_idle(const struct dpif_backer *);
 static void update_stats(struct dpif_backer *);
-static void rule_expire(struct rule_dpif *);
+static void rule_expire(struct rule_dpif *) OVS_REQUIRES(ofproto_mutex);
 static void expire_subfacets(struct dpif_backer *, int dp_max_idle);
 
 /* This function is called periodically by run().  Its job is to collect
@@ -3711,12 +3613,12 @@ expire(struct dpif_backer *backer)
 
         /* Expire OpenFlow flows whose idle_timeout or hard_timeout
          * has passed. */
-        ovs_mutex_lock(&ofproto->up.expirable_mutex);
+        ovs_mutex_lock(&ofproto_mutex);
         LIST_FOR_EACH_SAFE (rule, next_rule, expirable,
                             &ofproto->up.expirable) {
             rule_expire(rule_dpif_cast(rule));
         }
-        ovs_mutex_unlock(&ofproto->up.expirable_mutex);
+        ovs_mutex_unlock(&ofproto_mutex);
 
         /* All outstanding data in existing flows has been accounted, so it's a
          * good time to do bond rebalancing. */
@@ -3977,33 +3879,31 @@ expire_subfacets(struct dpif_backer *backer, int dp_max_idle)
  * then delete it entirely. */
 static void
 rule_expire(struct rule_dpif *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     uint16_t idle_timeout, hard_timeout;
-    long long int now;
-    uint8_t reason;
+    long long int now = time_msec();
+    int reason;
 
-    if (rule->up.pending) {
-        /* We'll have to expire it later. */
-        return;
-    }
+    ovs_assert(!rule->up.pending);
 
-    ovs_mutex_lock(&rule->up.timeout_mutex);
+    /* Has 'rule' expired? */
+    ovs_mutex_lock(&rule->up.mutex);
     hard_timeout = rule->up.hard_timeout;
     idle_timeout = rule->up.idle_timeout;
-    ovs_mutex_unlock(&rule->up.timeout_mutex);
-
-    /* Has 'rule' expired? */
-    now = time_msec();
     if (hard_timeout && now > rule->up.modified + hard_timeout * 1000) {
         reason = OFPRR_HARD_TIMEOUT;
     } else if (idle_timeout && now > rule->up.used + idle_timeout * 1000) {
         reason = OFPRR_IDLE_TIMEOUT;
     } else {
-        return;
+        reason = -1;
     }
+    ovs_mutex_unlock(&rule->up.mutex);
 
-    COVERAGE_INC(ofproto_dpif_expired);
-    ofproto_rule_expire(&rule->up, reason);
+    if (reason >= 0) {
+        COVERAGE_INC(ofproto_dpif_expired);
+        ofproto_rule_expire(&rule->up, reason);
+    }
 }
 \f
 /* Facets. */
@@ -4195,17 +4095,22 @@ facet_is_controller_flow(struct facet *facet)
     if (facet) {
         struct ofproto_dpif *ofproto = facet->ofproto;
         const struct ofpact *ofpacts;
+        struct rule_actions *actions;
         struct rule_dpif *rule;
         size_t ofpacts_len;
         bool is_controller;
 
         rule_dpif_lookup(ofproto, &facet->flow, NULL, &rule);
-        ofpacts_len = rule->up.ofpacts_len;
-        ofpacts = rule->up.ofpacts;
+        actions = rule_dpif_get_actions(rule);
+        rule_dpif_unref(rule);
+
+        ofpacts_len = actions->ofpacts_len;
+        ofpacts = actions->ofpacts;
         is_controller = ofpacts_len > 0
             && ofpacts->type == OFPACT_CONTROLLER
             && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len);
-        rule_dpif_release(rule);
+        rule_actions_unref(actions);
+
         return is_controller;
     }
     return false;
@@ -4299,7 +4204,7 @@ facet_check_consistency(struct facet *facet)
     rule_dpif_lookup(facet->ofproto, &facet->flow, NULL, &rule);
     xlate_in_init(&xin, facet->ofproto, &facet->flow, rule, 0, NULL);
     xlate_actions(&xin, &xout);
-    rule_dpif_release(rule);
+    rule_dpif_unref(rule);
 
     ok = ofpbuf_equal(&facet->xout.odp_actions, &xout.odp_actions)
         && facet->xout.slow == xout.slow;
@@ -4397,7 +4302,7 @@ facet_revalidate(struct facet *facet)
         || memcmp(&facet->xout.wc, &xout.wc, sizeof xout.wc)) {
         facet_remove(facet);
         xlate_out_uninit(&xout);
-        rule_dpif_release(new_rule);
+        rule_dpif_unref(new_rule);
         return false;
     }
 
@@ -4426,10 +4331,13 @@ facet_revalidate(struct facet *facet)
     facet->xout.nf_output_iface = xout.nf_output_iface;
     facet->xout.mirrors = xout.mirrors;
     facet->nf_flow.output_iface = facet->xout.nf_output_iface;
+
+    ovs_mutex_lock(&new_rule->up.mutex);
     facet->used = MAX(facet->used, new_rule->up.created);
+    ovs_mutex_unlock(&new_rule->up.mutex);
 
     xlate_out_uninit(&xout);
-    rule_dpif_release(new_rule);
+    rule_dpif_unref(new_rule);
     return true;
 }
 
@@ -4462,7 +4370,7 @@ flow_push_stats(struct ofproto_dpif *ofproto, struct flow *flow,
     xin.resubmit_stats = stats;
     xin.may_learn = may_learn;
     xlate_actions_for_side_effects(&xin);
-    rule_dpif_release(rule);
+    rule_dpif_unref(rule);
 }
 
 static void
@@ -4546,6 +4454,7 @@ rule_dpif_fail_open(const struct rule_dpif *rule)
 
 ovs_be64
 rule_dpif_get_flow_cookie(const struct rule_dpif *rule)
+    OVS_REQUIRES(rule->up.mutex)
 {
     return rule->up.flow_cookie;
 }
@@ -4557,12 +4466,13 @@ rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
     ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout);
 }
 
-void
-rule_dpif_get_actions(const struct rule_dpif *rule,
-                      const struct ofpact **ofpacts, size_t *ofpacts_len)
+/* Returns 'rule''s actions.  The caller owns a reference on the returned
+ * actions and must eventually release it (with rule_actions_unref()) to avoid
+ * a memory leak. */
+struct rule_actions *
+rule_dpif_get_actions(const struct rule_dpif *rule)
 {
-    *ofpacts = rule->up.ofpacts;
-    *ofpacts_len = rule->up.ofpacts_len;
+    return rule_get_actions(&rule->up);
 }
 \f
 /* Subfacets. */
@@ -4840,9 +4750,8 @@ bool
 rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
                           const struct flow *flow, struct flow_wildcards *wc,
                           uint8_t table_id, struct rule_dpif **rule)
-    OVS_TRY_RDLOCK(true, (*rule)->up.rwlock)
 {
-    struct cls_rule *cls_rule;
+    const struct cls_rule *cls_rule;
     struct classifier *cls;
     bool frag;
 
@@ -4875,11 +4784,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
     }
 
     *rule = rule_dpif_cast(rule_from_cls_rule(cls_rule));
-    if (*rule && ovs_rwlock_tryrdlock(&(*rule)->up.rwlock)) {
-        /* The rule is in the process of being removed.  Best we can do is
-         * pretend it isn't there. */
-        *rule = NULL;
-    }
+    rule_dpif_ref(*rule);
     ovs_rwlock_unlock(&cls->rwlock);
 
     return *rule != NULL;
@@ -4891,34 +4796,35 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
 void
 choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule,
                  struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule)
-    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     *rule = config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule;
-    ovs_rwlock_rdlock(&(*rule)->up.rwlock);
+    rule_dpif_ref(*rule);
+}
+
+void
+rule_dpif_ref(struct rule_dpif *rule)
+{
+    if (rule) {
+        ofproto_rule_ref(&rule->up);
+    }
 }
 
 void
-rule_dpif_release(struct rule_dpif *rule)
-    OVS_NO_THREAD_SAFETY_ANALYSIS
+rule_dpif_unref(struct rule_dpif *rule)
 {
     if (rule) {
-        ovs_rwlock_unlock(&rule->up.rwlock);
+        ofproto_rule_unref(&rule->up);
     }
 }
 
 static void
 complete_operation(struct rule_dpif *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
 
     ofproto->backer->need_revalidate = REV_FLOW_TABLE;
-    if (clogged) {
-        struct dpif_completion *c = xmalloc(sizeof *c);
-        c->op = rule->up.pending;
-        list_push_back(&ofproto->completions, &c->list_node);
-    } else {
-        ofoperation_complete(rule->up.pending, 0);
-    }
+    ofoperation_complete(rule->up.pending, 0);
 }
 
 static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
@@ -4954,6 +4860,7 @@ rule_construct(struct rule *rule_)
 
 static void
 rule_insert(struct rule *rule_)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
     complete_operation(rule);
@@ -4961,6 +4868,7 @@ rule_insert(struct rule *rule_)
 
 static void
 rule_delete(struct rule *rule_)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
     complete_operation(rule);
@@ -5025,6 +4933,7 @@ rule_execute(struct rule *rule, const struct flow *flow,
 
 static void
 rule_modify_actions(struct rule *rule_, bool reset_counters)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct rule_dpif *rule = rule_dpif_cast(rule_);
 
@@ -5339,21 +5248,32 @@ struct trace_ctx {
 static void
 trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
 {
+    struct rule_actions *actions;
+    ovs_be64 cookie;
+
     ds_put_char_multiple(result, '\t', level);
     if (!rule) {
         ds_put_cstr(result, "No match\n");
         return;
     }
 
+    ovs_mutex_lock(&rule->up.mutex);
+    cookie = rule->up.flow_cookie;
+    ovs_mutex_unlock(&rule->up.mutex);
+
     ds_put_format(result, "Rule: table=%"PRIu8" cookie=%#"PRIx64" ",
-                  rule ? rule->up.table_id : 0, ntohll(rule->up.flow_cookie));
+                  rule ? rule->up.table_id : 0, ntohll(cookie));
     cls_rule_format(&rule->up.cr, result);
     ds_put_char(result, '\n');
 
+    actions = rule_dpif_get_actions(rule);
+
     ds_put_char_multiple(result, '\t', level);
     ds_put_cstr(result, "OpenFlow ");
-    ofpacts_format(rule->up.ofpacts, rule->up.ofpacts_len, result);
+    ofpacts_format(actions->ofpacts, actions->ofpacts_len, result);
     ds_put_char(result, '\n');
+
+    rule_actions_unref(actions);
 }
 
 static void
@@ -5623,23 +5543,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
         xlate_out_uninit(&trace.xout);
     }
 
-    rule_dpif_release(rule);
-}
-
-static void
-ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-                  const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-    clogged = true;
-    unixctl_command_reply(conn, NULL);
-}
-
-static void
-ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-                    const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-    clogged = false;
-    unixctl_command_reply(conn, NULL);
+    rule_dpif_unref(rule);
 }
 
 /* Runs a self-check of flow translations in 'ofproto'.  Appends a message to
@@ -6058,10 +5962,6 @@ ofproto_dpif_unixctl_init(void)
                              ofproto_unixctl_fdb_flush, NULL);
     unixctl_command_register("fdb/show", "bridge", 1, 1,
                              ofproto_unixctl_fdb_show, NULL);
-    unixctl_command_register("ofproto/clog", "", 0, 0,
-                             ofproto_dpif_clog, NULL);
-    unixctl_command_register("ofproto/unclog", "", 0, 0,
-                             ofproto_dpif_unclog, NULL);
     unixctl_command_register("ofproto/self-check", "[bridge]", 0, 1,
                              ofproto_dpif_self_check, NULL);
     unixctl_command_register("dpif/dump-dps", "", 0, 0,
index 7efc8d7..14a9669 100644 (file)
@@ -61,24 +61,21 @@ struct OVS_LOCKABLE rule_dpif;
  *   actions into datapath actions. */
 
 void rule_dpif_lookup(struct ofproto_dpif *, const struct flow *,
-                      struct flow_wildcards *, struct rule_dpif **rule)
-    OVS_ACQ_RDLOCK(*rule);
+                      struct flow_wildcards *, struct rule_dpif **rule);
 
 bool rule_dpif_lookup_in_table(struct ofproto_dpif *, const struct flow *,
                                struct flow_wildcards *, uint8_t table_id,
-                               struct rule_dpif **rule)
-    OVS_TRY_RDLOCK(true, *rule);
+                               struct rule_dpif **rule);
 
-void rule_dpif_release(struct rule_dpif *rule) OVS_RELEASES(rule);
+void rule_dpif_ref(struct rule_dpif *);
+void rule_dpif_unref(struct rule_dpif *);
 
 void rule_dpif_credit_stats(struct rule_dpif *rule ,
                             const struct dpif_flow_stats *);
 
 bool rule_dpif_fail_open(const struct rule_dpif *rule);
 
-void rule_dpif_get_actions(const struct rule_dpif *rule,
-                           const struct ofpact **ofpacts,
-                           size_t *ofpacts_len);
+struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
 
 ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule);
 
@@ -88,8 +85,7 @@ void rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
 void choose_miss_rule(enum ofputil_port_config,
                       struct rule_dpif *miss_rule,
                       struct rule_dpif *no_packet_in_rule,
-                      struct rule_dpif **rule)
-    OVS_ACQ_RDLOCK(*rule);
+                      struct rule_dpif **rule);
 
 bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
 ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
index 4cbf47f..bbb9ba1 100644 (file)
 #ifndef OFPROTO_OFPROTO_PROVIDER_H
 #define OFPROTO_OFPROTO_PROVIDER_H 1
 
-/* Definitions for use within ofproto. */
+/* Definitions for use within ofproto.
+ *
+ *
+ * Thread-safety
+ * =============
+ *
+ * Lots of ofproto data structures are only accessed from a single thread.
+ * Those data structures are generally not thread-safe.
+ *
+ * The ofproto-dpif ofproto implementation accesses the flow table from
+ * multiple threads, including modifying the flow table from multiple threads
+ * via the "learn" action, so the flow table and various structures that index
+ * it have been made thread-safe.  Refer to comments on individual data
+ * structures for details.
+ */
 
 #include "cfm.h"
 #include "classifier.h"
+#include "guarded-list.h"
 #include "heap.h"
 #include "hindex.h"
 #include "list.h"
 #include "ofp-errors.h"
 #include "ofp-util.h"
 #include "ofproto/ofproto.h"
+#include "ovs-atomic.h"
 #include "ovs-thread.h"
 #include "shash.h"
 #include "simap.h"
@@ -38,6 +54,8 @@ struct ofputil_flow_mod;
 struct bfd_cfg;
 struct meter;
 
+extern struct ovs_mutex ofproto_mutex;
+
 /* An OpenFlow switch.
  *
  * With few exceptions, ofproto implementations may look at these fields but
@@ -73,13 +91,11 @@ struct ofproto {
     struct oftable *tables;
     int n_tables;
 
-    struct hindex cookies;      /* Rules indexed on their cookie values. */
+    /* Rules indexed on their cookie values, in all flow tables. */
+    struct hindex cookies OVS_GUARDED_BY(ofproto_mutex);
 
-    /* Optimisation for flow expiry.
-     * These flows should all be present in tables. */
-    struct ovs_mutex expirable_mutex;
-    struct list expirable OVS_GUARDED; /* Expirable 'struct rule"s in all
-                                          tables. */
+    /* List of expirable flows, in all flow tables. */
+    struct list expirable OVS_GUARDED_BY(ofproto_mutex);
 
     /* Meter table.
      * OpenFlow meters start at 1.  To avoid confusion we leave the first
@@ -91,11 +107,29 @@ struct ofproto {
     /* OpenFlow connections. */
     struct connmgr *connmgr;
 
-    /* Flow table operation tracking. */
-    int state;                  /* Internal state. */
-    struct list pending;        /* List of "struct ofopgroup"s. */
-    unsigned int n_pending;     /* list_size(&pending). */
-    struct hmap deletions;      /* All OFOPERATION_DELETE "ofoperation"s. */
+    /* Flow table operation tracking.
+     *
+     * 'state' is meaningful only within ofproto.c, one of the enum
+     * ofproto_state constants defined there.
+     *
+     * 'pending' is the list of "struct ofopgroup"s currently pending.
+     *
+     * 'n_pending' is the number of elements in 'pending'.
+     *
+     * 'deletions' contains pending ofoperations of type OFOPERATION_DELETE,
+     * indexed on its rule's flow.*/
+    int state;
+    struct list pending OVS_GUARDED_BY(ofproto_mutex);
+    unsigned int n_pending OVS_GUARDED_BY(ofproto_mutex);
+    struct hmap deletions OVS_GUARDED_BY(ofproto_mutex);
+
+    /* Delayed rule executions.
+     *
+     * We delay calls to ->ofproto_class->rule_execute() past releasing
+     * ofproto_mutex during a flow_mod, because otherwise a "learn" action
+     * triggered by the executing the packet would try to recursively modify
+     * the flow table and reacquire the global lock. */
+    struct guarded_list rule_executes; /* Contains "struct rule_execute"s. */
 
     /* Flow table operation logging. */
     int n_add, n_delete, n_modify; /* Number of unreported ops of each kind. */
@@ -171,7 +205,29 @@ enum oftable_flags {
     OFTABLE_READONLY = 1 << 1  /* Don't allow OpenFlow to change this table. */
 };
 
-/* A flow table within a "struct ofproto". */
+/* A flow table within a "struct ofproto".
+ *
+ *
+ * Thread-safety
+ * =============
+ *
+ * A cls->rwlock read-lock holder prevents rules from being added or deleted.
+ *
+ * Adding or removing rules requires holding ofproto_mutex AND the cls->rwlock
+ * write-lock.
+ *
+ * cls->rwlock should be held only briefly.  For extended access to a rule,
+ * increment its ref_count with ofproto_rule_ref().  A rule will not be freed
+ * until its ref_count reaches zero.
+ *
+ * Modifying a rule requires the rule's own mutex.  Holding cls->rwlock (for
+ * read or write) does not allow the holder to modify the rule.
+ *
+ * Freeing a rule requires ofproto_mutex and the cls->rwlock write-lock.  After
+ * removing the rule from the classifier, release a ref_count from the rule
+ * ('cls''s reference to the rule).
+ *
+ * Refer to the thread-safety notes on struct rule for more information.*/
 struct oftable {
     enum oftable_flags flags;
     struct classifier cls;      /* Contains "struct rule"s. */
@@ -215,60 +271,172 @@ struct oftable {
 /* An OpenFlow flow within a "struct ofproto".
  *
  * With few exceptions, ofproto implementations may look at these fields but
- * should not modify them. */
+ * should not modify them.
+ *
+ *
+ * Thread-safety
+ * =============
+ *
+ * Except near the beginning or ending of its lifespan, rule 'rule' belongs to
+ * the classifier rule->ofproto->tables[rule->table_id].cls.  The text below
+ * calls this classifier 'cls'.
+ *
+ * Motivation
+ * ----------
+ *
+ * The thread safety rules described here for "struct rule" are motivated by
+ * two goals:
+ *
+ *    - Prevent threads that read members of "struct rule" from reading bad
+ *      data due to changes by some thread concurrently modifying those
+ *      members.
+ *
+ *    - Prevent two threads making changes to members of a given "struct rule"
+ *      from interfering with each other.
+ *
+ *
+ * Rules
+ * -----
+ *
+ * A rule 'rule' may be accessed without a risk of being freed by code that
+ * holds a read-lock or write-lock on 'cls->rwlock' or that owns a reference to
+ * 'rule->ref_count' (or both).  Code that needs to hold onto a rule for a
+ * while should take 'cls->rwlock', find the rule it needs, increment
+ * 'rule->ref_count' with ofproto_rule_ref(), and drop 'cls->rwlock'.
+ *
+ * 'rule->ref_count' protects 'rule' from being freed.  It doesn't protect the
+ * rule from being deleted from 'cls' (that's 'cls->rwlock') and it doesn't
+ * protect members of 'rule' from modification (that's 'rule->rwlock').
+ *
+ * 'rule->mutex' protects the members of 'rule' from modification.  It doesn't
+ * protect the rule from being deleted from 'cls' (that's 'cls->rwlock') and it
+ * doesn't prevent the rule from being freed (that's 'rule->ref_count').
+ *
+ * Regarding thread safety, the members of a rule fall into the following
+ * categories:
+ *
+ *    - Immutable.  These members are marked 'const'.
+ *
+ *    - Members that may be safely read or written only by code holding
+ *      ofproto_mutex.  These are marked OVS_GUARDED_BY(ofproto_mutex).
+ *
+ *    - Members that may be safely read only by code holding ofproto_mutex or
+ *      'rule->mutex', and safely written only by coding holding ofproto_mutex
+ *      AND 'rule->mutex'.  These are marked OVS_GUARDED.
+ */
 struct rule {
-    struct list ofproto_node;    /* Owned by ofproto base code. */
-    struct ofproto *ofproto;     /* The ofproto that contains this rule. */
-    struct cls_rule cr;          /* In owning ofproto's classifier. */
+    /* Where this rule resides in an OpenFlow switch.
+     *
+     * These are immutable once the rule is constructed, hence 'const'. */
+    struct ofproto *const ofproto; /* The ofproto that contains this rule. */
+    const struct cls_rule cr;      /* In owning ofproto's classifier. */
+    const uint8_t table_id;        /* Index in ofproto's 'tables' array. */
+
+    /* Protects members marked OVS_GUARDED.
+     * Readers only need to hold this mutex.
+     * Writers must hold both this mutex AND ofproto_mutex. */
+    struct ovs_mutex mutex OVS_ACQ_AFTER(ofproto_mutex);
+
+    /* Number of references.
+     * The classifier owns one reference.
+     * Any thread trying to keep a rule from being freed should hold its own
+     * reference. */
+    atomic_uint ref_count;
+
+    /* Operation now in progress, if nonnull. */
+    struct ofoperation *pending OVS_GUARDED_BY(ofproto_mutex);
+
+    /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with
+     * a flow.. */
+    ovs_be64 flow_cookie OVS_GUARDED;
+    struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex);
+
+    /* Times. */
+    long long int created OVS_GUARDED; /* Creation time. */
+    long long int modified OVS_GUARDED; /* Time of last modification. */
+    long long int used OVS_GUARDED; /* Last use; time created if never used. */
+    enum ofputil_flow_mod_flags flags OVS_GUARDED;
+
+    /* Timeouts. */
+    uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */
+    uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */
 
-    struct ofoperation *pending; /* Operation now in progress, if nonnull. */
+    /* Eviction groups (see comment on struct eviction_group for explanation) .
+     *
+     * 'eviction_group' is this rule's eviction group, or NULL if it is not in
+     * any eviction group.  When 'eviction_group' is nonnull, 'evg_node' is in
+     * the ->eviction_group->rules hmap. */
+    struct eviction_group *eviction_group OVS_GUARDED_BY(ofproto_mutex);
+    struct heap_node evg_node OVS_GUARDED_BY(ofproto_mutex);
 
-    ovs_be64 flow_cookie;        /* Controller-issued identifier. Guarded by
-                                    rwlock. */
-    struct hindex_node cookie_node; /* In owning ofproto's 'cookies' index. */
+    /* OpenFlow actions.  See struct rule_actions for more thread-safety
+     * notes. */
+    struct rule_actions *actions OVS_GUARDED;
 
-    long long int created;       /* Creation time. */
-    long long int modified;      /* Time of last modification. */
-    long long int used;          /* Last use; time created if never used. */
-    uint8_t table_id;            /* Index in ofproto's 'tables' array. */
-    enum ofputil_flow_mod_flags flags;
+    /* In owning meter's 'rules' list.  An empty list if there is no meter. */
+    struct list meter_list_node OVS_GUARDED_BY(ofproto_mutex);
 
-    struct ovs_mutex timeout_mutex;
-    uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */
-    uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */
+    /* Flow monitors (e.g. for NXST_FLOW_MONITOR, related to struct ofmonitor).
+     *
+     * 'add_seqno' is the sequence number when this rule was created.
+     * 'modify_seqno' is the sequence number when this rule was last modified.
+     * See 'monitor_seqno' in connmgr.c for more information. */
+    enum nx_flow_monitor_flags monitor_flags OVS_GUARDED_BY(ofproto_mutex);
+    uint64_t add_seqno OVS_GUARDED_BY(ofproto_mutex);
+    uint64_t modify_seqno OVS_GUARDED_BY(ofproto_mutex);
 
-    /* Eviction groups. */
-    struct heap_node evg_node;   /* In eviction_group's "rules" heap. */
-    struct eviction_group *eviction_group; /* NULL if not in any group. */
+    /* Optimisation for flow expiry.  In ofproto's 'expirable' list if this
+     * rule is expirable, otherwise empty. */
+    struct list expirable OVS_GUARDED_BY(ofproto_mutex);
+};
 
-    /* The rwlock is used to protect those elements in struct rule which are
-     * accessed by multiple threads.  While maintaining a pointer to struct
-     * rule, threads are required to hold a readlock.  The main ofproto code is
-     * guaranteed not to evict the rule, or change any of the elements "Guarded
-     * by rwlock" without holding the writelock.
-     *
-     * A rule will not be evicted unless both its own and its classifier's
-     * write locks are held.  Therefore, while holding a classifier readlock,
-     * one can be assured that write locked rules are safe to reference. */
-    struct ovs_rwlock rwlock;
+void ofproto_rule_ref(struct rule *);
+void ofproto_rule_unref(struct rule *);
 
-    /* Guarded by rwlock. */
-    struct ofpact *ofpacts;      /* Sequence of "struct ofpacts". */
-    unsigned int ofpacts_len;    /* Size of 'ofpacts', in bytes. */
+struct rule_actions *rule_get_actions(const struct rule *rule)
+    OVS_EXCLUDED(rule->mutex);
+struct rule_actions *rule_get_actions__(const struct rule *rule)
+    OVS_REQUIRES(rule->mutex);
 
-    uint32_t meter_id;           /* Non-zero OF meter_id, or zero. */
-    struct list meter_list_node; /* In owning meter's 'rules' list. */
+/* A set of actions within a "struct rule".
+ *
+ *
+ * Thread-safety
+ * =============
+ *
+ * A struct rule_actions 'actions' may be accessed without a risk of being
+ * freed by code that holds a read-lock or write-lock on 'rule->mutex' (where
+ * 'rule' is the rule for which 'rule->actions == actions') or that owns a
+ * reference to 'actions->ref_count' (or both). */
+struct rule_actions {
+    atomic_uint ref_count;
+
+    /* These members are immutable: they do not change during the struct's
+     * lifetime.  */
+    struct ofpact *ofpacts;     /* Sequence of "struct ofpacts". */
+    unsigned int ofpacts_len;   /* Size of 'ofpacts', in bytes. */
+    uint32_t meter_id;          /* Non-zero OF meter_id, or zero. */
+};
+
+struct rule_actions *rule_actions_create(const struct ofpact *, size_t);
+void rule_actions_ref(struct rule_actions *);
+void rule_actions_unref(struct rule_actions *);
 
-    /* Flow monitors. */
-    enum nx_flow_monitor_flags monitor_flags;
-    uint64_t add_seqno;         /* Sequence number when added. */
-    uint64_t modify_seqno;      /* Sequence number when changed. */
+/* A set of rules to which an OpenFlow operation applies. */
+struct rule_collection {
+    struct rule **rules;        /* The rules. */
+    size_t n;                   /* Number of rules collected. */
 
-    /* Optimisation for flow expiry. */
-    struct list expirable;      /* In ofproto's 'expirable' list if this rule
-                                 * is expirable, otherwise empty. */
+    size_t capacity;            /* Number of rules that will fit in 'rules'. */
+    struct rule *stub[64];      /* Preallocated rules to avoid malloc(). */
 };
 
+void rule_collection_init(struct rule_collection *);
+void rule_collection_add(struct rule_collection *, struct rule *);
+void rule_collection_ref(struct rule_collection *) OVS_REQUIRES(ofproto_mutex);
+void rule_collection_unref(struct rule_collection *);
+void rule_collection_destroy(struct rule_collection *);
+
 /* Threshold at which to begin flow table eviction. Only affects the
  * ofproto-dpif implementation */
 extern unsigned flow_eviction_threshold;
@@ -287,21 +455,18 @@ rule_from_cls_rule(const struct cls_rule *cls_rule)
     return cls_rule ? CONTAINER_OF(cls_rule, struct rule, cr) : NULL;
 }
 
-void ofproto_rule_expire(struct rule *rule, uint8_t reason);
-void ofproto_rule_delete(struct ofproto *, struct classifier *cls,
-                         struct rule *) OVS_REQ_WRLOCK(cls->rwlock);
+void ofproto_rule_expire(struct rule *rule, uint8_t reason)
+    OVS_REQUIRES(ofproto_mutex);
+void ofproto_rule_delete(struct ofproto *, struct rule *)
+    OVS_EXCLUDED(ofproto_mutex);
 void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
                                   uint16_t hard_timeout)
-    OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex);
-
-bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
+    OVS_EXCLUDED(ofproto_mutex);
 
 void ofoperation_complete(struct ofoperation *, enum ofperr);
 
-bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port);
-bool ofproto_rule_has_out_group(const struct rule *, uint32_t group_id);
-
-bool ofproto_rule_is_hidden(const struct rule *);
+bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port)
+    OVS_REQUIRES(ofproto_mutex);
 
 /* A group within a "struct ofproto".
  *
@@ -316,7 +481,7 @@ struct ofgroup {
      * a group is ever write locked only while holding a write lock
      * on the container, the user's of groups will never face a group
      * in the write locked state. */
-    struct ovs_rwlock rwlock;
+    struct ovs_rwlock rwlock OVS_ACQ_AFTER(ofproto_mutex);
     struct hmap_node hmap_node; /* In struct ofproto's "groups" hmap. */
     struct ofproto *ofproto;    /* The ofproto that contains this group. */
     uint32_t group_id;
@@ -1096,9 +1261,10 @@ struct ofproto_class {
      *
      * Rule destruction must not fail. */
     struct rule *(*rule_alloc)(void);
-    enum ofperr (*rule_construct)(struct rule *rule);
-    void (*rule_insert)(struct rule *rule);
-    void (*rule_delete)(struct rule *rule);
+    enum ofperr (*rule_construct)(struct rule *rule)
+        /* OVS_REQUIRES(ofproto_mutex) */;
+    void (*rule_insert)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
+    void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
     void (*rule_destruct)(struct rule *rule);
     void (*rule_dealloc)(struct rule *rule);
 
@@ -1107,7 +1273,8 @@ struct ofproto_class {
      * in '*byte_count'.  UINT64_MAX indicates that the packet count or byte
      * count is unknown. */
     void (*rule_get_stats)(struct rule *rule, uint64_t *packet_count,
-                           uint64_t *byte_count);
+                           uint64_t *byte_count)
+        /* OVS_EXCLUDED(ofproto_mutex) */;
 
     /* Applies the actions in 'rule' to 'packet'.  (This implements sending
      * buffered packets for OpenFlow OFPT_FLOW_MOD commands.)
@@ -1153,7 +1320,8 @@ struct ofproto_class {
      *
      * ->rule_modify_actions() should not modify any base members of struct
      * rule. */
-    void (*rule_modify_actions)(struct rule *rule, bool reset_counters);
+    void (*rule_modify_actions)(struct rule *rule, bool reset_counters)
+        /* OVS_REQUIRES(ofproto_mutex) */;
 
     /* Changes the OpenFlow IP fragment handling policy to 'frag_handling',
      * which takes one of the following values, with the corresponding
@@ -1533,12 +1701,15 @@ int ofproto_class_unregister(const struct ofproto_class *);
 enum { OFPROTO_POSTPONE = 1 << 16 };
 BUILD_ASSERT_DECL(OFPROTO_POSTPONE < OFPERR_OFS);
 
-int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *);
+int ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *)
+    OVS_EXCLUDED(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *,
                       unsigned int priority,
-                      const struct ofpact *ofpacts, size_t ofpacts_len);
+                      const struct ofpact *ofpacts, size_t ofpacts_len)
+    OVS_EXCLUDED(ofproto_mutex);
 bool ofproto_delete_flow(struct ofproto *,
-                         const struct match *, unsigned int priority);
+                         const struct match *, unsigned int priority)
+    OVS_EXCLUDED(ofproto_mutex);
 void ofproto_flush_flows(struct ofproto *);
 
 #endif /* ofproto/ofproto-provider.h */
index 9605baa..1127900 100644 (file)
@@ -124,9 +124,7 @@ struct ofoperation {
 
     /* OFOPERATION_MODIFY, OFOPERATION_REPLACE: The old actions, if the actions
      * are changing. */
-    struct ofpact *ofpacts;
-    size_t ofpacts_len;
-    uint32_t meter_id;
+    struct rule_actions *actions;
 
     /* OFOPERATION_DELETE. */
     enum ofp_flow_removed_reason reason; /* Reason flow was removed. */
@@ -155,10 +153,9 @@ static void oftable_enable_eviction(struct oftable *,
                                     const struct mf_subfield *fields,
                                     size_t n_fields);
 
-static void oftable_remove_rule(struct rule *rule) OVS_RELEASES(rule->rwlock);
-static void oftable_remove_rule__(struct ofproto *ofproto,
-                                  struct classifier *cls, struct rule *rule)
-    OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->rwlock);
+static void oftable_remove_rule(struct rule *rule) OVS_REQUIRES(ofproto_mutex);
+static void oftable_remove_rule__(struct ofproto *, struct rule *)
+    OVS_REQUIRES(ofproto_mutex);
 static void oftable_insert_rule(struct rule *);
 
 /* A set of rules within a single OpenFlow table (oftable) that have the same
@@ -183,15 +180,60 @@ struct eviction_group {
     struct heap rules;          /* Contains "struct rule"s. */
 };
 
-static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep)
-    OVS_TRY_WRLOCK(true, (*rulep)->rwlock);
-static void ofproto_evict(struct ofproto *);
+static bool choose_rule_to_evict(struct oftable *table, struct rule **rulep);
+static void ofproto_evict(struct ofproto *) OVS_EXCLUDED(ofproto_mutex);
 static uint32_t rule_eviction_priority(struct rule *);
 static void eviction_group_add_rule(struct rule *);
 static void eviction_group_remove_rule(struct rule *);
 
+/* Criteria that flow_mod and other operations use for selecting rules on
+ * which to operate. */
+struct rule_criteria {
+    /* An OpenFlow table or 255 for all tables. */
+    uint8_t table_id;
+
+    /* OpenFlow matching criteria.  Interpreted different in "loose" way by
+     * collect_rules_loose() and "strict" way by collect_rules_strict(), as
+     * defined in the OpenFlow spec. */
+    struct cls_rule cr;
+
+    /* Matching criteria for the OpenFlow cookie.  Consider a bit B in a rule's
+     * cookie and the corresponding bits C in 'cookie' and M in 'cookie_mask'.
+     * The rule will not be selected if M is 1 and B != C.  */
+    ovs_be64 cookie;
+    ovs_be64 cookie_mask;
+
+    /* Selection based on actions within a rule:
+     *
+     * If out_port != OFPP_ANY, selects only rules that output to out_port.
+     * If out_group != OFPG_ALL, select only rules that output to out_group. */
+    ofp_port_t out_port;
+    uint32_t out_group;
+};
+
+static void rule_criteria_init(struct rule_criteria *, uint8_t table_id,
+                               const struct match *match,
+                               unsigned int priority,
+                               ovs_be64 cookie, ovs_be64 cookie_mask,
+                               ofp_port_t out_port, uint32_t out_group);
+static void rule_criteria_destroy(struct rule_criteria *);
+
+/* A packet that needs to be passed to rule_execute().
+ *
+ * (We can't do this immediately from ofopgroup_complete() because that holds
+ * ofproto_mutex, which rule_execute() needs released.) */
+struct rule_execute {
+    struct list list_node;      /* In struct ofproto's "rule_executes" list. */
+    struct rule *rule;          /* Owns a reference to the rule. */
+    ofp_port_t in_port;
+    struct ofpbuf *packet;      /* Owns the packet. */
+};
+
+static void run_rule_executes(struct ofproto *) OVS_EXCLUDED(ofproto_mutex);
+static void destroy_rule_executes(struct ofproto *);
+
 /* ofport. */
-static void ofport_destroy__(struct ofport *);
+static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
 static void ofport_destroy(struct ofport *);
 
 static void update_port(struct ofproto *, const char *devname);
@@ -199,7 +241,6 @@ static int init_ports(struct ofproto *);
 static void reinit_ports(struct ofproto *);
 
 /* rule. */
-static void ofproto_rule_destroy(struct rule *);
 static void ofproto_rule_destroy__(struct rule *);
 static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
 static bool rule_is_modifiable(const struct rule *);
@@ -210,15 +251,17 @@ static enum ofperr add_flow(struct ofproto *, struct ofconn *,
                             const struct ofp_header *);
 static enum ofperr modify_flows__(struct ofproto *, struct ofconn *,
                                   struct ofputil_flow_mod *,
-                                  const struct ofp_header *, struct list *);
+                                  const struct ofp_header *,
+                                  const struct rule_collection *);
 static void delete_flow__(struct rule *rule, struct ofopgroup *,
                           enum ofp_flow_removed_reason)
-    OVS_RELEASES(rule->rwlock);
+    OVS_REQUIRES(ofproto_mutex);
 static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *);
 static bool handle_openflow(struct ofconn *, const struct ofpbuf *);
 static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *,
                                      struct ofputil_flow_mod *,
-                                     const struct ofp_header *);
+                                     const struct ofp_header *)
+    OVS_EXCLUDED(ofproto_mutex);
 static void calc_duration(long long int start, long long int now,
                           uint32_t *sec, uint32_t *nsec);
 
@@ -237,6 +280,9 @@ static const struct ofproto_class **ofproto_classes;
 static size_t n_ofproto_classes;
 static size_t allocated_ofproto_classes;
 
+/* Global lock that protects all flow table operations. */
+struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
+
 unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
 unsigned n_handler_threads;
 enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
@@ -422,6 +468,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     }
 
     /* Initialize. */
+    ovs_mutex_lock(&ofproto_mutex);
     memset(ofproto, 0, sizeof *ofproto);
     ofproto->ofproto_class = class;
     ofproto->name = xstrdup(datapath_name);
@@ -446,12 +493,12 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->n_tables = 0;
     hindex_init(&ofproto->cookies);
     list_init(&ofproto->expirable);
-    ovs_mutex_init_recursive(&ofproto->expirable_mutex);
     ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
     ofproto->state = S_OPENFLOW;
     list_init(&ofproto->pending);
     ofproto->n_pending = 0;
     hmap_init(&ofproto->deletions);
+    guarded_list_init(&ofproto->rule_executes);
     ofproto->n_add = ofproto->n_delete = ofproto->n_modify = 0;
     ofproto->first_op = ofproto->last_op = LLONG_MIN;
     ofproto->next_op_report = LLONG_MAX;
@@ -461,6 +508,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->min_mtu = INT_MAX;
     ovs_rwlock_init(&ofproto->groups_rwlock);
     hmap_init(&ofproto->groups);
+    ovs_mutex_unlock(&ofproto_mutex);
 
     error = ofproto->ofproto_class->construct(ofproto);
     if (error) {
@@ -1080,30 +1128,49 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops)
     connmgr_get_snoops(ofproto->connmgr, snoops);
 }
 
+static void
+ofproto_rule_delete__(struct ofproto *ofproto, struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    struct ofopgroup *group;
+
+    ovs_assert(!rule->pending);
+
+    group = ofopgroup_create_unattached(ofproto);
+    delete_flow__(rule, group, OFPRR_DELETE);
+    ofopgroup_submit(group);
+}
+
 /* Deletes 'rule' from 'cls' within 'ofproto'.
  *
- * The 'cls' argument is redundant (it is &ofproto->tables[rule->table_id].cls)
- * but it allows Clang to do better checking. */
-static void
-ofproto_delete_rule(struct ofproto *ofproto, struct classifier *cls,
-                    struct rule *rule)
-    OVS_REQ_WRLOCK(cls->rwlock)
+ * Within an ofproto implementation, this function allows an ofproto
+ * implementation to destroy any rules that remain when its ->destruct()
+ * function is called.  This function is not suitable for use elsewhere in an
+ * ofproto implementation.
+ *
+ * This function implements steps 4.4 and 4.5 in the section titled "Rule Life
+ * Cycle" in ofproto-provider.h. */
+void
+ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofopgroup *group;
 
+    ovs_mutex_lock(&ofproto_mutex);
     ovs_assert(!rule->pending);
-    ovs_assert(cls == &ofproto->tables[rule->table_id].cls);
 
     group = ofopgroup_create_unattached(ofproto);
     ofoperation_create(group, rule, OFOPERATION_DELETE, OFPRR_DELETE);
-    ovs_rwlock_wrlock(&rule->rwlock);
-    oftable_remove_rule__(ofproto, cls, rule);
+    oftable_remove_rule__(ofproto, rule);
     ofproto->ofproto_class->rule_delete(rule);
     ofopgroup_submit(group);
+
+    ovs_mutex_unlock(&ofproto_mutex);
 }
 
 static void
 ofproto_flush__(struct ofproto *ofproto)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct oftable *table;
 
@@ -1111,6 +1178,7 @@ ofproto_flush__(struct ofproto *ofproto)
         ofproto->ofproto_class->flush(ofproto);
     }
 
+    ovs_mutex_lock(&ofproto_mutex);
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
         struct rule *rule, *next_rule;
         struct cls_cursor cursor;
@@ -1119,31 +1187,30 @@ ofproto_flush__(struct ofproto *ofproto)
             continue;
         }
 
-        ovs_rwlock_wrlock(&table->cls.rwlock);
+        ovs_rwlock_rdlock(&table->cls.rwlock);
         cls_cursor_init(&cursor, &table->cls, NULL);
+        ovs_rwlock_unlock(&table->cls.rwlock);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
             if (!rule->pending) {
-                ofproto_delete_rule(ofproto, &table->cls, rule);
+                ofproto_rule_delete__(ofproto, rule);
             }
         }
-        ovs_rwlock_unlock(&table->cls.rwlock);
     }
+    ovs_mutex_unlock(&ofproto_mutex);
 }
 
 static void delete_group(struct ofproto *ofproto, uint32_t group_id);
 
 static void
 ofproto_destroy__(struct ofproto *ofproto)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct oftable *table;
 
     ovs_assert(list_is_empty(&ofproto->pending));
-    ovs_assert(!ofproto->n_pending);
 
-    if (ofproto->meters) {
-        meter_delete(ofproto, 1, ofproto->meter_features.max_meters);
-        free(ofproto->meters);
-    }
+    destroy_rule_executes(ofproto);
+    guarded_list_destroy(&ofproto->rule_executes);
 
     delete_group(ofproto, OFPG_ALL);
     ovs_rwlock_destroy(&ofproto->groups_rwlock);
@@ -1173,12 +1240,12 @@ ofproto_destroy__(struct ofproto *ofproto)
 
     free(ofproto->vlan_bitmap);
 
-    ovs_mutex_destroy(&ofproto->expirable_mutex);
     ofproto->ofproto_class->dealloc(ofproto);
 }
 
 void
 ofproto_destroy(struct ofproto *p)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofport *ofport, *next_ofport;
 
@@ -1186,6 +1253,13 @@ ofproto_destroy(struct ofproto *p)
         return;
     }
 
+    if (p->meters) {
+        meter_delete(p, 1, p->meter_features.max_meters);
+        p->meter_features.max_meters = 0;
+        free(p->meters);
+        p->meters = NULL;
+    }
+
     ofproto_flush__(p);
     HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) {
         ofport_destroy(ofport);
@@ -1268,6 +1342,19 @@ ofproto_type_wait(const char *datapath_type)
     }
 }
 
+static bool
+any_pending_ops(const struct ofproto *p)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    bool b;
+
+    ovs_mutex_lock(&ofproto_mutex);
+    b = !list_is_empty(&p->pending);
+    ovs_mutex_unlock(&ofproto_mutex);
+
+    return b;
+}
+
 int
 ofproto_run(struct ofproto *p)
 {
@@ -1281,6 +1368,8 @@ ofproto_run(struct ofproto *p)
         VLOG_ERR_RL(&rl, "%s: run failed (%s)", p->name, ovs_strerror(error));
     }
 
+    run_rule_executes(p);
+
     /* Restore the eviction group heap invariant occasionally. */
     if (p->eviction_group_timer < time_msec()) {
         size_t i;
@@ -1297,6 +1386,7 @@ ofproto_run(struct ofproto *p)
                 continue;
             }
 
+            ovs_mutex_lock(&ofproto_mutex);
             HEAP_FOR_EACH (evg, size_node, &table->eviction_groups_by_size) {
                 heap_rebuild(&evg->rules);
             }
@@ -1310,6 +1400,7 @@ ofproto_run(struct ofproto *p)
                 }
             }
             ovs_rwlock_unlock(&table->cls.rwlock);
+            ovs_mutex_unlock(&ofproto_mutex);
         }
     }
 
@@ -1348,7 +1439,7 @@ ofproto_run(struct ofproto *p)
     case S_EVICT:
         connmgr_run(p->connmgr, NULL);
         ofproto_evict(p);
-        if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) {
+        if (!any_pending_ops(p)) {
             p->state = S_OPENFLOW;
         }
         break;
@@ -1356,7 +1447,7 @@ ofproto_run(struct ofproto *p)
     case S_FLUSH:
         connmgr_run(p->connmgr, NULL);
         ofproto_flush__(p);
-        if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) {
+        if (!any_pending_ops(p)) {
             connmgr_flushed(p->connmgr);
             p->state = S_OPENFLOW;
         }
@@ -1449,7 +1540,7 @@ ofproto_wait(struct ofproto *p)
     case S_EVICT:
     case S_FLUSH:
         connmgr_wait(p->connmgr, false);
-        if (list_is_empty(&p->pending) && hmap_is_empty(&p->deletions)) {
+        if (!any_pending_ops(p)) {
             poll_immediate_wake();
         }
         break;
@@ -1471,8 +1562,11 @@ ofproto_get_memory_usage(const struct ofproto *ofproto, struct simap *usage)
     unsigned int n_rules;
 
     simap_increase(usage, "ports", hmap_count(&ofproto->ports));
+
+    ovs_mutex_lock(&ofproto_mutex);
     simap_increase(usage, "ops",
                    ofproto->n_pending + hmap_count(&ofproto->deletions));
+    ovs_mutex_unlock(&ofproto_mutex);
 
     n_rules = 0;
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
@@ -1687,6 +1781,34 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port)
     return error;
 }
 
+static int
+simple_flow_mod(struct ofproto *ofproto,
+                const struct match *match, unsigned int priority,
+                const struct ofpact *ofpacts, size_t ofpacts_len,
+                enum ofp_flow_mod_command command)
+{
+    struct ofputil_flow_mod fm;
+
+    memset(&fm, 0, sizeof fm);
+    fm.match = *match;
+    fm.priority = priority;
+    fm.cookie = 0;
+    fm.new_cookie = 0;
+    fm.modify_cookie = false;
+    fm.table_id = 0;
+    fm.command = command;
+    fm.idle_timeout = 0;
+    fm.hard_timeout = 0;
+    fm.buffer_id = UINT32_MAX;
+    fm.out_port = OFPP_ANY;
+    fm.out_group = OFPG_ANY;
+    fm.flags = 0;
+    fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts);
+    fm.ofpacts_len = ofpacts_len;
+
+    return handle_flow_mod__(ofproto, NULL, &fm, NULL);
+}
+
 /* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
  * performs the 'n_actions' actions in 'actions'.  The new flow will not
  * timeout.
@@ -1702,25 +1824,34 @@ void
 ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
                  unsigned int priority,
                  const struct ofpact *ofpacts, size_t ofpacts_len)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     const struct rule *rule;
+    bool must_add;
 
+    /* First do a cheap check whether the rule we're looking for already exists
+     * with the actions that we want.  If it does, then we're done. */
     ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
     rule = rule_from_cls_rule(classifier_find_match_exactly(
                                   &ofproto->tables[0].cls, match, priority));
+    if (rule) {
+        ovs_mutex_lock(&rule->mutex);
+        must_add = !ofpacts_equal(rule->actions->ofpacts,
+                                  rule->actions->ofpacts_len,
+                                  ofpacts, ofpacts_len);
+        ovs_mutex_unlock(&rule->mutex);
+    } else {
+        must_add = true;
+    }
     ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
-    if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
-                                ofpacts, ofpacts_len)) {
-        struct ofputil_flow_mod fm;
 
-        memset(&fm, 0, sizeof fm);
-        fm.match = *match;
-        fm.priority = priority;
-        fm.buffer_id = UINT32_MAX;
-        fm.ofpacts = xmemdup(ofpacts, ofpacts_len);
-        fm.ofpacts_len = ofpacts_len;
-        add_flow(ofproto, NULL, &fm, NULL);
-        free(fm.ofpacts);
+    /* If there's no such rule or the rule doesn't have the actions we want,
+     * fall back to a executing a full flow mod.  We can't optimize this at
+     * all because we didn't take enough locks above to ensure that the flow
+     * table didn't already change beneath us.  */
+    if (must_add) {
+        simple_flow_mod(ofproto, match, priority, ofpacts, ofpacts_len,
+                        OFPFC_MODIFY_STRICT);
     }
 }
 
@@ -1728,9 +1859,11 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
  * OFPERR_* OpenFlow error code on failure, or OFPROTO_POSTPONE if the
  * operation cannot be initiated now but may be retried later.
  *
- * This is a helper function for in-band control and fail-open. */
+ * This is a helper function for in-band control and fail-open and the "learn"
+ * action. */
 int
 ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     return handle_flow_mod__(ofproto, NULL, fm, NULL);
 }
@@ -1742,30 +1875,26 @@ ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
 bool
 ofproto_delete_flow(struct ofproto *ofproto,
                     const struct match *target, unsigned int priority)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct classifier *cls = &ofproto->tables[0].cls;
     struct rule *rule;
 
+    /* First do a cheap check whether the rule we're looking for has already
+     * been deleted.  If so, then we're done. */
     ovs_rwlock_rdlock(&cls->rwlock);
     rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target,
                                                             priority));
     ovs_rwlock_unlock(&cls->rwlock);
     if (!rule) {
-        /* No such rule -> success. */
-        return true;
-    } else if (rule->pending) {
-        /* An operation on the rule is already pending -> failure.
-         * Caller must retry later if it's important. */
-        return false;
-    } else {
-        /* Initiate deletion -> success. */
-        ovs_rwlock_wrlock(&cls->rwlock);
-        ofproto_delete_rule(ofproto, cls, rule);
-        ovs_rwlock_unlock(&cls->rwlock);
-
         return true;
     }
 
+    /* Fall back to a executing a full flow mod.  We can't optimize this at all
+     * because we didn't take enough locks above to ensure that the flow table
+     * didn't already change beneath us.  */
+    return simple_flow_mod(ofproto, target, priority, NULL, 0,
+                           OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE;
 }
 
 /* Starts the process of deleting all of the flows from all of ofproto's flow
@@ -2256,61 +2385,134 @@ update_mtu(struct ofproto *p, struct ofport *port)
     }
 }
 \f
-static void
-ofproto_rule_destroy(struct rule *rule)
+void
+ofproto_rule_ref(struct rule *rule)
 {
     if (rule) {
-        rule->ofproto->ofproto_class->rule_destruct(rule);
-        ofproto_rule_destroy__(rule);
+        unsigned int orig;
+
+        atomic_add(&rule->ref_count, 1, &orig);
+        ovs_assert(orig != 0);
     }
 }
 
+void
+ofproto_rule_unref(struct rule *rule)
+{
+    if (rule) {
+        unsigned int orig;
+
+        atomic_sub(&rule->ref_count, 1, &orig);
+        if (orig == 1) {
+            rule->ofproto->ofproto_class->rule_destruct(rule);
+            ofproto_rule_destroy__(rule);
+        } else {
+            ovs_assert(orig != 0);
+        }
+    }
+}
+
+struct rule_actions *
+rule_get_actions(const struct rule *rule)
+    OVS_EXCLUDED(rule->mutex)
+{
+    struct rule_actions *actions;
+
+    ovs_mutex_lock(&rule->mutex);
+    actions = rule_get_actions__(rule);
+    ovs_mutex_unlock(&rule->mutex);
+
+    return actions;
+}
+
+struct rule_actions *
+rule_get_actions__(const struct rule *rule)
+    OVS_REQUIRES(rule->mutex)
+{
+    rule_actions_ref(rule->actions);
+    return rule->actions;
+}
+
 static void
 ofproto_rule_destroy__(struct rule *rule)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
-    cls_rule_destroy(&rule->cr);
-    free(rule->ofpacts);
-    ovs_mutex_destroy(&rule->timeout_mutex);
-    ovs_rwlock_destroy(&rule->rwlock);
+    cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
+    rule_actions_unref(rule->actions);
+    ovs_mutex_destroy(&rule->mutex);
     rule->ofproto->ofproto_class->rule_dealloc(rule);
 }
 
-/* This function allows an ofproto implementation to destroy any rules that
- * remain when its ->destruct() function is called..  This function implements
- * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
- * ofproto-provider.h.
- *
- * This function should only be called from an ofproto implementation's
- * ->destruct() function.  It is not suitable elsewhere. */
+/* Creates and returns a new 'struct rule_actions', with a ref_count of 1,
+ * whose actions are a copy of from the 'ofpacts_len' bytes of 'ofpacts'. */
+struct rule_actions *
+rule_actions_create(const struct ofpact *ofpacts, size_t ofpacts_len)
+{
+    struct rule_actions *actions;
+
+    actions = xmalloc(sizeof *actions);
+    atomic_init(&actions->ref_count, 1);
+    actions->ofpacts = xmemdup(ofpacts, ofpacts_len);
+    actions->ofpacts_len = ofpacts_len;
+    actions->meter_id = ofpacts_get_meter(ofpacts, ofpacts_len);
+    return actions;
+}
+
+/* Increments 'actions''s ref_count. */
 void
-ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls,
-                    struct rule *rule)
-    OVS_REQ_WRLOCK(cls->rwlock)
+rule_actions_ref(struct rule_actions *actions)
 {
-    ofproto_delete_rule(ofproto, cls, rule);
+    if (actions) {
+        unsigned int orig;
+
+        atomic_add(&actions->ref_count, 1, &orig);
+        ovs_assert(orig != 0);
+    }
+}
+
+/* Decrements 'actions''s ref_count and frees 'actions' if the ref_count
+ * reaches 0. */
+void
+rule_actions_unref(struct rule_actions *actions)
+{
+    if (actions) {
+        unsigned int orig;
+
+        atomic_sub(&actions->ref_count, 1, &orig);
+        if (orig == 1) {
+            free(actions);
+        } else {
+            ovs_assert(orig != 0);
+        }
+    }
 }
 
 /* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
  * that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
-bool
+static bool
 ofproto_rule_has_out_port(const struct rule *rule, ofp_port_t port)
+    OVS_REQUIRES(ofproto_mutex)
 {
     return (port == OFPP_ANY
-            || ofpacts_output_to_port(rule->ofpacts, rule->ofpacts_len, port));
+            || ofpacts_output_to_port(rule->actions->ofpacts,
+                                      rule->actions->ofpacts_len, port));
 }
 
 /* Returns true if 'rule' has group and equals group_id. */
-bool
+static bool
 ofproto_rule_has_out_group(const struct rule *rule, uint32_t group_id)
+    OVS_REQUIRES(ofproto_mutex)
 {
     return (group_id == OFPG11_ANY
-            || ofpacts_output_to_group(rule->ofpacts, rule->ofpacts_len, group_id));
+            || ofpacts_output_to_group(rule->actions->ofpacts,
+                                       rule->actions->ofpacts_len, group_id));
 }
 
 /* Returns true if a rule related to 'op' has an OpenFlow OFPAT_OUTPUT or
  * OFPAT_ENQUEUE action that outputs to 'out_port'. */
 bool
 ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (ofproto_rule_has_out_port(op->rule, out_port)) {
         return true;
@@ -2323,28 +2525,56 @@ ofoperation_has_out_port(const struct ofoperation *op, ofp_port_t out_port)
 
     case OFOPERATION_MODIFY:
     case OFOPERATION_REPLACE:
-        return ofpacts_output_to_port(op->ofpacts, op->ofpacts_len, out_port);
+        return ofpacts_output_to_port(op->actions->ofpacts,
+                                      op->actions->ofpacts_len, out_port);
     }
 
     NOT_REACHED();
 }
 
-/* Executes the actions indicated by 'rule' on 'packet' and credits 'rule''s
- * statistics appropriately.
- *
- * 'packet' doesn't necessarily have to match 'rule'.  'rule' will be credited
- * with statistics for 'packet' either way.
- *
- * Takes ownership of 'packet'. */
-static int
-rule_execute(struct rule *rule, ofp_port_t in_port, struct ofpbuf *packet)
+static void
+rule_execute_destroy(struct rule_execute *e)
 {
-    struct flow flow;
-    union flow_in_port in_port_;
+    ofproto_rule_unref(e->rule);
+    list_remove(&e->list_node);
+    free(e);
+}
+
+/* Executes all "rule_execute" operations queued up in ofproto->rule_executes,
+ * by passing them to the ofproto provider. */
+static void
+run_rule_executes(struct ofproto *ofproto)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    struct rule_execute *e, *next;
+    struct list executes;
+
+    guarded_list_pop_all(&ofproto->rule_executes, &executes);
+    LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
+        union flow_in_port in_port_;
+        struct flow flow;
+
+        in_port_.ofp_port = e->in_port;
+        flow_extract(e->packet, 0, 0, NULL, &in_port_, &flow);
+        ofproto->ofproto_class->rule_execute(e->rule, &flow, e->packet);
+
+        rule_execute_destroy(e);
+    }
+}
 
-    in_port_.ofp_port = in_port;
-    flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
-    return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet);
+/* Destroys and discards all "rule_execute" operations queued up in
+ * ofproto->rule_executes. */
+static void
+destroy_rule_executes(struct ofproto *ofproto)
+{
+    struct rule_execute *e, *next;
+    struct list executes;
+
+    guarded_list_pop_all(&ofproto->rule_executes, &executes);
+    LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
+        ofpbuf_delete(e->packet);
+        rule_execute_destroy(e);
+    }
 }
 
 /* Returns true if 'rule' should be hidden from the controller.
@@ -2352,7 +2582,7 @@ rule_execute(struct rule *rule, ofp_port_t in_port, struct ofpbuf *packet)
  * Rules with priority higher than UINT16_MAX are set up by ofproto itself
  * (e.g. by in-band control) and are intentionally hidden from the
  * controller. */
-bool
+static bool
 ofproto_rule_is_hidden(const struct rule *rule)
 {
     return rule->cr.priority > UINT16_MAX;
@@ -2495,30 +2725,6 @@ reject_slave_controller(struct ofconn *ofconn)
     }
 }
 
-/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
- * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
- *
- * This function relies on the order of 'ofpacts' being correct (as checked by
- * ofpacts_verify()). */
-static uint32_t
-find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
-{
-    const struct ofpact *a;
-
-    OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
-        enum ovs_instruction_type inst;
-
-        inst = ovs_instruction_type_from_ofpact_type(a->type);
-        if (a->type == OFPACT_METER) {
-            return ofpact_get_METER(a)->meter_id;
-        } else if (inst > OVSINST_OFPIT13_METER) {
-            break;
-        }
-    }
-
-    return 0;
-}
-
 /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate
  * for a packet with the prerequisites satisfied by 'flow' in table 'table_id'.
  * 'flow' may be temporarily modified, but is restored at return.
@@ -2537,7 +2743,7 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
         return error;
     }
 
-    mid = find_meter(ofpacts, ofpacts_len);
+    mid = ofpacts_get_meter(ofpacts, ofpacts_len);
     if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
         return OFPERR_OFPMMFC_INVALID_METER;
     }
@@ -2711,12 +2917,12 @@ handle_table_stats_request(struct ofconn *ofconn,
     for (i = 0; i < p->n_tables; i++) {
         ots[i].table_id = i;
         sprintf(ots[i].name, "table%zu", i);
-        ots[i].match = htonll(OFPXMT12_MASK);
-        ots[i].wildcards = htonll(OFPXMT12_MASK);
+        ots[i].match = htonll(OFPXMT13_MASK);
+        ots[i].wildcards = htonll(OFPXMT13_MASK);
         ots[i].write_actions = htonl(OFPAT11_OUTPUT);
         ots[i].apply_actions = htonl(OFPAT11_OUTPUT);
-        ots[i].write_setfields = htonll(OFPXMT12_MASK);
-        ots[i].apply_setfields = htonll(OFPXMT12_MASK);
+        ots[i].write_setfields = htonll(OFPXMT13_MASK);
+        ots[i].apply_setfields = htonll(OFPXMT13_MASK);
         ots[i].metadata_match = htonll(UINT64_MAX);
         ots[i].metadata_write = htonll(UINT64_MAX);
         ots[i].instructions = htonl(OFPIT11_ALL);
@@ -2832,6 +3038,7 @@ hash_cookie(ovs_be64 cookie)
 
 static void
 cookies_insert(struct ofproto *ofproto, struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     hindex_insert(&ofproto->cookies, &rule->cookie_node,
                   hash_cookie(rule->flow_cookie));
@@ -2839,6 +3046,7 @@ cookies_insert(struct ofproto *ofproto, struct rule *rule)
 
 static void
 cookies_remove(struct ofproto *ofproto, struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     hindex_remove(&ofproto->cookies, &rule->cookie_node);
 }
@@ -2846,13 +3054,14 @@ cookies_remove(struct ofproto *ofproto, struct rule *rule)
 static void
 ofproto_rule_change_cookie(struct ofproto *ofproto, struct rule *rule,
                            ovs_be64 new_cookie)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (new_cookie != rule->flow_cookie) {
         cookies_remove(ofproto, rule);
 
-        ovs_rwlock_wrlock(&rule->rwlock);
+        ovs_mutex_lock(&rule->mutex);
         rule->flow_cookie = new_cookie;
-        ovs_rwlock_unlock(&rule->rwlock);
+        ovs_mutex_unlock(&rule->mutex);
 
         cookies_insert(ofproto, rule);
     }
@@ -2935,169 +3144,229 @@ next_matching_table(const struct ofproto *ofproto,
          (TABLE) != NULL;                                         \
          (TABLE) = next_matching_table(OFPROTO, TABLE, TABLE_ID))
 
-/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
- * 'table_id' is 0xff) that match 'match' in the "loose" way required for
- * OpenFlow OFPFC_MODIFY and OFPFC_DELETE requests and puts them on list
- * 'rules'.
+/* Initializes 'criteria' in a straightforward way based on the other
+ * parameters.
  *
- * If 'out_port' is anything other than OFPP_ANY, then only rules that output
- * to 'out_port' are included.
+ * For "loose" matching, the 'priority' parameter is unimportant and may be
+ * supplied as 0. */
+static void
+rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id,
+                   const struct match *match, unsigned int priority,
+                   ovs_be64 cookie, ovs_be64 cookie_mask,
+                   ofp_port_t out_port, uint32_t out_group)
+{
+    criteria->table_id = table_id;
+    cls_rule_init(&criteria->cr, match, priority);
+    criteria->cookie = cookie;
+    criteria->cookie_mask = cookie_mask;
+    criteria->out_port = out_port;
+    criteria->out_group = out_group;
+}
+
+static void
+rule_criteria_destroy(struct rule_criteria *criteria)
+{
+    cls_rule_destroy(&criteria->cr);
+}
+
+void
+rule_collection_init(struct rule_collection *rules)
+{
+    rules->rules = rules->stub;
+    rules->n = 0;
+    rules->capacity = ARRAY_SIZE(rules->stub);
+}
+
+void
+rule_collection_add(struct rule_collection *rules, struct rule *rule)
+{
+    if (rules->n >= rules->capacity) {
+        size_t old_size, new_size;
+
+        old_size = rules->capacity * sizeof *rules->rules;
+        rules->capacity *= 2;
+        new_size = rules->capacity * sizeof *rules->rules;
+
+        if (rules->rules == rules->stub) {
+            rules->rules = xmalloc(new_size);
+            memcpy(rules->rules, rules->stub, old_size);
+        } else {
+            rules->rules = xrealloc(rules->rules, new_size);
+        }
+    }
+
+    rules->rules[rules->n++] = rule;
+}
+
+void
+rule_collection_ref(struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    size_t i;
+
+    for (i = 0; i < rules->n; i++) {
+        ofproto_rule_ref(rules->rules[i]);
+    }
+}
+
+void
+rule_collection_unref(struct rule_collection *rules)
+{
+    size_t i;
+
+    for (i = 0; i < rules->n; i++) {
+        ofproto_rule_unref(rules->rules[i]);
+    }
+}
+
+void
+rule_collection_destroy(struct rule_collection *rules)
+{
+    if (rules->rules != rules->stub) {
+        free(rules->rules);
+    }
+}
+
+static enum ofperr
+collect_rule(struct rule *rule, const struct rule_criteria *c,
+             struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    if (ofproto_rule_is_hidden(rule)) {
+        return 0;
+    } else if (rule->pending) {
+        return OFPROTO_POSTPONE;
+    } else {
+        if ((c->table_id == rule->table_id || c->table_id == 0xff)
+            && ofproto_rule_has_out_port(rule, c->out_port)
+            && ofproto_rule_has_out_group(rule, c->out_group)
+            && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask)) {
+            rule_collection_add(rules, rule);
+        }
+        return 0;
+    }
+}
+
+/* Searches 'ofproto' for rules that match the criteria in 'criteria'.  Matches
+ * on classifiers rules are done in the "loose" way required for OpenFlow
+ * OFPFC_MODIFY and OFPFC_DELETE requests.  Puts the selected rules on list
+ * 'rules'.
  *
  * Hidden rules are always omitted.
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static enum ofperr
-collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
-                    const struct match *match,
-                    ovs_be64 cookie, ovs_be64 cookie_mask,
-                    ofp_port_t out_port, uint32_t out_group,
-                    struct list *rules)
+collect_rules_loose(struct ofproto *ofproto,
+                    const struct rule_criteria *criteria,
+                    struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct oftable *table;
-    struct cls_rule cr;
     enum ofperr error;
 
-    error = check_table_id(ofproto, table_id);
+    rule_collection_init(rules);
+
+    error = check_table_id(ofproto, criteria->table_id);
     if (error) {
-        return error;
+        goto exit;
     }
 
-    list_init(rules);
-    cls_rule_init(&cr, match, 0);
-
-    if (cookie_mask == htonll(UINT64_MAX)) {
+    if (criteria->cookie_mask == htonll(UINT64_MAX)) {
         struct rule *rule;
 
-        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
+        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node,
+                                   hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
-            if (table_id != rule->table_id && table_id != 0xff) {
-                continue;
-            }
-            if (ofproto_rule_is_hidden(rule)) {
-                continue;
-            }
-            if (cls_rule_is_loose_match(&rule->cr, &cr.match)) {
-                if (rule->pending) {
-                    error = OFPROTO_POSTPONE;
-                    goto exit;
-                }
-                if (rule->flow_cookie == cookie /* Hash collisions possible. */
-                    && ofproto_rule_has_out_port(rule, out_port)
-                    && ofproto_rule_has_out_group(rule, out_group)) {
-                    list_push_back(rules, &rule->ofproto_node);
+            if (cls_rule_is_loose_match(&rule->cr, &criteria->cr.match)) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
                 }
             }
         }
-        goto exit;
-    }
-
-    FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
-        struct cls_cursor cursor;
-        struct rule *rule;
+    } else {
+        FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) {
+            struct cls_cursor cursor;
+            struct rule *rule;
 
-        ovs_rwlock_rdlock(&table->cls.rwlock);
-        cls_cursor_init(&cursor, &table->cls, &cr);
-        CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-            if (rule->pending) {
-                ovs_rwlock_unlock(&table->cls.rwlock);
-                error = OFPROTO_POSTPONE;
-                goto exit;
-            }
-            if (!ofproto_rule_is_hidden(rule)
-                && ofproto_rule_has_out_port(rule, out_port)
-                && ofproto_rule_has_out_group(rule, out_group)
-                    && !((rule->flow_cookie ^ cookie) & cookie_mask)) {
-                list_push_back(rules, &rule->ofproto_node);
+            ovs_rwlock_rdlock(&table->cls.rwlock);
+            cls_cursor_init(&cursor, &table->cls, &criteria->cr);
+            CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
+                }
             }
+            ovs_rwlock_unlock(&table->cls.rwlock);
         }
-        ovs_rwlock_unlock(&table->cls.rwlock);
     }
 
 exit:
-    cls_rule_destroy(&cr);
+    if (error) {
+        rule_collection_destroy(rules);
+    }
     return error;
 }
 
-/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
- * 'table_id' is 0xff) that match 'match' in the "strict" way required for
- * OpenFlow OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests and puts them
- * on list 'rules'.
- *
- * If 'out_port' is anything other than OFPP_ANY, then only rules that output
- * to 'out_port' are included.
+/* Searches 'ofproto' for rules that match the criteria in 'criteria'.  Matches
+ * on classifiers rules are done in the "strict" way required for OpenFlow
+ * OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests.  Puts the selected
+ * rules on list 'rules'.
  *
  * Hidden rules are always omitted.
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static enum ofperr
-collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
-                     const struct match *match, unsigned int priority,
-                     ovs_be64 cookie, ovs_be64 cookie_mask,
-                     ofp_port_t out_port, uint32_t out_group,
-                     struct list *rules)
+collect_rules_strict(struct ofproto *ofproto,
+                     const struct rule_criteria *criteria,
+                     struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct oftable *table;
-    struct cls_rule cr;
     int error;
 
-    error = check_table_id(ofproto, table_id);
+    rule_collection_init(rules);
+
+    error = check_table_id(ofproto, criteria->table_id);
     if (error) {
-        return error;
+        goto exit;
     }
 
-    list_init(rules);
-    cls_rule_init(&cr, match, priority);
-
-    if (cookie_mask == htonll(UINT64_MAX)) {
+    if (criteria->cookie_mask == htonll(UINT64_MAX)) {
         struct rule *rule;
 
-        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
+        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node,
+                                   hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
-            if (table_id != rule->table_id && table_id != 0xff) {
-                continue;
-            }
-            if (ofproto_rule_is_hidden(rule)) {
-                continue;
-            }
-            if (cls_rule_equal(&rule->cr, &cr)) {
-                if (rule->pending) {
-                    error = OFPROTO_POSTPONE;
-                    goto exit;
-                }
-                if (rule->flow_cookie == cookie /* Hash collisions possible. */
-                    && ofproto_rule_has_out_port(rule, out_port)
-                    && ofproto_rule_has_out_group(rule, out_group)) {
-                    list_push_back(rules, &rule->ofproto_node);
+            if (cls_rule_equal(&rule->cr, &criteria->cr)) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
                 }
             }
         }
-        goto exit;
-    }
-
-    FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
-        struct rule *rule;
+    } else {
+        FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) {
+            struct rule *rule;
 
-        ovs_rwlock_rdlock(&table->cls.rwlock);
-        rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls,
-                                                               &cr));
-        ovs_rwlock_unlock(&table->cls.rwlock);
-        if (rule) {
-            if (rule->pending) {
-                error = OFPROTO_POSTPONE;
-                goto exit;
-            }
-            if (!ofproto_rule_is_hidden(rule)
-                && ofproto_rule_has_out_port(rule, out_port)
-                && ofproto_rule_has_out_group(rule, out_group)
-                    && !((rule->flow_cookie ^ cookie) & cookie_mask)) {
-                list_push_back(rules, &rule->ofproto_node);
+            ovs_rwlock_rdlock(&table->cls.rwlock);
+            rule = rule_from_cls_rule(classifier_find_rule_exactly(
+                                          &table->cls, &criteria->cr));
+            ovs_rwlock_unlock(&table->cls.rwlock);
+            if (rule) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
+                }
             }
         }
     }
 
 exit:
-    cls_rule_destroy(&cr);
-    return 0;
+    if (error) {
+        rule_collection_destroy(rules);
+    }
+    return error;
 }
 
 /* Returns 'age_ms' (a duration in milliseconds), converted to seconds and
@@ -3113,52 +3382,76 @@ age_secs(long long int age_ms)
 static enum ofperr
 handle_flow_stats_request(struct ofconn *ofconn,
                           const struct ofp_header *request)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_stats_request fsr;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     struct list replies;
-    struct list rules;
-    struct rule *rule;
     enum ofperr error;
+    size_t i;
 
     error = ofputil_decode_flow_stats_request(&fsr, request);
     if (error) {
         return error;
     }
 
-    error = collect_rules_loose(ofproto, fsr.table_id, &fsr.match,
-                                fsr.cookie, fsr.cookie_mask,
-                                fsr.out_port, fsr.out_group, &rules);
+    rule_criteria_init(&criteria, fsr.table_id, &fsr.match, 0, fsr.cookie,
+                       fsr.cookie_mask, fsr.out_port, fsr.out_group);
+
+    ovs_mutex_lock(&ofproto_mutex);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+    if (!error) {
+        rule_collection_ref(&rules);
+    }
+    ovs_mutex_unlock(&ofproto_mutex);
+
     if (error) {
         return error;
     }
 
     ofpmp_init(&replies, request);
-    LIST_FOR_EACH (rule, ofproto_node, &rules) {
+    for (i = 0; i < rules.n; i++) {
+        struct rule *rule = rules.rules[i];
         long long int now = time_msec();
         struct ofputil_flow_stats fs;
+        long long int created, used, modified;
+        struct rule_actions *actions;
+        enum ofputil_flow_mod_flags flags;
 
-        minimatch_expand(&rule->cr.match, &fs.match);
-        fs.priority = rule->cr.priority;
+        ovs_mutex_lock(&rule->mutex);
         fs.cookie = rule->flow_cookie;
-        fs.table_id = rule->table_id;
-        calc_duration(rule->created, now, &fs.duration_sec, &fs.duration_nsec);
-        fs.idle_age = age_secs(now - rule->used);
-        fs.hard_age = age_secs(now - rule->modified);
-        ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
-                                               &fs.byte_count);
-        fs.ofpacts = rule->ofpacts;
-        fs.ofpacts_len = rule->ofpacts_len;
-
-        ovs_mutex_lock(&rule->timeout_mutex);
         fs.idle_timeout = rule->idle_timeout;
         fs.hard_timeout = rule->hard_timeout;
-        ovs_mutex_unlock(&rule->timeout_mutex);
+        created = rule->created;
+        used = rule->used;
+        modified = rule->modified;
+        actions = rule_get_actions__(rule);
+        flags = rule->flags;
+        ovs_mutex_unlock(&rule->mutex);
 
-        fs.flags = rule->flags;
+        minimatch_expand(&rule->cr.match, &fs.match);
+        fs.table_id = rule->table_id;
+        calc_duration(created, now, &fs.duration_sec, &fs.duration_nsec);
+        fs.priority = rule->cr.priority;
+        fs.idle_age = age_secs(now - used);
+        fs.hard_age = age_secs(now - modified);
+        ofproto->ofproto_class->rule_get_stats(rule, &fs.packet_count,
+                                               &fs.byte_count);
+        fs.ofpacts = actions->ofpacts;
+        fs.ofpacts_len = actions->ofpacts_len;
 
+        fs.flags = flags;
         ofputil_append_flow_stats_reply(&fs, &replies);
+
+        rule_actions_unref(actions);
     }
+
+    rule_collection_unref(&rules);
+    rule_collection_destroy(&rules);
+
     ofconn_send_replies(ofconn, &replies);
 
     return 0;
@@ -3168,22 +3461,32 @@ static void
 flow_stats_ds(struct rule *rule, struct ds *results)
 {
     uint64_t packet_count, byte_count;
+    struct rule_actions *actions;
+    long long int created;
 
     rule->ofproto->ofproto_class->rule_get_stats(rule,
                                                  &packet_count, &byte_count);
 
+    ovs_mutex_lock(&rule->mutex);
+    actions = rule_get_actions__(rule);
+    created = rule->created;
+    ovs_mutex_unlock(&rule->mutex);
+
     if (rule->table_id != 0) {
         ds_put_format(results, "table_id=%"PRIu8", ", rule->table_id);
     }
-    ds_put_format(results, "duration=%llds, ",
-                  (time_msec() - rule->created) / 1000);
+    ds_put_format(results, "duration=%llds, ", (time_msec() - created) / 1000);
     ds_put_format(results, "priority=%u, ", rule->cr.priority);
     ds_put_format(results, "n_packets=%"PRIu64", ", packet_count);
     ds_put_format(results, "n_bytes=%"PRIu64", ", byte_count);
     cls_rule_format(&rule->cr, results);
     ds_put_char(results, ',');
-    ofpacts_format(rule->ofpacts, rule->ofpacts_len, results);
+
+    ofpacts_format(actions->ofpacts, actions->ofpacts_len, results);
+
     ds_put_cstr(results, "\n");
+
+    rule_actions_unref(actions);
 }
 
 /* Adds a pretty-printed description of all flows to 'results', including
@@ -3234,31 +3537,43 @@ ofproto_port_get_cfm_status(const struct ofproto *ofproto, ofp_port_t ofp_port,
 static enum ofperr
 handle_aggregate_stats_request(struct ofconn *ofconn,
                                const struct ofp_header *oh)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_stats_request request;
     struct ofputil_aggregate_stats stats;
     bool unknown_packets, unknown_bytes;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     struct ofpbuf *reply;
-    struct list rules;
-    struct rule *rule;
     enum ofperr error;
+    size_t i;
 
     error = ofputil_decode_flow_stats_request(&request, oh);
     if (error) {
         return error;
     }
 
-    error = collect_rules_loose(ofproto, request.table_id, &request.match,
-                                request.cookie, request.cookie_mask,
-                                request.out_port, request.out_group, &rules);
+    rule_criteria_init(&criteria, request.table_id, &request.match, 0,
+                       request.cookie, request.cookie_mask,
+                       request.out_port, request.out_group);
+
+    ovs_mutex_lock(&ofproto_mutex);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+    if (!error) {
+        rule_collection_ref(&rules);
+    }
+    ovs_mutex_unlock(&ofproto_mutex);
+
     if (error) {
         return error;
     }
 
     memset(&stats, 0, sizeof stats);
     unknown_packets = unknown_bytes = false;
-    LIST_FOR_EACH (rule, ofproto_node, &rules) {
+    for (i = 0; i < rules.n; i++) {
+        struct rule *rule = rules.rules[i];
         uint64_t packet_count;
         uint64_t byte_count;
 
@@ -3286,6 +3601,9 @@ handle_aggregate_stats_request(struct ofconn *ofconn,
         stats.byte_count = UINT64_MAX;
     }
 
+    rule_collection_unref(&rules);
+    rule_collection_destroy(&rules);
+
     reply = ofputil_encode_aggregate_stats_reply(&stats, oh);
     ofconn_send_reply(ofconn, reply);
 
@@ -3394,6 +3712,7 @@ static bool
 is_flow_deletion_pending(const struct ofproto *ofproto,
                          const struct cls_rule *cls_rule,
                          uint8_t table_id)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (!hmap_is_empty(&ofproto->deletions)) {
         struct ofoperation *op;
@@ -3410,32 +3729,34 @@ is_flow_deletion_pending(const struct ofproto *ofproto,
     return false;
 }
 
-static enum ofperr
-evict_rule_from_table(struct ofproto *ofproto, struct oftable *table)
+static bool
+should_evict_a_rule(struct oftable *table, unsigned int extra_space)
+    OVS_REQUIRES(ofproto_mutex)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
-    struct rule *rule;
-    size_t n_rules;
-
-    ovs_rwlock_rdlock(&table->cls.rwlock);
-    n_rules = classifier_count(&table->cls);
-    ovs_rwlock_unlock(&table->cls.rwlock);
-
-    if (n_rules < table->max_flows) {
-        return 0;
-    } else if (!choose_rule_to_evict(table, &rule)) {
-        return OFPERR_OFPFMFC_TABLE_FULL;
-    } else if (rule->pending) {
-        ovs_rwlock_unlock(&rule->rwlock);
-        return OFPROTO_POSTPONE;
-    } else {
-        struct ofopgroup *group;
+    return classifier_count(&table->cls) + extra_space > table->max_flows;
+}
 
-        group = ofopgroup_create_unattached(ofproto);
-        delete_flow__(rule, group, OFPRR_EVICTION);
-        ofopgroup_submit(group);
+static enum ofperr
+evict_rules_from_table(struct ofproto *ofproto, struct oftable *table,
+                       unsigned int extra_space)
+    OVS_REQUIRES(ofproto_mutex)
+{
+    while (should_evict_a_rule(table, extra_space)) {
+        struct rule *rule;
 
-        return 0;
+        if (!choose_rule_to_evict(table, &rule)) {
+            return OFPERR_OFPFMFC_TABLE_FULL;
+        } else if (rule->pending) {
+            return OFPROTO_POSTPONE;
+        } else {
+            struct ofopgroup *group = ofopgroup_create_unattached(ofproto);
+            delete_flow__(rule, group, OFPRR_EVICTION);
+            ofopgroup_submit(group);
+        }
     }
+
+    return 0;
 }
 
 /* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
@@ -3446,14 +3767,14 @@ evict_rule_from_table(struct ofproto *ofproto, struct oftable *table)
  * error code on failure, or OFPROTO_POSTPONE if the operation cannot be
  * initiated now but may be retried later.
  *
- * Upon successful return, takes ownership of 'fm->ofpacts'.  On failure,
- * ownership remains with the caller.
+ * The caller retains ownership of 'fm->ofpacts'.
  *
  * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
  * if any. */
 static enum ofperr
 add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
          struct ofputil_flow_mod *fm, const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct oftable *table;
     struct ofopgroup *group;
@@ -3505,12 +3826,15 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
         } else if (rule->pending) {
             return OFPROTO_POSTPONE;
         } else {
-            struct list rules;
+            struct rule_collection rules;
 
-            list_init(&rules);
-            list_push_back(&rules, &rule->ofproto_node);
+            rule_collection_init(&rules);
+            rule_collection_add(&rules, rule);
             fm->modify_cookie = true;
-            return modify_flows__(ofproto, ofconn, fm, request, &rules);
+            error = modify_flows__(ofproto, ofconn, fm, request, &rules);
+            rule_collection_destroy(&rules);
+
+            return error;
         }
     }
 
@@ -3543,7 +3867,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     }
 
     /* If necessary, evict an existing rule to clear out space. */
-    error = evict_rule_from_table(ofproto, table);
+    error = evict_rules_from_table(ofproto, table, 1);
     if (error) {
         cls_rule_destroy(&cr);
         return error;
@@ -3559,31 +3883,28 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     }
 
     /* Initialize base state. */
-    rule->ofproto = ofproto;
-    cls_rule_move(&rule->cr, &cr);
+    *CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto;
+    cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), &cr);
+    atomic_init(&rule->ref_count, 1);
     rule->pending = NULL;
     rule->flow_cookie = fm->new_cookie;
     rule->created = rule->modified = rule->used = time_msec();
 
-    ovs_mutex_init(&rule->timeout_mutex);
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_init(&rule->mutex);
+    ovs_mutex_lock(&rule->mutex);
     rule->idle_timeout = fm->idle_timeout;
     rule->hard_timeout = fm->hard_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
 
-    rule->table_id = table - ofproto->tables;
+    *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables;
     rule->flags = fm->flags & OFPUTIL_FF_STATE;
-
-    rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
-    rule->ofpacts_len = fm->ofpacts_len;
-    rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+    rule->actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
     list_init(&rule->meter_list_node);
     rule->eviction_group = NULL;
     list_init(&rule->expirable);
     rule->monitor_flags = 0;
     rule->add_seqno = 0;
     rule->modify_seqno = 0;
-    ovs_rwlock_init(&rule->rwlock);
 
     /* Construct rule, initializing derived state. */
     error = ofproto->ofproto_class->rule_construct(rule);
@@ -3615,17 +3936,19 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 static enum ofperr
 modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
                struct ofputil_flow_mod *fm, const struct ofp_header *request,
-               struct list *rules)
+               const struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     enum ofoperation_type type;
     struct ofopgroup *group;
-    struct rule *rule;
     enum ofperr error;
+    size_t i;
 
     type = fm->command == OFPFC_ADD ? OFOPERATION_REPLACE : OFOPERATION_MODIFY;
     group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
     error = OFPERR_OFPBRC_EPERM;
-    LIST_FOR_EACH (rule, ofproto_node, rules) {
+    for (i = 0; i < rules->n; i++) {
+        struct rule *rule = rules->rules[i];
         struct ofoperation *op;
         bool actions_changed;
         bool reset_counters;
@@ -3647,7 +3970,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         }
 
         actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
-                                         rule->ofpacts, rule->ofpacts_len);
+                                         rule->actions->ofpacts,
+                                         rule->actions->ofpacts_len);
 
         op = ofoperation_create(group, rule, type, 0);
 
@@ -3655,10 +3979,10 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
             ofproto_rule_change_cookie(ofproto, rule, fm->new_cookie);
         }
         if (type == OFOPERATION_REPLACE) {
-            ovs_mutex_lock(&rule->timeout_mutex);
+            ovs_mutex_lock(&rule->mutex);
             rule->idle_timeout = fm->idle_timeout;
             rule->hard_timeout = fm->hard_timeout;
-            ovs_mutex_unlock(&rule->timeout_mutex);
+            ovs_mutex_unlock(&rule->mutex);
 
             rule->flags = fm->flags & OFPUTIL_FF_STATE;
             if (fm->idle_timeout || fm->hard_timeout) {
@@ -3672,16 +3996,15 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
 
         reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
         if (actions_changed || reset_counters) {
-            op->ofpacts = rule->ofpacts;
-            op->ofpacts_len = rule->ofpacts_len;
-            op->meter_id = rule->meter_id;
+            struct rule_actions *new_actions;
 
-            ovs_rwlock_wrlock(&rule->rwlock);
-            rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
-            rule->ofpacts_len = fm->ofpacts_len;
-            ovs_rwlock_unlock(&rule->rwlock);
+            op->actions = rule->actions;
+            new_actions = rule_actions_create(fm->ofpacts, fm->ofpacts_len);
+
+            ovs_mutex_lock(&rule->mutex);
+            rule->actions = new_actions;
+            ovs_mutex_unlock(&rule->mutex);
 
-            rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
             rule->ofproto->ofproto_class->rule_modify_actions(rule,
                                                               reset_counters);
         } else {
@@ -3696,6 +4019,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
 static enum ofperr
 modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
                  struct ofputil_flow_mod *fm, const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) {
         return 0;
@@ -3712,20 +4036,26 @@ static enum ofperr
 modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
                    struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct list rules;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     int error;
 
-    error = collect_rules_loose(ofproto, fm->table_id, &fm->match,
-                                fm->cookie, fm->cookie_mask,
-                                OFPP_ANY, OFPG11_ANY, &rules);
-    if (error) {
-        return error;
-    } else if (list_is_empty(&rules)) {
-        return modify_flows_add(ofproto, ofconn, fm, request);
-    } else {
-        return modify_flows__(ofproto, ofconn, fm, request, &rules);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, 0,
+                       fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
+    if (!error) {
+        error = (rules.n > 0
+                 ? modify_flows__(ofproto, ofconn, fm, request, &rules)
+                 : modify_flows_add(ofproto, ofconn, fm, request));
     }
+
+    rule_collection_destroy(&rules);
+
+    return error;
 }
 
 /* Implements OFPFC_MODIFY_STRICT.  Returns 0 on success or an OpenFlow error
@@ -3737,22 +4067,28 @@ static enum ofperr
 modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
                    struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct list rules;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     int error;
 
-    error = collect_rules_strict(ofproto, fm->table_id, &fm->match,
-                                 fm->priority, fm->cookie, fm->cookie_mask,
-                                 OFPP_ANY, OFPG11_ANY, &rules);
-    if (error) {
-        return error;
-    } else if (list_is_empty(&rules)) {
-        return modify_flows_add(ofproto, ofconn, fm, request);
-    } else {
-        return list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
-                                                          fm, request, &rules)
-                                         : 0;
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority,
+                       fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
+    error = collect_rules_strict(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
+    if (!error) {
+        if (rules.n == 0) {
+            error =  modify_flows_add(ofproto, ofconn, fm, request);
+        } else if (rules.n == 1) {
+            error = modify_flows__(ofproto, ofconn, fm, request, &rules);
+        }
     }
+
+    rule_collection_destroy(&rules);
+
+    return error;
 }
 \f
 /* OFPFC_DELETE implementation. */
@@ -3760,6 +4096,7 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
 static void
 delete_flow__(struct rule *rule, struct ofopgroup *group,
               enum ofp_flow_removed_reason reason)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = rule->ofproto;
 
@@ -3775,16 +4112,17 @@ delete_flow__(struct rule *rule, struct ofopgroup *group,
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static enum ofperr
 delete_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
-               const struct ofp_header *request, struct list *rules,
+               const struct ofp_header *request,
+               const struct rule_collection *rules,
                enum ofp_flow_removed_reason reason)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct rule *rule, *next;
     struct ofopgroup *group;
+    size_t i;
 
     group = ofopgroup_create(ofproto, ofconn, request, UINT32_MAX);
-    LIST_FOR_EACH_SAFE (rule, next, ofproto_node, rules) {
-        ovs_rwlock_wrlock(&rule->rwlock);
-        delete_flow__(rule, group, reason);
+    for (i = 0; i < rules->n; i++) {
+        delete_flow__(rules->rules[i], group, reason);
     }
     ofopgroup_submit(group);
 
@@ -3796,17 +4134,24 @@ static enum ofperr
 delete_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct list rules;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     enum ofperr error;
 
-    error = collect_rules_loose(ofproto, fm->table_id, &fm->match,
-                                fm->cookie, fm->cookie_mask,
-                                fm->out_port, fm->out_group, &rules);
-    return (error ? error
-            : !list_is_empty(&rules) ? delete_flows__(ofproto, ofconn, request,
-                                                      &rules, OFPRR_DELETE)
-            : 0);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, 0,
+                       fm->cookie, fm->cookie_mask,
+                       fm->out_port, fm->out_group);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
+    if (!error && rules.n > 0) {
+        error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE);
+    }
+    rule_collection_destroy(&rules);
+
+    return error;
 }
 
 /* Implements OFPFC_DELETE_STRICT. */
@@ -3814,22 +4159,29 @@ static enum ofperr
 delete_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct list rules;
+    struct rule_criteria criteria;
+    struct rule_collection rules;
     enum ofperr error;
 
-    error = collect_rules_strict(ofproto, fm->table_id, &fm->match,
-                                 fm->priority, fm->cookie, fm->cookie_mask,
-                                 fm->out_port, fm->out_group, &rules);
-    return (error ? error
-            : list_is_singleton(&rules) ? delete_flows__(ofproto, ofconn,
-                                                         request, &rules,
-                                                         OFPRR_DELETE)
-            : 0);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority,
+                       fm->cookie, fm->cookie_mask,
+                       fm->out_port, fm->out_group);
+    error = collect_rules_strict(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
+    if (!error && rules.n > 0) {
+        error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE);
+    }
+    rule_collection_destroy(&rules);
+
+    return error;
 }
 
 static void
 ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofputil_flow_removed fr;
 
@@ -3845,10 +4197,10 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
     fr.table_id = rule->table_id;
     calc_duration(rule->created, time_msec(),
                   &fr.duration_sec, &fr.duration_nsec);
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     fr.idle_timeout = rule->idle_timeout;
     fr.hard_timeout = rule->hard_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
     rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count,
                                                  &fr.byte_count);
 
@@ -3866,17 +4218,15 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
  * OpenFlow flows. */
 void
 ofproto_rule_expire(struct rule *rule, uint8_t reason)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = rule->ofproto;
-    struct classifier *cls = &ofproto->tables[rule->table_id].cls;
 
     ovs_assert(reason == OFPRR_HARD_TIMEOUT || reason == OFPRR_IDLE_TIMEOUT
                || reason == OFPRR_DELETE || reason == OFPRR_GROUP_DELETE);
-    ofproto_rule_send_removed(rule, reason);
 
-    ovs_rwlock_wrlock(&cls->rwlock);
-    ofproto_delete_rule(ofproto, cls, rule);
-    ovs_rwlock_unlock(&cls->rwlock);
+    ofproto_rule_send_removed(rule, reason);
+    ofproto_rule_delete__(ofproto, rule);
 }
 
 /* Reduces '*timeout' to no more than 'max'.  A value of zero in either case
@@ -3897,26 +4247,27 @@ reduce_timeout(uint16_t max, uint16_t *timeout)
 void
 ofproto_rule_reduce_timeouts(struct rule *rule,
                              uint16_t idle_timeout, uint16_t hard_timeout)
-    OVS_EXCLUDED(rule->ofproto->expirable_mutex, rule->timeout_mutex)
+    OVS_EXCLUDED(ofproto_mutex, rule->mutex)
 {
     if (!idle_timeout && !hard_timeout) {
         return;
     }
 
-    ovs_mutex_lock(&rule->ofproto->expirable_mutex);
+    ovs_mutex_lock(&ofproto_mutex);
     if (list_is_empty(&rule->expirable)) {
         list_insert(&rule->ofproto->expirable, &rule->expirable);
     }
-    ovs_mutex_unlock(&rule->ofproto->expirable_mutex);
+    ovs_mutex_unlock(&ofproto_mutex);
 
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     reduce_timeout(idle_timeout, &rule->idle_timeout);
     reduce_timeout(hard_timeout, &rule->hard_timeout);
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
 }
 \f
 static enum ofperr
 handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_mod fm;
@@ -3975,36 +4326,50 @@ exit:
 static enum ofperr
 handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
                   struct ofputil_flow_mod *fm, const struct ofp_header *oh)
+    OVS_EXCLUDED(ofproto_mutex)
 {
-    if (ofproto->n_pending >= 50) {
-        ovs_assert(!list_is_empty(&ofproto->pending));
-        return OFPROTO_POSTPONE;
-    }
+    enum ofperr error;
 
-    switch (fm->command) {
-    case OFPFC_ADD:
-        return add_flow(ofproto, ofconn, fm, oh);
+    ovs_mutex_lock(&ofproto_mutex);
+    if (ofproto->n_pending < 50) {
+        switch (fm->command) {
+        case OFPFC_ADD:
+            error = add_flow(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_MODIFY:
-        return modify_flows_loose(ofproto, ofconn, fm, oh);
+        case OFPFC_MODIFY:
+            error = modify_flows_loose(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_MODIFY_STRICT:
-        return modify_flow_strict(ofproto, ofconn, fm, oh);
+        case OFPFC_MODIFY_STRICT:
+            error = modify_flow_strict(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_DELETE:
-        return delete_flows_loose(ofproto, ofconn, fm, oh);
+        case OFPFC_DELETE:
+            error = delete_flows_loose(ofproto, ofconn, fm, oh);
+            break;
 
-    case OFPFC_DELETE_STRICT:
-        return delete_flow_strict(ofproto, ofconn, fm, oh);
+        case OFPFC_DELETE_STRICT:
+            error = delete_flow_strict(ofproto, ofconn, fm, oh);
+            break;
 
-    default:
-        if (fm->command > 0xff) {
-            VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but "
-                         "flow_mod_table_id extension is not enabled",
-                         ofproto->name);
+        default:
+            if (fm->command > 0xff) {
+                VLOG_WARN_RL(&rl, "%s: flow_mod has explicit table_id but "
+                             "flow_mod_table_id extension is not enabled",
+                             ofproto->name);
+            }
+            error = OFPERR_OFPFMFC_BAD_COMMAND;
+            break;
         }
-        return OFPERR_OFPFMFC_BAD_COMMAND;
+    } else {
+        ovs_assert(!list_is_empty(&ofproto->pending));
+        error = OFPROTO_POSTPONE;
     }
+    ovs_mutex_unlock(&ofproto_mutex);
+
+    run_rule_executes(ofproto);
+    return error;
 }
 
 static enum ofperr
@@ -4185,8 +4550,10 @@ static void
 ofproto_compose_flow_refresh_update(const struct rule *rule,
                                     enum nx_flow_monitor_flags flags,
                                     struct list *msgs)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofoperation *op = rule->pending;
+    const struct rule_actions *actions;
     struct ofputil_flow_update fu;
     struct match match;
 
@@ -4199,21 +4566,20 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
     fu.event = (flags & (NXFMF_INITIAL | NXFMF_ADD)
                 ? NXFME_ADDED : NXFME_MODIFIED);
     fu.reason = 0;
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     fu.idle_timeout = rule->idle_timeout;
     fu.hard_timeout = rule->hard_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
     fu.table_id = rule->table_id;
     fu.cookie = rule->flow_cookie;
     minimatch_expand(&rule->cr.match, &match);
     fu.match = &match;
     fu.priority = rule->cr.priority;
+
     if (!(flags & NXFMF_ACTIONS)) {
-        fu.ofpacts = NULL;
-        fu.ofpacts_len = 0;
+        actions = NULL;
     } else if (!op) {
-        fu.ofpacts = rule->ofpacts;
-        fu.ofpacts_len = rule->ofpacts_len;
+        actions = rule->actions;
     } else {
         /* An operation is in progress.  Use the previous version of the flow's
          * actions, so that when the operation commits we report the change. */
@@ -4223,24 +4589,19 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
 
         case OFOPERATION_MODIFY:
         case OFOPERATION_REPLACE:
-            if (op->ofpacts) {
-                fu.ofpacts = op->ofpacts;
-                fu.ofpacts_len = op->ofpacts_len;
-            } else {
-                fu.ofpacts = rule->ofpacts;
-                fu.ofpacts_len = rule->ofpacts_len;
-            }
+            actions = op->actions ? op->actions : rule->actions;
             break;
 
         case OFOPERATION_DELETE:
-            fu.ofpacts = rule->ofpacts;
-            fu.ofpacts_len = rule->ofpacts_len;
+            actions = rule->actions;
             break;
 
         default:
             NOT_REACHED();
         }
     }
+    fu.ofpacts = actions ? actions->ofpacts : NULL;
+    fu.ofpacts_len = actions ? actions->ofpacts_len : 0;
 
     if (list_is_empty(msgs)) {
         ofputil_start_flow_update(msgs);
@@ -4249,11 +4610,14 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
 }
 
 void
-ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs)
+ofmonitor_compose_refresh_updates(struct rule_collection *rules,
+                                  struct list *msgs)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct rule *rule;
+    size_t i;
 
-    LIST_FOR_EACH (rule, ofproto_node, rules) {
+    for (i = 0; i < rules->n; i++) {
+        struct rule *rule = rules->rules[i];
         enum nx_flow_monitor_flags flags = rule->monitor_flags;
         rule->monitor_flags = 0;
 
@@ -4264,7 +4628,8 @@ ofmonitor_compose_refresh_updates(struct list *rules, struct list *msgs)
 static void
 ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m,
                                        struct rule *rule, uint64_t seqno,
-                                       struct list *rules)
+                                       struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     enum nx_flow_monitor_flags update;
 
@@ -4295,7 +4660,7 @@ ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m,
     }
 
     if (!rule->monitor_flags) {
-        list_push_back(rules, &rule->ofproto_node);
+        rule_collection_add(rules, rule);
     }
     rule->monitor_flags |= update | (m->flags & NXFMF_ACTIONS);
 }
@@ -4303,7 +4668,8 @@ ofproto_collect_ofmonitor_refresh_rule(const struct ofmonitor *m,
 static void
 ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m,
                                         uint64_t seqno,
-                                        struct list *rules)
+                                        struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     const struct ofproto *ofproto = ofconn_get_ofproto(m->ofconn);
     const struct ofoperation *op;
@@ -4339,7 +4705,8 @@ ofproto_collect_ofmonitor_refresh_rules(const struct ofmonitor *m,
 
 static void
 ofproto_collect_ofmonitor_initial_rules(struct ofmonitor *m,
-                                        struct list *rules)
+                                        struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (m->flags & NXFMF_INITIAL) {
         ofproto_collect_ofmonitor_refresh_rules(m, 0, rules);
@@ -4348,20 +4715,22 @@ ofproto_collect_ofmonitor_initial_rules(struct ofmonitor *m,
 
 void
 ofmonitor_collect_resume_rules(struct ofmonitor *m,
-                               uint64_t seqno, struct list *rules)
+                               uint64_t seqno, struct rule_collection *rules)
+    OVS_REQUIRES(ofproto_mutex)
 {
     ofproto_collect_ofmonitor_refresh_rules(m, seqno, rules);
 }
 
 static enum ofperr
 handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofmonitor **monitors;
     size_t n_monitors, allocated_monitors;
+    struct rule_collection rules;
     struct list replies;
     enum ofperr error;
-    struct list rules;
     struct ofpbuf b;
     size_t i;
 
@@ -4369,6 +4738,8 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh)
     ofpbuf_use_const(&b, oh, ntohs(oh->length));
     monitors = NULL;
     n_monitors = allocated_monitors = 0;
+
+    ovs_mutex_lock(&ofproto_mutex);
     for (;;) {
         struct ofputil_flow_monitor_request request;
         struct ofmonitor *m;
@@ -4400,20 +4771,25 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh)
         monitors[n_monitors++] = m;
     }
 
-    list_init(&rules);
+    rule_collection_init(&rules);
     for (i = 0; i < n_monitors; i++) {
         ofproto_collect_ofmonitor_initial_rules(monitors[i], &rules);
     }
 
     ofpmp_init(&replies, oh);
     ofmonitor_compose_refresh_updates(&rules, &replies);
-    ofconn_send_replies(ofconn, &replies);
+    ovs_mutex_unlock(&ofproto_mutex);
 
+    rule_collection_destroy(&rules);
+
+    ofconn_send_replies(ofconn, &replies);
     free(monitors);
 
     return 0;
 
 error:
+    ovs_mutex_unlock(&ofproto_mutex);
+
     for (i = 0; i < n_monitors; i++) {
         ofmonitor_destroy(monitors[i]);
     }
@@ -4423,18 +4799,25 @@ error:
 
 static enum ofperr
 handle_flow_monitor_cancel(struct ofconn *ofconn, const struct ofp_header *oh)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofmonitor *m;
+    enum ofperr error;
     uint32_t id;
 
     id = ofputil_decode_flow_monitor_cancel(oh);
+
+    ovs_mutex_lock(&ofproto_mutex);
     m = ofmonitor_lookup(ofconn, id);
-    if (!m) {
-        return OFPERR_NXBRC_FM_BAD_ID;
+    if (m) {
+        ofmonitor_destroy(m);
+        error = 0;
+    } else {
+        error = OFPERR_NXBRC_FM_BAD_ID;
     }
+    ovs_mutex_unlock(&ofproto_mutex);
 
-    ofmonitor_destroy(m);
-    return 0;
+    return error;
 }
 
 /* Meters implementation.
@@ -4505,6 +4888,7 @@ meter_create(const struct ofputil_meter_config *config,
 
 static void
 meter_delete(struct ofproto *ofproto, uint32_t first, uint32_t last)
+    OVS_REQUIRES(ofproto_mutex)
 {
     uint32_t mid;
     for (mid = first; mid <= last; ++mid) {
@@ -4562,11 +4946,13 @@ handle_modify_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm)
 static enum ofperr
 handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh,
                     struct ofputil_meter_mod *mm)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     uint32_t meter_id = mm->meter.meter_id;
+    struct rule_collection rules;
+    enum ofperr error = 0;
     uint32_t first, last;
-    struct list rules;
 
     if (meter_id == OFPM13_ALL) {
         first = 1;
@@ -4580,7 +4966,8 @@ handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh,
 
     /* First delete the rules that use this meter.  If any of those rules are
      * currently being modified, postpone the whole operation until later. */
-    list_init(&rules);
+    rule_collection_init(&rules);
+    ovs_mutex_lock(&ofproto_mutex);
     for (meter_id = first; meter_id <= last; ++meter_id) {
         struct meter *meter = ofproto->meters[meter_id];
         if (meter && !list_is_empty(&meter->rules)) {
@@ -4588,20 +4975,25 @@ handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh,
 
             LIST_FOR_EACH (rule, meter_list_node, &meter->rules) {
                 if (rule->pending) {
-                    return OFPROTO_POSTPONE;
+                    error = OFPROTO_POSTPONE;
+                    goto exit;
                 }
-                list_push_back(&rules, &rule->ofproto_node);
+                rule_collection_add(&rules, rule);
             }
         }
     }
-    if (!list_is_empty(&rules)) {
+    if (rules.n > 0) {
         delete_flows__(ofproto, ofconn, oh, &rules, OFPRR_METER_DELETE);
     }
 
     /* Delete the meters. */
     meter_delete(ofproto, first, last);
 
-    return 0;
+exit:
+    ovs_mutex_unlock(&ofproto_mutex);
+    rule_collection_destroy(&rules);
+
+    return error;
 }
 
 static enum ofperr
@@ -4630,9 +5022,12 @@ handle_meter_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 
     if (mm.command != OFPMC13_DELETE) {
         /* Fails also when meters are not implemented by the provider. */
-        if (!meter_id || meter_id > ofproto->meter_features.max_meters) {
+        if (meter_id == 0 || meter_id > OFPM13_MAX) {
             error = OFPERR_OFPMMFC_INVALID_METER;
             goto exit_free_bands;
+        } else if (meter_id > ofproto->meter_features.max_meters) {
+            error = OFPERR_OFPMMFC_OUT_OF_METERS;
+            goto exit_free_bands;
         }
         if (mm.meter.n_bands > ofproto->meter_features.max_bands) {
             error = OFPERR_OFPMMFC_OUT_OF_BANDS;
@@ -5171,6 +5566,7 @@ handle_table_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 
 static enum ofperr
 handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     const struct ofp_header *oh = msg->data;
     enum ofptype type;
@@ -5330,6 +5726,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
 
 static bool
 handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg)
+    OVS_EXCLUDED(ofproto_mutex)
 {
     int error = handle_openflow__(ofconn, ofp_msg);
     if (error && error != OFPROTO_POSTPONE) {
@@ -5348,6 +5745,7 @@ handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg)
  * ofoperation_create() and then submit it with ofopgroup_submit(). */
 static struct ofopgroup *
 ofopgroup_create_unattached(struct ofproto *ofproto)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofopgroup *group = xzalloc(sizeof *group);
     group->ofproto = ofproto;
@@ -5372,6 +5770,7 @@ ofopgroup_create_unattached(struct ofproto *ofproto)
 static struct ofopgroup *
 ofopgroup_create(struct ofproto *ofproto, struct ofconn *ofconn,
                  const struct ofp_header *request, uint32_t buffer_id)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofopgroup *group = ofopgroup_create_unattached(ofproto);
     if (ofconn) {
@@ -5395,6 +5794,7 @@ ofopgroup_create(struct ofproto *ofproto, struct ofconn *ofconn,
  * groups. */
 static void
 ofopgroup_submit(struct ofopgroup *group)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (!group->n_running) {
         ofopgroup_complete(group);
@@ -5406,6 +5806,7 @@ ofopgroup_submit(struct ofopgroup *group)
 
 static void
 ofopgroup_complete(struct ofopgroup *group)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = group->ofproto;
 
@@ -5434,8 +5835,23 @@ ofopgroup_complete(struct ofopgroup *group)
                 error = ofconn_pktbuf_retrieve(group->ofconn, group->buffer_id,
                                                &packet, &in_port);
                 if (packet) {
+                    struct rule_execute *re;
+
                     ovs_assert(!error);
-                    error = rule_execute(op->rule, in_port, packet);
+
+                    ofproto_rule_ref(op->rule);
+
+                    re = xmalloc(sizeof *re);
+                    re->rule = op->rule;
+                    re->in_port = in_port;
+                    re->packet = packet;
+
+                    if (!guarded_list_push_back(&ofproto->rule_executes,
+                                                &re->list_node, 1024)) {
+                        ofproto_rule_unref(op->rule);
+                        ofpbuf_delete(re->packet);
+                        free(re);
+                    }
                 }
                 break;
             }
@@ -5463,7 +5879,7 @@ ofopgroup_complete(struct ofopgroup *group)
         if (!(op->error
               || ofproto_rule_is_hidden(rule)
               || (op->type == OFOPERATION_MODIFY
-                  && op->ofpacts
+                  && op->actions
                   && rule->flow_cookie == op->flow_cookie))) {
             /* Check that we can just cast from ofoperation_type to
              * nx_flow_update_event. */
@@ -5511,15 +5927,14 @@ ofopgroup_complete(struct ofopgroup *group)
                     }
                 }
             } else {
-                ovs_rwlock_wrlock(&rule->rwlock);
                 oftable_remove_rule(rule);
-                ofproto_rule_destroy(rule);
+                ofproto_rule_unref(rule);
             }
             break;
 
         case OFOPERATION_DELETE:
             ovs_assert(!op->error);
-            ofproto_rule_destroy(rule);
+            ofproto_rule_unref(rule);
             op->rule = NULL;
             break;
 
@@ -5534,20 +5949,20 @@ ofopgroup_complete(struct ofopgroup *group)
                 }
             } else {
                 ofproto_rule_change_cookie(ofproto, rule, op->flow_cookie);
-                ovs_mutex_lock(&rule->timeout_mutex);
+                ovs_mutex_lock(&rule->mutex);
                 rule->idle_timeout = op->idle_timeout;
                 rule->hard_timeout = op->hard_timeout;
-                ovs_mutex_unlock(&rule->timeout_mutex);
-                if (op->ofpacts) {
-                    free(rule->ofpacts);
+                ovs_mutex_unlock(&rule->mutex);
+                if (op->actions) {
+                    struct rule_actions *old_actions;
 
-                    ovs_rwlock_wrlock(&rule->rwlock);
-                    rule->ofpacts = op->ofpacts;
-                    rule->ofpacts_len = op->ofpacts_len;
-                    ovs_rwlock_unlock(&rule->rwlock);
+                    ovs_mutex_lock(&rule->mutex);
+                    old_actions = rule->actions;
+                    rule->actions = op->actions;
+                    ovs_mutex_unlock(&rule->mutex);
 
-                    op->ofpacts = NULL;
-                    op->ofpacts_len = 0;
+                    op->actions = NULL;
+                    rule_actions_unref(old_actions);
                 }
                 rule->flags = op->flags;
             }
@@ -5590,6 +6005,7 @@ static struct ofoperation *
 ofoperation_create(struct ofopgroup *group, struct rule *rule,
                    enum ofoperation_type type,
                    enum ofp_flow_removed_reason reason)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = group->ofproto;
     struct ofoperation *op;
@@ -5603,10 +6019,10 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule,
     op->type = type;
     op->reason = reason;
     op->flow_cookie = rule->flow_cookie;
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     op->idle_timeout = rule->idle_timeout;
     op->hard_timeout = rule->hard_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
     op->flags = rule->flags;
 
     group->n_running++;
@@ -5621,6 +6037,7 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule,
 
 static void
 ofoperation_destroy(struct ofoperation *op)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofopgroup *group = op->group;
 
@@ -5631,7 +6048,7 @@ ofoperation_destroy(struct ofoperation *op)
         hmap_remove(&group->ofproto->deletions, &op->hmap_node);
     }
     list_remove(&op->group_node);
-    free(op->ofpacts);
+    rule_actions_unref(op->actions);
     free(op);
 }
 
@@ -5662,13 +6079,19 @@ ofoperation_complete(struct ofoperation *op, enum ofperr error)
 {
     struct ofopgroup *group = op->group;
 
-    ovs_assert(op->rule->pending == op);
     ovs_assert(group->n_running > 0);
     ovs_assert(!error || op->type != OFOPERATION_DELETE);
 
     op->error = error;
     if (!--group->n_running && !list_is_empty(&group->ofproto_node)) {
+        /* This function can be called from ->rule_construct(), in which case
+         * ofproto_mutex is held, or it can be called from ->run(), in which
+         * case ofproto_mutex is not held.  But only in the latter case can we
+         * arrive here, so we can safely take ofproto_mutex now. */
+        ovs_mutex_lock(&ofproto_mutex);
+        ovs_assert(op->rule->pending == op);
         ofopgroup_complete(group);
+        ovs_mutex_unlock(&ofproto_mutex);
     }
 }
 \f
@@ -5709,6 +6132,7 @@ pick_fallback_dpid(void)
  * or with no timeouts are not evictable.) */
 static bool
 choose_rule_to_evict(struct oftable *table, struct rule **rulep)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct eviction_group *evg;
 
@@ -5733,10 +6157,8 @@ choose_rule_to_evict(struct oftable *table, struct rule **rulep)
         struct rule *rule;
 
         HEAP_FOR_EACH (rule, evg_node, &evg->rules) {
-            if (!ovs_rwlock_trywrlock(&rule->rwlock)) {
-                *rulep = rule;
-                return true;
-            }
+            *rulep = rule;
+            return true;
         }
     }
 
@@ -5752,39 +6174,13 @@ choose_rule_to_evict(struct oftable *table, struct rule **rulep)
 static void
 ofproto_evict(struct ofproto *ofproto)
 {
-    struct ofopgroup *group;
     struct oftable *table;
 
-    group = ofopgroup_create_unattached(ofproto);
+    ovs_mutex_lock(&ofproto_mutex);
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
-        while (table->eviction_fields) {
-            struct rule *rule;
-            size_t n_rules;
-
-            ovs_rwlock_rdlock(&table->cls.rwlock);
-            n_rules = classifier_count(&table->cls);
-            ovs_rwlock_unlock(&table->cls.rwlock);
-
-            if (n_rules <= table->max_flows) {
-                break;
-            }
-
-            if (!choose_rule_to_evict(table, &rule)) {
-                break;
-            }
-
-            if (rule->pending) {
-                ovs_rwlock_unlock(&rule->rwlock);
-                break;
-            }
-
-            ofoperation_create(group, rule,
-                               OFOPERATION_DELETE, OFPRR_EVICTION);
-            oftable_remove_rule(rule);
-            ofproto->ofproto_class->rule_delete(rule);
-        }
+        evict_rules_from_table(ofproto, table, 0);
     }
-    ofopgroup_submit(group);
+    ovs_mutex_unlock(&ofproto_mutex);
 }
 \f
 /* Eviction groups. */
@@ -5803,6 +6199,7 @@ eviction_group_priority(size_t n_rules)
  * adds or removes rules in 'evg'. */
 static void
 eviction_group_resized(struct oftable *table, struct eviction_group *evg)
+    OVS_REQUIRES(ofproto_mutex)
 {
     heap_change(&table->eviction_groups_by_size, &evg->size_node,
                 eviction_group_priority(heap_count(&evg->rules)));
@@ -5818,6 +6215,7 @@ eviction_group_resized(struct oftable *table, struct eviction_group *evg)
  *   - Frees 'evg'. */
 static void
 eviction_group_destroy(struct oftable *table, struct eviction_group *evg)
+    OVS_REQUIRES(ofproto_mutex)
 {
     while (!heap_is_empty(&evg->rules)) {
         struct rule *rule;
@@ -5834,6 +6232,7 @@ eviction_group_destroy(struct oftable *table, struct eviction_group *evg)
 /* Removes 'rule' from its eviction group, if any. */
 static void
 eviction_group_remove_rule(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (rule->eviction_group) {
         struct oftable *table = &rule->ofproto->tables[rule->table_id];
@@ -5853,6 +6252,7 @@ eviction_group_remove_rule(struct rule *rule)
  * returns the hash value. */
 static uint32_t
 eviction_group_hash_rule(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct oftable *table = &rule->ofproto->tables[rule->table_id];
     const struct mf_subfield *sf;
@@ -5890,6 +6290,7 @@ eviction_group_hash_rule(struct rule *rule)
  * if necessary. */
 static struct eviction_group *
 eviction_group_find(struct oftable *table, uint32_t id)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct eviction_group *evg;
 
@@ -5911,6 +6312,7 @@ eviction_group_find(struct oftable *table, uint32_t id)
  * for eviction. */
 static uint32_t
 rule_eviction_priority(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     long long int hard_expiration;
     long long int idle_expiration;
@@ -5918,7 +6320,7 @@ rule_eviction_priority(struct rule *rule)
     uint32_t expiration_offset;
 
     /* Calculate time of expiration. */
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     hard_expiration = (rule->hard_timeout
                        ? rule->modified + rule->hard_timeout * 1000
                        : LLONG_MAX);
@@ -5926,7 +6328,7 @@ rule_eviction_priority(struct rule *rule)
                        ? rule->used + rule->idle_timeout * 1000
                        : LLONG_MAX);
     expiration = MIN(hard_expiration, idle_expiration);
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
     if (expiration == LLONG_MAX) {
         return 0;
     }
@@ -5950,14 +6352,15 @@ rule_eviction_priority(struct rule *rule)
  * The caller must ensure that 'rule' is not already in an eviction group. */
 static void
 eviction_group_add_rule(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = rule->ofproto;
     struct oftable *table = &ofproto->tables[rule->table_id];
     bool has_timeout;
 
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     has_timeout = rule->hard_timeout || rule->idle_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
 
     if (table->eviction_fields && has_timeout) {
         struct eviction_group *evg;
@@ -6022,6 +6425,7 @@ oftable_set_name(struct oftable *table, const char *name)
  * This function configures the former policy on 'table'. */
 static void
 oftable_disable_eviction(struct oftable *table)
+    OVS_REQUIRES(ofproto_mutex)
 {
     if (table->eviction_fields) {
         struct eviction_group *evg, *next;
@@ -6048,6 +6452,7 @@ oftable_disable_eviction(struct oftable *table)
 static void
 oftable_enable_eviction(struct oftable *table,
                         const struct mf_subfield *fields, size_t n_fields)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct cls_cursor cursor;
     struct rule *rule;
@@ -6080,61 +6485,60 @@ oftable_enable_eviction(struct oftable *table,
 
 /* Removes 'rule' from the oftable that contains it. */
 static void
-oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls,
-                      struct rule *rule)
-    OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->rwlock)
+oftable_remove_rule__(struct ofproto *ofproto, struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    classifier_remove(cls, &rule->cr);
+    struct classifier *cls = &ofproto->tables[rule->table_id].cls;
+
+    ovs_rwlock_wrlock(&cls->rwlock);
+    classifier_remove(cls, CONST_CAST(struct cls_rule *, &rule->cr));
+    ovs_rwlock_unlock(&cls->rwlock);
+
     cookies_remove(ofproto, rule);
+
     eviction_group_remove_rule(rule);
-    ovs_mutex_lock(&ofproto->expirable_mutex);
     if (!list_is_empty(&rule->expirable)) {
         list_remove(&rule->expirable);
     }
-    ovs_mutex_unlock(&ofproto->expirable_mutex);
     if (!list_is_empty(&rule->meter_list_node)) {
         list_remove(&rule->meter_list_node);
         list_init(&rule->meter_list_node);
     }
-    ovs_rwlock_unlock(&rule->rwlock);
 }
 
 static void
 oftable_remove_rule(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
-    struct ofproto *ofproto = rule->ofproto;
-    struct oftable *table = &ofproto->tables[rule->table_id];
-
-    ovs_rwlock_wrlock(&table->cls.rwlock);
-    oftable_remove_rule__(ofproto, &table->cls, rule);
-    ovs_rwlock_unlock(&table->cls.rwlock);
+    oftable_remove_rule__(rule->ofproto, rule);
 }
 
 /* Inserts 'rule' into its oftable, which must not already contain any rule for
  * the same cls_rule. */
 static void
 oftable_insert_rule(struct rule *rule)
+    OVS_REQUIRES(ofproto_mutex)
 {
     struct ofproto *ofproto = rule->ofproto;
     struct oftable *table = &ofproto->tables[rule->table_id];
     bool may_expire;
 
-    ovs_mutex_lock(&rule->timeout_mutex);
+    ovs_mutex_lock(&rule->mutex);
     may_expire = rule->hard_timeout || rule->idle_timeout;
-    ovs_mutex_unlock(&rule->timeout_mutex);
+    ovs_mutex_unlock(&rule->mutex);
 
     if (may_expire) {
-        ovs_mutex_lock(&ofproto->expirable_mutex);
         list_insert(&ofproto->expirable, &rule->expirable);
-        ovs_mutex_unlock(&ofproto->expirable_mutex);
     }
+
     cookies_insert(ofproto, rule);
-    if (rule->meter_id) {
-        struct meter *meter = ofproto->meters[rule->meter_id];
+
+    if (rule->actions->meter_id) {
+        struct meter *meter = ofproto->meters[rule->actions->meter_id];
         list_insert(&meter->rules, &rule->meter_list_node);
     }
     ovs_rwlock_wrlock(&table->cls.rwlock);
-    classifier_insert(&table->cls, &rule->cr);
+    classifier_insert(&table->cls, CONST_CAST(struct cls_rule *, &rule->cr));
     ovs_rwlock_unlock(&table->cls.rwlock);
     eviction_group_add_rule(rule);
 }
@@ -6204,6 +6608,7 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap)
     OFPROTO_FOR_EACH_TABLE (oftable, ofproto) {
         const struct cls_table *table;
 
+        ovs_rwlock_rdlock(&oftable->cls.rwlock);
         HMAP_FOR_EACH (table, hmap_node, &oftable->cls.tables) {
             if (minimask_get_vid_mask(&table->mask) == VLAN_VID_MASK) {
                 const struct cls_rule *rule;
@@ -6215,6 +6620,7 @@ ofproto_get_vlan_usage(struct ofproto *ofproto, unsigned long int *vlan_bitmap)
                 }
             }
         }
+        ovs_rwlock_unlock(&oftable->cls.rwlock);
     }
 }
 
index 7cd0485..8670127 100644 (file)
@@ -519,7 +519,8 @@ do_show_log(int argc, char *argv[])
             date = shash_find_data(json_object(json), "_date");
             if (date && date->type == JSON_INTEGER) {
                 time_t t = json_integer(date);
-                char *s = xastrftime(" %Y-%m-%d %H:%M:%S", t, true);
+                char *s = xastrftime_msec(" %Y-%m-%d %H:%M:%S",
+                                          t * 1000LL, true);
                 fputs(s, stdout);
                 free(s);
             }
index 25ae4ea..14679d9 100644 (file)
@@ -60,7 +60,8 @@ class Vlog:
         if not Vlog.__inited:
             return
 
-        now = datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")
+        dt = datetime.datetime.utcnow();
+        now = dt.strftime("%Y-%m-%dT%H:%M:%S.%%iZ") % (dt.microsecond/1000)
         syslog_message = ("%s|%s|%s|%s"
                            % (Vlog.__msg_num, self.name, level, message))
 
index 91ee85a..0e1d41b 100644 (file)
@@ -732,13 +732,13 @@ AT_CLEANUP
 AT_SETUP([ofproto - flow table configuration (OpenFlow 1.2)])
 OVS_VSWITCHD_START
 # Check the default configuration.
-(mid="wild=0xffffffffff, max=1000000,"
+(mid="wild=0xfffffffff, max=1000000,"
  tail="
                lookup=0, matched=0
-               match=0xffffffffff, instructions=0x00000007, config=0x00000003
+               match=0xfffffffff, instructions=0x00000007, config=0x00000003
                write_actions=0x00000000, apply_actions=0x00000000
-               write_setfields=0x000000ffffffffff
-               apply_setfields=0x000000ffffffffff
+               write_setfields=0x0000000fffffffff
+               apply_setfields=0x0000000fffffffff
                metadata_match=0xffffffffffffffff
                metadata_write=0xffffffffffffffff"
  echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables
@@ -763,9 +763,9 @@ AT_CHECK(
 # Check that the configuration was updated.
 mv expout orig-expout
 (echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables
-  0: main    : wild=0xffffffffff, max=1000000, active=0"
+  0: main    : wild=0xfffffffff, max=1000000, active=0"
  tail -n +3 orig-expout | head -7
- echo "  1: table1  : wild=0xffffffffff, max=  1024, active=0"
+ echo "  1: table1  : wild=0xfffffffff, max=  1024, active=0"
  tail -n +11 orig-expout) > expout
 AT_CHECK([ovs-ofctl -O OpenFlow12 dump-tables br0], [0], [expout])
 OVS_VSWITCHD_STOP
index 957d872..35336ce 100644 (file)
@@ -9,7 +9,7 @@ AT_CHECK([$PYTHON $srcdir/test-vlog.py --log-file log_file \
 
 AT_CHECK([diff log_file stderr_log])
 
-AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z|//' \
+AT_CHECK([sed -e 's/.*-.*-.*T..:..:..\....Z|//' \
 -e 's/File ".*", line [[0-9]][[0-9]]*,/File <name>, line <number>,/' \
 stderr_log], [0], [dnl
 0|module_0|EMER|emergency
index df56309..1cf888d 100644 (file)
@@ -158,13 +158,20 @@ The current date and time in ISO 8601 format (YYYY\-MM\-DD HH:MM:SS).
 .IP \fB%d{\fIformat\fB}\fR
 The current date and time in the specified \fIformat\fR, which takes
 the same format as the \fItemplate\fR argument to \fBstrftime\fR(3).
+As an extension, any \fB#\fR characters in \fIformat\fR will be
+replaced by fractional seconds, e.g. use \fB%H:%M:%S.###\fR for the
+time to the nearest millisecond.  Sub-second times are only
+approximate and currently decimal places after the third will always
+be reported as zero.
 .
 .IP \fB%D\fR
 The current UTC date and time in ISO 8601 format (YYYY\-MM\-DD HH:MM:SS).
 .
 .IP \fB%D{\fIformat\fB}\fR
-The current UTC date and time in the specified \fIformat\fR, which takes
-the same format as the \fItemplate\fR argument to \fBstrftime\fR(3).
+The current UTC date and time in the specified \fIformat\fR, which
+takes the same format as the \fItemplate\fR argument to
+\fBstrftime\fR(3).  Supports the same extension for sub-second
+resolution as \fB%d{\fR...\fB}\fR.
 .
 .IP \fB%m\fR
 The message being logged.
index 95bf1bf..a5a5c02 100644 (file)
@@ -1396,7 +1396,8 @@ monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests)
             run(retval, "vconn_recv");
 
             if (timestamp) {
-                char *s = xastrftime("%Y-%m-%d %H:%M:%S: ", time_wall(), true);
+                char *s = xastrftime_msec("%Y-%m-%d %H:%M:%S.###: ",
+                                          time_wall_msec(), true);
                 fputs(s, stderr);
                 free(s);
             }
index 5e54e0b..da2dc42 100644 (file)
@@ -2259,11 +2259,10 @@ instant_stats_run(void)
                 iface_refresh_cfm_stats(iface);
 
                 smap_init(&smap);
-                if (!ofproto_port_get_bfd_status(br->ofproto, iface->ofp_port,
-                                                 &smap)) {
-                    ovsrec_interface_set_bfd_status(iface->cfg, &smap);
-                    smap_destroy(&smap);
-                }
+                ofproto_port_get_bfd_status(br->ofproto, iface->ofp_port,
+                                            &smap);
+                ovsrec_interface_set_bfd_status(iface->cfg, &smap);
+                smap_destroy(&smap);
             }
         }
     }