net: openvswitch: introduce common code for flushing flows

To avoid some issues, for example RCU usage warning and double free,
we should flush the flows under ovs_lock. This patch refactors
table_instance_destroy and introduces table_instance_flow_flush
which can be invoked by __dp_destroy or ovs_flow_tbl_flush.

Fixes: 50b0e61b32ee ("net: openvswitch: fix possible memleak on destroy flow-table")
Reported-by: Johan Knöös <jknoos@google.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-August/050489.html
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 42f8cc70bb2cb7790f4e48fc8353e9c340beb2c6..6e47ef7ef0363059bf7f7e02ee28079c7fbfda54 100644 (file)
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1756,6 +1756,7 @@ err:
 /* Called with ovs_mutex. */
 static void __dp_destroy(struct datapath *dp)
 {
+       struct flow_table *table = &dp->table;
        int i;
 
        for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++) {
@@ -1774,7 +1775,14 @@ static void __dp_destroy(struct datapath *dp)
         */
        ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL));
 
-       /* RCU destroy the flow table */
+       /* Flush sw_flow in the tables. RCU cb only releases resource
+        * such as dp, ports and tables. That may avoid some issues
+        * such as RCU usage warning.
+        */
+       table_instance_flow_flush(table, ovsl_dereference(table->ti),
+                                 ovsl_dereference(table->ufid_ti));
+
+       /* RCU destroy the ports, meters and flow tables. */
        call_rcu(&dp->rcu, destroy_dp_rcu);
 }
 

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 8c12675cbb679efc2a69a3248d2ebe56d279fd1b..e2235849a57e9c6ff19fca796af116823293b130 100644 (file)
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -473,19 +473,15 @@ static void table_instance_flow_free(struct flow_table *table,
        flow_mask_remove(table, flow->mask);
 }
 
-static void table_instance_destroy(struct flow_table *table,
-                                  struct table_instance *ti,
-                                  struct table_instance *ufid_ti,
-                                  bool deferred)
+/* Must be called with OVS mutex held. */
+void table_instance_flow_flush(struct flow_table *table,
+                              struct table_instance *ti,
+                              struct table_instance *ufid_ti)
 {
        int i;
 
-       if (!ti)
-               return;
-
-       BUG_ON(!ufid_ti);
        if (ti->keep_flows)
-               goto skip_flows;
+               return;
 
        for (i = 0; i < ti->n_buckets; i++) {
                struct sw_flow *flow;
@@ -497,18 +493,16 @@ static void table_instance_destroy(struct flow_table *table,
 
                        table_instance_flow_free(table, ti, ufid_ti,
                                                 flow, false);
-                       ovs_flow_free(flow, deferred);
+                       ovs_flow_free(flow, true);
                }
        }
+}
 
-skip_flows:
-       if (deferred) {
-               call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb);
-               call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb);
-       } else {
-               __table_instance_destroy(ti);
-               __table_instance_destroy(ufid_ti);
-       }
+static void table_instance_destroy(struct table_instance *ti,
+                                  struct table_instance *ufid_ti)
+{
+       call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb);
+       call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb);
 }
 
 /* No need for locking this function is called from RCU callback or
@@ -523,7 +517,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table)
 
        call_rcu(&mc->rcu, mask_cache_rcu_cb);
        call_rcu(&ma->rcu, mask_array_rcu_cb);
-       table_instance_destroy(table, ti, ufid_ti, false);
+       table_instance_destroy(ti, ufid_ti);
 }
 
 struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti,
@@ -641,7 +635,8 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table)
        flow_table->count = 0;
        flow_table->ufid_count = 0;
 
-       table_instance_destroy(flow_table, old_ti, old_ufid_ti, true);
+       table_instance_flow_flush(flow_table, old_ti, old_ufid_ti);
+       table_instance_destroy(old_ti, old_ufid_ti);
        return 0;
 
 err_free_ti:

diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
index 74ce48fecba93a0907550746c6b6511c018f5342..6e7d4ac5935316c8da89d9e4dab0826eb21bc3c8 100644 (file)
--- a/net/openvswitch/flow_table.h
+++ b/net/openvswitch/flow_table.h
@@ -105,5 +105,8 @@ void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
                       bool full, const struct sw_flow_mask *mask);
 
 void ovs_flow_masks_rebalance(struct flow_table *table);
+void table_instance_flow_flush(struct flow_table *table,
+                              struct table_instance *ti,
+                              struct table_instance *ufid_ti);
 
 #endif /* flow_table.h */