| Message ID | 20200916173311.30956-12-gvrose8192@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Linux kernels up to 5.8.x | expand |
On Wed, Sep 16, 2020 at 10:33 AM Greg Rose <gvrose8192@gmail.com> wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > Upstream commit: > commit 50b0e61b32ee890a75b4377d5fbe770a86d6a4c1 > Author: Tonghao Zhang <xiangxia.m.yue@gmail.com> > Date: Fri Nov 1 22:23:52 2019 +0800 > > net: openvswitch: fix possible memleak on destroy flow-table > > 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.yue@gmail.com> > Tested-by: Greg Rose <gvrose8192@gmail.com> > Acked-by: Pravin B Shelar <pshelar@ovn.org> > Signed-off-by: David S. Miller <davem@davemloft.net> > > Added additional compat layer fixup for WRITE_ONCE() > > Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com> > Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > Signed-off-by: Greg Rose <gvrose8192@gmail.com> > --- > datapath/flow_table.c | 186 +++++++++--------- > .../linux/compat/include/linux/compiler.h | 13 ++ > 2 files changed, 111 insertions(+), 88 deletions(-) > > diff --git a/datapath/flow_table.c b/datapath/flow_table.c > index ca2efe94d..bd05dd394 100644 > --- a/datapath/flow_table.c > +++ b/datapath/flow_table.c > @@ -234,6 +234,74 @@ static int tbl_mask_array_realloc(struct flow_table *tbl, int size) > return 0; > } > > +static int tbl_mask_array_add_mask(struct flow_table *tbl, > + struct sw_flow_mask *new) > +{ > + struct mask_array *ma = ovsl_dereference(tbl->mask_array); > + int err, ma_count = READ_ONCE(ma->count); > + > + if (ma_count >= ma->max) { > + err = tbl_mask_array_realloc(tbl, ma->max + > + MASK_ARRAY_SIZE_MIN); > + if (err) > + return err; > + > + ma = ovsl_dereference(tbl->mask_array); > + } > + > + BUG_ON(ovsl_dereference(ma->masks[ma_count])); > + > + rcu_assign_pointer(ma->masks[ma_count], new); > + WRITE_ONCE(ma->count, ma_count +1); > + > + return 0; > +} > + > +static void tbl_mask_array_del_mask(struct flow_table *tbl, > + struct sw_flow_mask *mask) > +{ > + struct mask_array *ma = ovsl_dereference(tbl->mask_array); > + int i, ma_count = READ_ONCE(ma->count); > + > + /* Remove the deleted mask pointers from the array */ > + for (i = 0; i < ma_count; i++) { > + if (mask == ovsl_dereference(ma->masks[i])) > + goto found; > + } > + > + BUG(); > + return; > + > +found: > + WRITE_ONCE(ma->count, ma_count -1); > + > + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); > + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); > + > + kfree_rcu(mask, rcu); > + > + /* Shrink the mask array if necessary. */ > + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && > + ma_count <= (ma->max / 3)) > + tbl_mask_array_realloc(tbl, ma->max / 2); > +} > + > +/* Remove 'mask' from the mask list, if it is not needed any more. */ > +static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) > +{ > + if (mask) { > + /* ovs-lock is required to protect mask-refcount and > + * mask list. > + */ > + ASSERT_OVSL(); > + BUG_ON(!mask->ref_count); > + mask->ref_count--; > + > + if (!mask->ref_count) > + tbl_mask_array_del_mask(tbl, mask); > + } > +} > + > int ovs_flow_tbl_init(struct flow_table *table) > { > struct table_instance *ti, *ufid_ti; > @@ -280,7 +348,28 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) > __table_instance_destroy(ti); > } > > -static void table_instance_destroy(struct table_instance *ti, > +static void table_instance_flow_free(struct flow_table *table, > + struct table_instance *ti, > + struct table_instance *ufid_ti, > + struct sw_flow *flow, > + bool count) > +{ > + hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); > + if (count) > + table->count--; > + > + if (ovs_identifier_is_ufid(&flow->id)) { > + hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); > + > + if (count) > + table->ufid_count--; > + } > + > + 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) > { > @@ -297,13 +386,12 @@ static void table_instance_destroy(struct table_instance *ti, > struct sw_flow *flow; > struct hlist_head *head = &ti->buckets[i]; > struct hlist_node *n; > - int ver = ti->node_ver; > - int ufid_ver = ufid_ti->node_ver; > > - hlist_for_each_entry_safe(flow, n, head, flow_table.node[ver]) { > - hlist_del_rcu(&flow->flow_table.node[ver]); > - if (ovs_identifier_is_ufid(&flow->id)) > - hlist_del_rcu(&flow->ufid_table.node[ufid_ver]); > + hlist_for_each_entry_safe(flow, n, head, > + flow_table.node[ti->node_ver]) { > + > + table_instance_flow_free(table, ti, ufid_ti, > + flow, false); > ovs_flow_free(flow, deferred); > } > } > @@ -328,7 +416,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table) > > free_percpu(table->mask_cache); > kfree(rcu_dereference_raw(table->mask_array)); > - table_instance_destroy(ti, ufid_ti, false); > + table_instance_destroy(table, ti, ufid_ti, false); > } > > struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, > @@ -444,7 +532,7 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) > flow_table->count = 0; > flow_table->ufid_count = 0; > > - table_instance_destroy(old_ti, old_ufid_ti, true); > + table_instance_destroy(flow_table, old_ti, old_ufid_ti, true); > return 0; > > err_free_ti: > @@ -722,51 +810,6 @@ static struct table_instance *table_instance_expand(struct table_instance *ti, > return table_instance_rehash(ti, ti->n_buckets * 2, ufid); > } > > -static void tbl_mask_array_del_mask(struct flow_table *tbl, > - struct sw_flow_mask *mask) > -{ > - struct mask_array *ma = ovsl_dereference(tbl->mask_array); > - int i, ma_count = READ_ONCE(ma->count); > - > - /* Remove the deleted mask pointers from the array */ > - for (i = 0; i < ma_count; i++) { > - if (mask == ovsl_dereference(ma->masks[i])) > - goto found; > - } > - > - BUG(); > - return; > - > -found: > - WRITE_ONCE(ma->count, ma_count -1); > - > - rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); > - RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); > - > - kfree_rcu(mask, rcu); > - > - /* Shrink the mask array if necessary. */ > - if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && > - ma_count <= (ma->max / 3)) > - tbl_mask_array_realloc(tbl, ma->max / 2); > -} > - > -/* Remove 'mask' from the mask list, if it is not needed any more. */ > -static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) > -{ > - if (mask) { > - /* ovs-lock is required to protect mask-refcount and > - * mask list. > - */ > - ASSERT_OVSL(); > - BUG_ON(!mask->ref_count); > - mask->ref_count--; > - > - if (!mask->ref_count) > - tbl_mask_array_del_mask(tbl, mask); > - } > -} > - > /* Must be called with OVS mutex held. */ > void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) > { > @@ -774,17 +817,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) > struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti); > > BUG_ON(table->count == 0); > - hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); > - table->count--; > - if (ovs_identifier_is_ufid(&flow->id)) { > - hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); > - table->ufid_count--; > - } > - > - /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be > - * accessible as long as the RCU read lock is held. > - */ > - flow_mask_remove(table, flow->mask); > + table_instance_flow_free(table, ti, ufid_ti, flow, true); > } > > static struct sw_flow_mask *mask_alloc(void) > @@ -827,29 +860,6 @@ static struct sw_flow_mask *flow_mask_find(const struct flow_table *tbl, > return NULL; > } > > -static int tbl_mask_array_add_mask(struct flow_table *tbl, > - struct sw_flow_mask *new) > -{ > - struct mask_array *ma = ovsl_dereference(tbl->mask_array); > - int err, ma_count = READ_ONCE(ma->count); > - > - if (ma_count >= ma->max) { > - err = tbl_mask_array_realloc(tbl, ma->max + > - MASK_ARRAY_SIZE_MIN); > - if (err) > - return err; > - > - ma = ovsl_dereference(tbl->mask_array); > - } > - > - BUG_ON(ovsl_dereference(ma->masks[ma_count])); > - > - rcu_assign_pointer(ma->masks[ma_count], new); > - WRITE_ONCE(ma->count, ma_count +1); > - > - return 0; > -} > - > /* Add 'mask' into the mask list, if it is not already there. */ > static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow, > const struct sw_flow_mask *new) > diff --git a/datapath/linux/compat/include/linux/compiler.h b/datapath/linux/compat/include/linux/compiler.h > index 65f3ba6f4..e15c61d2a 100644 > --- a/datapath/linux/compat/include/linux/compiler.h > +++ b/datapath/linux/compat/include/linux/compiler.h > @@ -15,4 +15,17 @@ > #define READ_ONCE(x) (x) > #endif > > +#ifndef WRITE_ONCE > +#define __WRITE_ONCE(x, val) \ > +do { \ > + *(volatile typeof(x) *)&(x) = (val); \ > +} while (0) > + > +#define WRITE_ONCE(x, val) \ > +do { \ > + __WRITE_ONCE(x, val); \ > +} while (0) > +#endif > + > + > #endif Hi Greg, Just a minor a nit on the WRITE_ONCE() compatibility code, in the upstream kernel WRITE_ONCE is defined as #define WRITE_ONCE(x, val) \ do { \ compiletime_assert_rwonce_type(x); \ __WRITE_ONCE(x, val); \ } while (0) I guess there is a reason for you to not backport compiletime_assert_rwonce_type()? If yes, should we just define WRITE_ONCE() for simplicity? #define WRITE_ONCE(x, val) \ do { \ *(volatile typeof(x) *)&(x) = (val); \ } while (0) Thanks, -Yi-Hung
On 9/29/2020 2:43 PM, Yi-Hung Wei wrote: > On Wed, Sep 16, 2020 at 10:33 AM Greg Rose <gvrose8192@gmail.com> wrote: >> >> From: Tonghao Zhang <xiangxia.m.yue@gmail.com> >> >> Upstream commit: >> commit 50b0e61b32ee890a75b4377d5fbe770a86d6a4c1 >> Author: Tonghao Zhang <xiangxia.m.yue@gmail.com> >> Date: Fri Nov 1 22:23:52 2019 +0800 >> >> net: openvswitch: fix possible memleak on destroy flow-table >> >> 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.yue@gmail.com> >> Tested-by: Greg Rose <gvrose8192@gmail.com> >> Acked-by: Pravin B Shelar <pshelar@ovn.org> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> >> Added additional compat layer fixup for WRITE_ONCE() >> >> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com> >> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >> --- >> datapath/flow_table.c | 186 +++++++++--------- >> .../linux/compat/include/linux/compiler.h | 13 ++ >> 2 files changed, 111 insertions(+), 88 deletions(-) >> >> diff --git a/datapath/flow_table.c b/datapath/flow_table.c >> index ca2efe94d..bd05dd394 100644 >> --- a/datapath/flow_table.c >> +++ b/datapath/flow_table.c >> @@ -234,6 +234,74 @@ static int tbl_mask_array_realloc(struct flow_table *tbl, int size) >> return 0; >> } >> >> +static int tbl_mask_array_add_mask(struct flow_table *tbl, >> + struct sw_flow_mask *new) >> +{ >> + struct mask_array *ma = ovsl_dereference(tbl->mask_array); >> + int err, ma_count = READ_ONCE(ma->count); >> + >> + if (ma_count >= ma->max) { >> + err = tbl_mask_array_realloc(tbl, ma->max + >> + MASK_ARRAY_SIZE_MIN); >> + if (err) >> + return err; >> + >> + ma = ovsl_dereference(tbl->mask_array); >> + } >> + >> + BUG_ON(ovsl_dereference(ma->masks[ma_count])); >> + >> + rcu_assign_pointer(ma->masks[ma_count], new); >> + WRITE_ONCE(ma->count, ma_count +1); >> + >> + return 0; >> +} >> + >> +static void tbl_mask_array_del_mask(struct flow_table *tbl, >> + struct sw_flow_mask *mask) >> +{ >> + struct mask_array *ma = ovsl_dereference(tbl->mask_array); >> + int i, ma_count = READ_ONCE(ma->count); >> + >> + /* Remove the deleted mask pointers from the array */ >> + for (i = 0; i < ma_count; i++) { >> + if (mask == ovsl_dereference(ma->masks[i])) >> + goto found; >> + } >> + >> + BUG(); >> + return; >> + >> +found: >> + WRITE_ONCE(ma->count, ma_count -1); >> + >> + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); >> + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); >> + >> + kfree_rcu(mask, rcu); >> + >> + /* Shrink the mask array if necessary. */ >> + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && >> + ma_count <= (ma->max / 3)) >> + tbl_mask_array_realloc(tbl, ma->max / 2); >> +} >> + >> +/* Remove 'mask' from the mask list, if it is not needed any more. */ >> +static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) >> +{ >> + if (mask) { >> + /* ovs-lock is required to protect mask-refcount and >> + * mask list. >> + */ >> + ASSERT_OVSL(); >> + BUG_ON(!mask->ref_count); >> + mask->ref_count--; >> + >> + if (!mask->ref_count) >> + tbl_mask_array_del_mask(tbl, mask); >> + } >> +} >> + >> int ovs_flow_tbl_init(struct flow_table *table) >> { >> struct table_instance *ti, *ufid_ti; >> @@ -280,7 +348,28 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) >> __table_instance_destroy(ti); >> } >> >> -static void table_instance_destroy(struct table_instance *ti, >> +static void table_instance_flow_free(struct flow_table *table, >> + struct table_instance *ti, >> + struct table_instance *ufid_ti, >> + struct sw_flow *flow, >> + bool count) >> +{ >> + hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); >> + if (count) >> + table->count--; >> + >> + if (ovs_identifier_is_ufid(&flow->id)) { >> + hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); >> + >> + if (count) >> + table->ufid_count--; >> + } >> + >> + 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) >> { >> @@ -297,13 +386,12 @@ static void table_instance_destroy(struct table_instance *ti, >> struct sw_flow *flow; >> struct hlist_head *head = &ti->buckets[i]; >> struct hlist_node *n; >> - int ver = ti->node_ver; >> - int ufid_ver = ufid_ti->node_ver; >> >> - hlist_for_each_entry_safe(flow, n, head, flow_table.node[ver]) { >> - hlist_del_rcu(&flow->flow_table.node[ver]); >> - if (ovs_identifier_is_ufid(&flow->id)) >> - hlist_del_rcu(&flow->ufid_table.node[ufid_ver]); >> + hlist_for_each_entry_safe(flow, n, head, >> + flow_table.node[ti->node_ver]) { >> + >> + table_instance_flow_free(table, ti, ufid_ti, >> + flow, false); >> ovs_flow_free(flow, deferred); >> } >> } >> @@ -328,7 +416,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table) >> >> free_percpu(table->mask_cache); >> kfree(rcu_dereference_raw(table->mask_array)); >> - table_instance_destroy(ti, ufid_ti, false); >> + table_instance_destroy(table, ti, ufid_ti, false); >> } >> >> struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, >> @@ -444,7 +532,7 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) >> flow_table->count = 0; >> flow_table->ufid_count = 0; >> >> - table_instance_destroy(old_ti, old_ufid_ti, true); >> + table_instance_destroy(flow_table, old_ti, old_ufid_ti, true); >> return 0; >> >> err_free_ti: >> @@ -722,51 +810,6 @@ static struct table_instance *table_instance_expand(struct table_instance *ti, >> return table_instance_rehash(ti, ti->n_buckets * 2, ufid); >> } >> >> -static void tbl_mask_array_del_mask(struct flow_table *tbl, >> - struct sw_flow_mask *mask) >> -{ >> - struct mask_array *ma = ovsl_dereference(tbl->mask_array); >> - int i, ma_count = READ_ONCE(ma->count); >> - >> - /* Remove the deleted mask pointers from the array */ >> - for (i = 0; i < ma_count; i++) { >> - if (mask == ovsl_dereference(ma->masks[i])) >> - goto found; >> - } >> - >> - BUG(); >> - return; >> - >> -found: >> - WRITE_ONCE(ma->count, ma_count -1); >> - >> - rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); >> - RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); >> - >> - kfree_rcu(mask, rcu); >> - >> - /* Shrink the mask array if necessary. */ >> - if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && >> - ma_count <= (ma->max / 3)) >> - tbl_mask_array_realloc(tbl, ma->max / 2); >> -} >> - >> -/* Remove 'mask' from the mask list, if it is not needed any more. */ >> -static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) >> -{ >> - if (mask) { >> - /* ovs-lock is required to protect mask-refcount and >> - * mask list. >> - */ >> - ASSERT_OVSL(); >> - BUG_ON(!mask->ref_count); >> - mask->ref_count--; >> - >> - if (!mask->ref_count) >> - tbl_mask_array_del_mask(tbl, mask); >> - } >> -} >> - >> /* Must be called with OVS mutex held. */ >> void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) >> { >> @@ -774,17 +817,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) >> struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti); >> >> BUG_ON(table->count == 0); >> - hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); >> - table->count--; >> - if (ovs_identifier_is_ufid(&flow->id)) { >> - hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); >> - table->ufid_count--; >> - } >> - >> - /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be >> - * accessible as long as the RCU read lock is held. >> - */ >> - flow_mask_remove(table, flow->mask); >> + table_instance_flow_free(table, ti, ufid_ti, flow, true); >> } >> >> static struct sw_flow_mask *mask_alloc(void) >> @@ -827,29 +860,6 @@ static struct sw_flow_mask *flow_mask_find(const struct flow_table *tbl, >> return NULL; >> } >> >> -static int tbl_mask_array_add_mask(struct flow_table *tbl, >> - struct sw_flow_mask *new) >> -{ >> - struct mask_array *ma = ovsl_dereference(tbl->mask_array); >> - int err, ma_count = READ_ONCE(ma->count); >> - >> - if (ma_count >= ma->max) { >> - err = tbl_mask_array_realloc(tbl, ma->max + >> - MASK_ARRAY_SIZE_MIN); >> - if (err) >> - return err; >> - >> - ma = ovsl_dereference(tbl->mask_array); >> - } >> - >> - BUG_ON(ovsl_dereference(ma->masks[ma_count])); >> - >> - rcu_assign_pointer(ma->masks[ma_count], new); >> - WRITE_ONCE(ma->count, ma_count +1); >> - >> - return 0; >> -} >> - >> /* Add 'mask' into the mask list, if it is not already there. */ >> static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow, >> const struct sw_flow_mask *new) >> diff --git a/datapath/linux/compat/include/linux/compiler.h b/datapath/linux/compat/include/linux/compiler.h >> index 65f3ba6f4..e15c61d2a 100644 >> --- a/datapath/linux/compat/include/linux/compiler.h >> +++ b/datapath/linux/compat/include/linux/compiler.h >> @@ -15,4 +15,17 @@ >> #define READ_ONCE(x) (x) >> #endif >> >> +#ifndef WRITE_ONCE >> +#define __WRITE_ONCE(x, val) \ >> +do { \ >> + *(volatile typeof(x) *)&(x) = (val); \ >> +} while (0) >> + >> +#define WRITE_ONCE(x, val) \ >> +do { \ >> + __WRITE_ONCE(x, val); \ >> +} while (0) >> +#endif >> + >> + >> #endif > > Hi Greg, > > Just a minor a nit on the WRITE_ONCE() compatibility code, in the > upstream kernel WRITE_ONCE is defined as > > #define WRITE_ONCE(x, val) \ > do { \ > compiletime_assert_rwonce_type(x); \ > __WRITE_ONCE(x, val); \ > } while (0) > > I guess there is a reason for you to not backport > compiletime_assert_rwonce_type()? If yes, should we just define > WRITE_ONCE() for simplicity? > #define WRITE_ONCE(x, val) \ > do { \ > *(volatile typeof(x) *)&(x) = (val); \ > } while (0) > > Thanks, > > -Yi-Hung > Sure, sounds good to me. I'll fix this one up for the next patch series. - Greg
diff --git a/datapath/flow_table.c b/datapath/flow_table.c index ca2efe94d..bd05dd394 100644 --- a/datapath/flow_table.c +++ b/datapath/flow_table.c @@ -234,6 +234,74 @@ static int tbl_mask_array_realloc(struct flow_table *tbl, int size) return 0; } +static int tbl_mask_array_add_mask(struct flow_table *tbl, + struct sw_flow_mask *new) +{ + struct mask_array *ma = ovsl_dereference(tbl->mask_array); + int err, ma_count = READ_ONCE(ma->count); + + if (ma_count >= ma->max) { + err = tbl_mask_array_realloc(tbl, ma->max + + MASK_ARRAY_SIZE_MIN); + if (err) + return err; + + ma = ovsl_dereference(tbl->mask_array); + } + + BUG_ON(ovsl_dereference(ma->masks[ma_count])); + + rcu_assign_pointer(ma->masks[ma_count], new); + WRITE_ONCE(ma->count, ma_count +1); + + return 0; +} + +static void tbl_mask_array_del_mask(struct flow_table *tbl, + struct sw_flow_mask *mask) +{ + struct mask_array *ma = ovsl_dereference(tbl->mask_array); + int i, ma_count = READ_ONCE(ma->count); + + /* Remove the deleted mask pointers from the array */ + for (i = 0; i < ma_count; i++) { + if (mask == ovsl_dereference(ma->masks[i])) + goto found; + } + + BUG(); + return; + +found: + WRITE_ONCE(ma->count, ma_count -1); + + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); + + kfree_rcu(mask, rcu); + + /* Shrink the mask array if necessary. */ + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && + ma_count <= (ma->max / 3)) + tbl_mask_array_realloc(tbl, ma->max / 2); +} + +/* Remove 'mask' from the mask list, if it is not needed any more. */ +static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) +{ + if (mask) { + /* ovs-lock is required to protect mask-refcount and + * mask list. + */ + ASSERT_OVSL(); + BUG_ON(!mask->ref_count); + mask->ref_count--; + + if (!mask->ref_count) + tbl_mask_array_del_mask(tbl, mask); + } +} + int ovs_flow_tbl_init(struct flow_table *table) { struct table_instance *ti, *ufid_ti; @@ -280,7 +348,28 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) __table_instance_destroy(ti); } -static void table_instance_destroy(struct table_instance *ti, +static void table_instance_flow_free(struct flow_table *table, + struct table_instance *ti, + struct table_instance *ufid_ti, + struct sw_flow *flow, + bool count) +{ + hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); + if (count) + table->count--; + + if (ovs_identifier_is_ufid(&flow->id)) { + hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); + + if (count) + table->ufid_count--; + } + + 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) { @@ -297,13 +386,12 @@ static void table_instance_destroy(struct table_instance *ti, struct sw_flow *flow; struct hlist_head *head = &ti->buckets[i]; struct hlist_node *n; - int ver = ti->node_ver; - int ufid_ver = ufid_ti->node_ver; - hlist_for_each_entry_safe(flow, n, head, flow_table.node[ver]) { - hlist_del_rcu(&flow->flow_table.node[ver]); - if (ovs_identifier_is_ufid(&flow->id)) - hlist_del_rcu(&flow->ufid_table.node[ufid_ver]); + hlist_for_each_entry_safe(flow, n, head, + flow_table.node[ti->node_ver]) { + + table_instance_flow_free(table, ti, ufid_ti, + flow, false); ovs_flow_free(flow, deferred); } } @@ -328,7 +416,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table) free_percpu(table->mask_cache); kfree(rcu_dereference_raw(table->mask_array)); - table_instance_destroy(ti, ufid_ti, false); + table_instance_destroy(table, ti, ufid_ti, false); } struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti, @@ -444,7 +532,7 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) flow_table->count = 0; flow_table->ufid_count = 0; - table_instance_destroy(old_ti, old_ufid_ti, true); + table_instance_destroy(flow_table, old_ti, old_ufid_ti, true); return 0; err_free_ti: @@ -722,51 +810,6 @@ static struct table_instance *table_instance_expand(struct table_instance *ti, return table_instance_rehash(ti, ti->n_buckets * 2, ufid); } -static void tbl_mask_array_del_mask(struct flow_table *tbl, - struct sw_flow_mask *mask) -{ - struct mask_array *ma = ovsl_dereference(tbl->mask_array); - int i, ma_count = READ_ONCE(ma->count); - - /* Remove the deleted mask pointers from the array */ - for (i = 0; i < ma_count; i++) { - if (mask == ovsl_dereference(ma->masks[i])) - goto found; - } - - BUG(); - return; - -found: - WRITE_ONCE(ma->count, ma_count -1); - - rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]); - RCU_INIT_POINTER(ma->masks[ma_count -1], NULL); - - kfree_rcu(mask, rcu); - - /* Shrink the mask array if necessary. */ - if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) && - ma_count <= (ma->max / 3)) - tbl_mask_array_realloc(tbl, ma->max / 2); -} - -/* Remove 'mask' from the mask list, if it is not needed any more. */ -static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask) -{ - if (mask) { - /* ovs-lock is required to protect mask-refcount and - * mask list. - */ - ASSERT_OVSL(); - BUG_ON(!mask->ref_count); - mask->ref_count--; - - if (!mask->ref_count) - tbl_mask_array_del_mask(tbl, mask); - } -} - /* Must be called with OVS mutex held. */ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) { @@ -774,17 +817,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow) struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti); BUG_ON(table->count == 0); - hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); - table->count--; - if (ovs_identifier_is_ufid(&flow->id)) { - hlist_del_rcu(&flow->ufid_table.node[ufid_ti->node_ver]); - table->ufid_count--; - } - - /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be - * accessible as long as the RCU read lock is held. - */ - flow_mask_remove(table, flow->mask); + table_instance_flow_free(table, ti, ufid_ti, flow, true); } static struct sw_flow_mask *mask_alloc(void) @@ -827,29 +860,6 @@ static struct sw_flow_mask *flow_mask_find(const struct flow_table *tbl, return NULL; } -static int tbl_mask_array_add_mask(struct flow_table *tbl, - struct sw_flow_mask *new) -{ - struct mask_array *ma = ovsl_dereference(tbl->mask_array); - int err, ma_count = READ_ONCE(ma->count); - - if (ma_count >= ma->max) { - err = tbl_mask_array_realloc(tbl, ma->max + - MASK_ARRAY_SIZE_MIN); - if (err) - return err; - - ma = ovsl_dereference(tbl->mask_array); - } - - BUG_ON(ovsl_dereference(ma->masks[ma_count])); - - rcu_assign_pointer(ma->masks[ma_count], new); - WRITE_ONCE(ma->count, ma_count +1); - - return 0; -} - /* Add 'mask' into the mask list, if it is not already there. */ static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow, const struct sw_flow_mask *new) diff --git a/datapath/linux/compat/include/linux/compiler.h b/datapath/linux/compat/include/linux/compiler.h index 65f3ba6f4..e15c61d2a 100644 --- a/datapath/linux/compat/include/linux/compiler.h +++ b/datapath/linux/compat/include/linux/compiler.h @@ -15,4 +15,17 @@ #define READ_ONCE(x) (x) #endif +#ifndef WRITE_ONCE +#define __WRITE_ONCE(x, val) \ +do { \ + *(volatile typeof(x) *)&(x) = (val); \ +} while (0) + +#define WRITE_ONCE(x, val) \ +do { \ + __WRITE_ONCE(x, val); \ +} while (0) +#endif + + #endif