On Fri, Oct 18, 2019 at 6:38 AM Pravin Shelar <pshe...@ovn.org> wrote:
>
> On Wed, Oct 16, 2019 at 5:50 AM <xiangxia.m....@gmail.com> wrote:
> >
> > From: Tonghao Zhang <xiangxia.m....@gmail.com>
> >
> > When we destroy the flow tables which may contain the flow_mask,
> > so release the flow mask struct.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m....@gmail.com>
> > Tested-by: Greg Rose <gvrose8...@gmail.com>
> > ---
> > 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
....
