On Fri, Oct 18, 2019 at 6:38 AM Pravin Shelar [off-list ref] wrote:

On Wed, Oct 16, 2019 at 5:50 AM [off-list ref] wrote:

quoted

From: Tonghao Zhang <redacted>

When we destroy the flow tables which may contain the flow_mask,
so release the flow mask struct.

Signed-off-by: Tonghao Zhang <redacted>
Tested-by: Greg Rose <redacted>
---
 net/openvswitch/flow_table.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 5df5182..d5d768e 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c

@@ -295,6 +295,18 @@ static void table_instance_destroy(struct table_instance *ti,
        }
 }

+static void tbl_mask_array_destroy(struct flow_table *tbl)
+{
+       struct mask_array *ma = ovsl_dereference(tbl->mask_array);
+       int i;
+
+       /* Free the flow-mask and kfree_rcu the NULL is allowed. */
+       for (i = 0; i < ma->max; i++)
+               kfree_rcu(rcu_dereference_raw(ma->masks[i]), rcu);
+
+       kfree_rcu(rcu_dereference_raw(tbl->mask_array), rcu);
+}
+
 /* No need for locking this function is called from RCU callback or
  * error path.
  */

@@ -304,7 +316,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table)
        struct table_instance *ufid_ti = rcu_dereference_raw(table->ufid_ti);

        free_percpu(table->mask_cache);
-       kfree_rcu(rcu_dereference_raw(table->mask_array), rcu);
+       tbl_mask_array_destroy(table);
        table_instance_destroy(ti, ufid_ti, false);
 }

This should not be required. mask is linked to a flow and gets
released when flow is removed.
Does the memory leak occur when OVS module is abruptly unloaded and
userspace does not cleanup flow table?

When we destroy the ovs datapath or net namespace is destroyed , the
mask memory will be happened. The call tree:
ovs_exit_net/ovs_dp_cmd_del
-->__dp_destroy
-->destroy_dp_rcu
-->ovs_flow_tbl_destroy
-->table_instance_destroy (which don't release the mask memory because
don't call the ovs_flow_tbl_remove /flow_mask_remove directly or
indirectly).

but one thing, when we flush the flow, we don't flush the mask flow.(
If necessary, one patch should be sent)

In that case better fix could be calling ovs_flow_tbl_remove()
equivalent from table_instance_destroy when it is iterating flow
table.

I think operation of  the flow mask and flow table should use
different API, for example:
for flow mask, we use the:
-tbl_mask_array_add_mask
-tbl_mask_array_del_mask
-tbl_mask_array_alloc
-tbl_mask_array_realloc
-tbl_mask_array_destroy(this patch introduce.)

table instance:
-table_instance_alloc
-table_instance_destroy
....