[ovs-dev] [threaded-learning v2 12/25] guarded-list: New data structure for thread-safe queue.
Ben Pfaff
blp at nicira.com
Thu Sep 12 07:39:24 UTC 2013
We already had queues that were suitable for replacement by this data
structure, and I intend to add another one later on.
flow_miss_batch_ofproto_destroyed() did not work well with the guarded-list
structure (it required either adding a lot more functions or breaking the
abstraction) so I changed the caller to just use udpif_revalidate().
Checking reval_seq at the end of handle_miss_upcalls() also didn't work
well with the abstraction, so I decided that since this was a corner case
anyway it would be acceptable to just drop those in flow_miss_batch_next().
Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Ethan Jackson <ethan at nicira.com>
---
lib/automake.mk | 2 +
lib/guarded-list.c | 97 ++++++++++++++++++++
lib/guarded-list.h | 41 +++++++++
ofproto/ofproto-dpif-upcall.c | 196 ++++++++++++-----------------------------
ofproto/ofproto-dpif-upcall.h | 6 +-
ofproto/ofproto-dpif.c | 77 ++++------------
6 files changed, 216 insertions(+), 203 deletions(-)
create mode 100644 lib/guarded-list.c
create mode 100644 lib/guarded-list.h
diff --git a/lib/automake.mk b/lib/automake.mk
index da1896a..92cfc13 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -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 \
diff --git a/lib/guarded-list.c b/lib/guarded-list.c
new file mode 100644
index 0000000..cbb2030
--- /dev/null
+++ b/lib/guarded-list.c
@@ -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
index 0000000..625865d
--- /dev/null
+++ b/lib/guarded-list.h
@@ -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 */
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ae856a4..850633d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -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"
@@ -79,26 +80,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 +111,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 +140,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);
@@ -229,33 +216,16 @@ 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 +235,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 +259,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 +279,24 @@ upcall_destroy(struct upcall *upcall)
struct flow_miss_batch *
flow_miss_batch_next(struct udpif *udpif)
{
- struct flow_miss_batch *next = NULL;
+ for (;;) {
+ struct flow_miss_batch *next;
+ unsigned int reval_seq;
+ struct list *next_node;
- 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_node = guarded_list_pop_front(&udpif->fmbs);
+ if (!next_node) {
+ return NULL;
+ }
+
+ 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;
}
/* Destroys and deallocates 'fmb'. */
@@ -347,52 +318,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 +342,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);
}
/* The dispatcher thread is responsible for receving upcalls from the kernel,
@@ -618,17 +549,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);
}
@@ -762,20 +692,18 @@ 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);
-
/* Construct the to-do list.
*
* This just amounts to extracting the flow from each packet and sticking
* 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 +736,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 +792,11 @@ 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);
- 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);
+ if (guarded_list_push_back(&udpif->fmbs, &fmb->list_node,
+ MAX_QUEUE_LENGTH)) {
seq_change(udpif->wait_seq);
} else {
COVERAGE_INC(fmb_queue_overflow);
- ovs_mutex_unlock(&udpif->fmb_mutex);
flow_miss_batch_destroy(fmb);
}
}
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index 8e8264e..57d462d 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -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 *);
/* 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
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6f1a4e5..bf5af33 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -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"
@@ -507,13 +508,8 @@ 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 flow_mods; /* Contains "struct flow_mod"s. */
+ struct guarded_list pins; /* Contains "struct ofputil_packet_in"s. */
};
/* By default, flows in the datapath are wildcarded (megaflows). They
@@ -560,18 +556,11 @@ 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);
+ if (!guarded_list_push_back(&ofproto->flow_mods, &fm->list_node, 1024)) {
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);
}
/* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
@@ -580,18 +569,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);
}
/* Factory functions. */
@@ -1024,7 +1006,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;
@@ -1286,17 +1267,8 @@ construct(struct ofproto *ofproto_)
classifier_init(&ofproto->facets);
ofproto->consistency_rl = LLONG_MIN;
- 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->flow_mods);
+ guarded_list_init(&ofproto->pins);
ofproto_dpif_unixctl_init();
@@ -1422,6 +1394,7 @@ destruct(struct ofproto *ofproto_)
struct ofputil_packet_in *pin, *next_pin;
struct ofputil_flow_mod *fm, *next_fm;
struct facet *facet, *next_facet;
+ struct list flow_mods, pins;
struct cls_cursor cursor;
struct oftable *table;
@@ -1437,7 +1410,9 @@ 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);
@@ -1452,25 +1427,21 @@ destruct(struct ofproto *ofproto_)
ovs_rwlock_unlock(&table->cls.rwlock);
}
- ovs_mutex_lock(&ofproto->flow_mod_mutex);
- LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &ofproto->flow_mods) {
+ guarded_list_pop_all(&ofproto->flow_mods, &flow_mods);
+ LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &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);
+ guarded_list_destroy(&ofproto->flow_mods);
- 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);
@@ -1508,12 +1479,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);
-
+ guarded_list_pop_all(&ofproto->flow_mods, &flow_mods);
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)) {
@@ -1526,12 +1492,7 @@ run_fast(struct ofproto *ofproto_)
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);
--
1.7.10.4
More information about the dev
mailing list