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.soman@citrix.com>
[blp@nicira.com modified the patch to remove netflow_expire() and
 rewrote the commit message]
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/netflow.c b/ofproto/netflow.c
index e9382af..c7af010 100644 (file)
--- a/ofproto/netflow.c
+++ b/ofproto/netflow.c
@@ -275,19 +275,6 @@ netflow_expire__(struct netflow *nf, struct netflow_flow *nf_flow)
     nf_flow->tcp_flags = 0;
 }
 
-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)
 {
@@ -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 (file)
--- 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 906ccb4..a126210 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1317,7 +1317,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);
@@ -1402,7 +1401,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 64dfbfa..b4a1ed4 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3618,7 +3618,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);