VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
+COVERAGE_DEFINE(upcall_duplicate_flow);
+
/* A thread that reads upcalls from dpif, forwards each upcall's packet,
* and possibly sets up a kernel flow as a cache. */
struct handler {
long long int created; /* Estimation of creation time. */
bool mark; /* Used by mark and sweep GC algorithm. */
+ bool flow_exists; /* Ensures flows are only deleted once. */
struct odputil_keybuf key_buf; /* Memory for 'key'. */
struct xlate_cache *xcache; /* Cache for xlate entries that
struct hmap *);
static void handle_upcalls(struct handler *, struct hmap *, struct upcall *,
size_t n_upcalls);
+static void udpif_stop_threads(struct udpif *);
+static void udpif_start_threads(struct udpif *, size_t n_handlers,
+ size_t n_revalidators);
static void *udpif_flow_dumper(void *);
static void *udpif_upcall_handler(void *);
static void *udpif_revalidator(void *);
void
udpif_destroy(struct udpif *udpif)
{
- udpif_set_threads(udpif, 0, 0);
- udpif_flush(udpif);
+ udpif_stop_threads(udpif);
list_remove(&udpif->list_node);
latch_destroy(&udpif->exit_latch);
free(udpif);
}
-/* Tells 'udpif' how many threads it should use to handle upcalls. Disables
- * all threads if 'n_handlers' and 'n_revalidators' is zero. 'udpif''s
- * datapath handle must have packet reception enabled before starting threads.
- */
-void
-udpif_set_threads(struct udpif *udpif, size_t n_handlers,
- size_t n_revalidators)
+/* Stops the handler and revalidator threads, must be enclosed in
+ * ovsrcu quiescent state unless when destroying udpif. */
+static void
+udpif_stop_threads(struct udpif *udpif)
{
- int error;
-
- ovsrcu_quiesce_start();
- /* Stop the old threads (if any). */
- if (udpif->handlers &&
- (udpif->n_handlers != n_handlers
- || udpif->n_revalidators != n_revalidators)) {
+ if (udpif && (udpif->n_handlers != 0 || udpif->n_revalidators != 0)) {
size_t i;
latch_set(&udpif->exit_latch);
udpif->handlers = NULL;
udpif->n_handlers = 0;
}
+}
- error = dpif_handlers_set(udpif->dpif, n_handlers);
- if (error) {
- VLOG_ERR("failed to configure handlers in dpif %s: %s",
- dpif_name(udpif->dpif), ovs_strerror(error));
- return;
- }
-
- /* Start new threads (if necessary). */
- if (!udpif->handlers && n_handlers) {
+/* Starts the handler and revalidator threads, must be enclosed in
+ * ovsrcu quiescent state. */
+static void
+udpif_start_threads(struct udpif *udpif, size_t n_handlers,
+ size_t n_revalidators)
+{
+ if (udpif && (!udpif->handlers && !udpif->revalidators)) {
size_t i;
udpif->n_handlers = n_handlers;
}
xpthread_create(&udpif->flow_dumper, NULL, udpif_flow_dumper, udpif);
}
+}
+
+/* Tells 'udpif' how many threads it should use to handle upcalls.
+ * 'n_handlers' and 'n_revalidators' can never be zero. 'udpif''s
+ * datapath handle must have packet reception enabled before starting
+ * threads. */
+void
+udpif_set_threads(struct udpif *udpif, size_t n_handlers,
+ size_t n_revalidators)
+{
+ int error;
+
+ ovs_assert(udpif);
+ ovs_assert(n_handlers && n_revalidators);
+
+ ovsrcu_quiesce_start();
+ if (udpif->n_handlers != n_handlers
+ || udpif->n_revalidators != n_revalidators) {
+ udpif_stop_threads(udpif);
+ }
+ error = dpif_handlers_set(udpif->dpif, n_handlers);
+ if (error) {
+ VLOG_ERR("failed to configure handlers in dpif %s: %s",
+ dpif_name(udpif->dpif), ovs_strerror(error));
+ return;
+ }
+
+ if (!udpif->handlers && !udpif->revalidators) {
+ udpif_start_threads(udpif, n_handlers, n_revalidators);
+ }
ovsrcu_quiesce_end();
}
* its main loop once. */
size_t n_handlers = udpif->n_handlers;
size_t n_revalidators = udpif->n_revalidators;
- udpif_set_threads(udpif, 0, 0);
- udpif_set_threads(udpif, n_handlers, n_revalidators);
+
+ ovsrcu_quiesce_start();
+ udpif_stop_threads(udpif);
+ udpif_start_threads(udpif, n_handlers, n_revalidators);
+ ovsrcu_quiesce_end();
}
/* Notifies 'udpif' that something changed which may render previous
n_handlers = udpif->n_handlers;
n_revalidators = udpif->n_revalidators;
- udpif_set_threads(udpif, 0, 0);
+ ovsrcu_quiesce_start();
+
+ udpif_stop_threads(udpif);
dpif_flow_flush(udpif->dpif);
- udpif_set_threads(udpif, n_handlers, n_revalidators);
+ udpif_start_threads(udpif, n_handlers, n_revalidators);
+
+ ovsrcu_quiesce_end();
}
/* Removes all flows from all datapaths. */
ukey->key_len = key_len;
ukey->mark = false;
+ ukey->flow_exists = true;
ukey->created = used ? used : time_msec();
memset(&ukey->stats, 0, sizeof ukey->stats);
ukey->xcache = NULL;
xoutp = NULL;
actions = NULL;
netflow = NULL;
- may_learn = push.n_packets > 0;
/* If we don't need to revalidate, we can simply push the stats contained
* in the udump, otherwise we'll have to get the actions so we can check
goto exit;
}
+ may_learn = push.n_packets > 0;
if (ukey->xcache && !udump->need_revalidate) {
xlate_push_stats(ukey->xcache, may_learn, &push);
ok = true;
used = ukey->created;
}
+ if (ukey && (ukey->mark || !ukey->flow_exists)) {
+ /* The flow has already been dumped. This can occasionally occur
+ * if the datapath is changed in the middle of a flow dump. Rather
+ * than perform the same work twice, skip the flow this time. */
+ COVERAGE_INC(upcall_duplicate_flow);
+ continue;
+ }
+
if (must_del || (used && used < now - max_idle)) {
struct dump_op *dop = &ops[n_ops++];
+ if (ukey) {
+ ukey->flow_exists = false;
+ }
dump_op_init(dop, udump->key, udump->key_len, ukey, udump);
continue;
}
ukey->mark = true;
if (!revalidate_ukey(udpif, udump, ukey)) {
+ ukey->flow_exists = false;
dpif_flow_del(udpif->dpif, udump->key, udump->key_len, NULL);
- ukey_delete(revalidator, ukey);
+ /* The ukey will be cleaned up by revalidator_sweep().
+ * This helps to avoid deleting the same flow twice. */
}
list_remove(&udump->list_node);
HMAP_FOR_EACH_SAFE (ukey, next, hmap_node, &revalidator->ukeys) {
if (!purge && ukey->mark) {
ukey->mark = false;
+ } else if (!ukey->flow_exists) {
+ ukey_delete(revalidator, ukey);
} else {
struct dump_op *op = &ops[n_ops++];