Sorry about the delay. I took a few minutes to properly understand
the situation this morning. I applied the following form of your
patch to master and branch-2.[123]:
--8<--------------------------cut here-------------------------->8--
From: Anoob Soman <anoob.so...@citrix.com>
Date: Tue, 20 May 2014 12:40:35 +0100
Subject: [PATCH] netflow: Fold netflow_expire() into netflow_flow_clear().
netflow_flow_clear() asserted that no packets or bytes were included
in the statistics for the flow being cleared. Before threading Open
vSwitch, this assertion was always true because netflow_expire() was
always called before calling netflow_flow_clear(). Since Open
vSwitch was threaded, however, it was possible that a packet arrived
after netflow_expire() but before netflow_flow_clear(), since each of
these function separately took the netflow mutex.
This commit fixes the problem by merging netflow_expire() into
netflow_flow_clear(), under a single acquisition of the netflow
mutex.
Signed-off-by: Anoob Soman <anoob.so...@citrix.com>
[b...@nicira.com modified the patch to remove netflow_expire() and
rewrote the commit message]
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
ofproto/netflow.c | 16 +---------------
ofproto/netflow.h | 3 +--
ofproto/ofproto-dpif-upcall.c | 2 --
ofproto/ofproto-dpif-xlate.c | 1 -
4 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/ofproto/netflow.c b/ofproto/netflow.c
index e9382af..c7af010 100644
--- a/ofproto/netflow.c
+++ b/ofproto/netflow.c
@@ -276,19 +276,6 @@ netflow_expire__(struct netflow *nf, struct netflow_flow
*nf_flow)
}
void
-netflow_expire(struct netflow *nf, struct flow *flow) OVS_EXCLUDED(mutex)
-{
- struct netflow_flow *nf_flow;
-
- ovs_mutex_lock(&mutex);
- nf_flow = netflow_flow_lookup(nf, flow);
- if (nf_flow) {
- netflow_expire__(nf, nf_flow);
- }
- ovs_mutex_unlock(&mutex);
-}
-
-void
netflow_flow_clear(struct netflow *nf, struct flow *flow) OVS_EXCLUDED(mutex)
{
struct netflow_flow *nf_flow;
@@ -296,8 +283,7 @@ netflow_flow_clear(struct netflow *nf, struct flow *flow)
OVS_EXCLUDED(mutex)
ovs_mutex_lock(&mutex);
nf_flow = netflow_flow_lookup(nf, flow);
if (nf_flow) {
- ovs_assert(!nf_flow->packet_count);
- ovs_assert(!nf_flow->byte_count);
+ netflow_expire__(nf, nf_flow);
hmap_remove(&nf->flows, &nf_flow->hmap_node);
free(nf_flow);
}
diff --git a/ofproto/netflow.h b/ofproto/netflow.h
index e89b75e..94dd3ff 100644
--- a/ofproto/netflow.h
+++ b/ofproto/netflow.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2014 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -46,7 +46,6 @@ void netflow_unref(struct netflow *);
bool netflow_exists(void);
int netflow_set_options(struct netflow *, const struct netflow_options *);
-void netflow_expire(struct netflow *, struct flow *);
void netflow_run(struct netflow *);
void netflow_wait(struct netflow *);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 67a0ed8..d38b843 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1219,7 +1219,6 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
exit:
if (netflow) {
if (!ok) {
- netflow_expire(netflow, &flow);
netflow_flow_clear(netflow, &flow);
}
netflow_unref(netflow);
@@ -1304,7 +1303,6 @@ push_dump_ops__(struct udpif *udpif, struct dump_op *ops,
size_t n_ops)
xlate_actions_for_side_effects(&xin);
if (netflow) {
- netflow_expire(netflow, &flow);
netflow_flow_clear(netflow, &flow);
netflow_unref(netflow);
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eded9d8..71eaad1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3956,7 +3956,6 @@ xlate_dev_unref(struct xc_entry *entry)
static void
xlate_cache_clear_netflow(struct netflow *netflow, struct flow *flow)
{
- netflow_expire(netflow, flow);
netflow_flow_clear(netflow, flow);
netflow_unref(netflow);
free(flow);
