Message ID | 20200824073602.70812-3-xiangxia.m.yue@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | net: openvswitch: improve codes | expand |
From: xiangxia.m.yue@gmail.com Date: Mon, 24 Aug 2020 15:36:01 +0800 > To avoid a bug when deleting flows in the future, add > BUG_ON in flush flows function. BUG_ON() is too severe, I think WARN_ON() or similar are sufficient because the kernel can try to continue operating if this condition is detected. And you can force the values to zero in such a situation. Thank you.
On Mon, Aug 24, 2020 at 9:09 PM David Miller <davem@davemloft.net> wrote: > > From: xiangxia.m.yue@gmail.com > Date: Mon, 24 Aug 2020 15:36:01 +0800 > > > To avoid a bug when deleting flows in the future, add > > BUG_ON in flush flows function. > > BUG_ON() is too severe, I think WARN_ON() or similar are sufficient > because the kernel can try to continue operating if this condition is > detected. > > And you can force the values to zero in such a situation. Thanks, David will be changed to: diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index 0473758035b5..5378282e1d13 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -492,8 +492,11 @@ void table_instance_flow_flush(struct flow_table *table, } } - BUG_ON(table->count != 0); - BUG_ON(table->ufid_count != 0); + if (WARN_ON(table->count != 0 || + table->ufid_count != 0)) { + table->count = 0; + table->ufid_count = 0 + } } > Thank you.
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index 4b7ab62d0e1a..f8a21dd80e72 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -459,18 +459,14 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu) 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) + struct sw_flow *flow) { hlist_del_rcu(&flow->flow_table.node[ti->node_ver]); - if (count) - table->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--; + table->ufid_count--; } flow_mask_remove(table, flow->mask); @@ -495,10 +491,13 @@ void table_instance_flow_flush(struct flow_table *table, flow_table.node[ti->node_ver]) { table_instance_flow_free(table, ti, ufid_ti, - flow, false); + flow); ovs_flow_free(flow, true); } } + + BUG_ON(table->count != 0); + BUG_ON(table->ufid_count != 0); } static void table_instance_destroy(struct table_instance *ti, @@ -635,8 +634,6 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table) rcu_assign_pointer(flow_table->ti, new_ti); rcu_assign_pointer(flow_table->ufid_ti, new_ufid_ti); flow_table->last_rehash = jiffies; - flow_table->count = 0; - flow_table->ufid_count = 0; table_instance_flow_flush(flow_table, old_ti, old_ufid_ti); table_instance_destroy(old_ti, old_ufid_ti); @@ -954,7 +951,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); - table_instance_flow_free(table, ti, ufid_ti, flow, true); + table_instance_flow_free(table, ti, ufid_ti, flow); } static struct sw_flow_mask *mask_alloc(void)