| Message ID | 20200818100923.46840-3-xiangxia.m.yue@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | net: openvswitch: improve codes | expand |
From: xiangxia.m.yue@gmail.com Date: Tue, 18 Aug 2020 18:09:22 +0800 > Decrease table->count and ufid_count unconditionally, You don't explain why this is a valid transformation. Is it a bug fix? Can it never happen? What are the details and how did you determine this?
On Thu, Aug 20, 2020 at 6:52 AM David Miller <davem@davemloft.net> wrote: > From: xiangxia.m.yue@gmail.com > > Date: Tue, 18 Aug 2020 18:09:22 +0800 > > > > > Decrease table->count and ufid_count unconditionally, > > > > You don't explain why this is a valid transformation. > > > > Is it a bug fix? No > > > > Can it never happen? > > > > What are the details and how did you determine this? I want to simplify the codes, when flushing the flow, previous codes don't count the flow, because we have set them(flow counter/ufid counter)to 0. Now don't set counter and count flow and ufid flow when deleting them, and I add BUG_ON to make sure other patch no bug when flushing flow. > > -- Best regards, Tonghao
From: Tonghao Zhang <xiangxia.m.yue@gmail.com> Date: Thu, 20 Aug 2020 07:21:33 +0800 > On Thu, Aug 20, 2020 at 6:52 AM David Miller <davem@davemloft.net> wrote: > >> From: xiangxia.m.yue@gmail.com >> >> Date: Tue, 18 Aug 2020 18:09:22 +0800 >> >> >> >> > Decrease table->count and ufid_count unconditionally, >> >> >> >> You don't explain why this is a valid transformation. >> >> >> >> Is it a bug fix? > > No > >> >> >> >> Can it never happen? >> >> >> >> What are the details and how did you determine this? > > I want to simplify the codes, when flushing the flow, previous codes don't > count the flow, because we have set them(flow counter/ufid counter)to 0. > Now don't set counter and count flow and ufid flow when deleting them, and > I add BUG_ON to make sure other patch no bug when flushing flow. Add these details to your commit message, please.
On Thu, Aug 20, 2020 at 9:52 AM David Miller <davem@davemloft.net> wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > Date: Thu, 20 Aug 2020 07:21:33 +0800 > > > On Thu, Aug 20, 2020 at 6:52 AM David Miller <davem@davemloft.net> wrote: > > > >> From: xiangxia.m.yue@gmail.com > >> > >> Date: Tue, 18 Aug 2020 18:09:22 +0800 > >> > >> > >> > >> > Decrease table->count and ufid_count unconditionally, > >> > >> > >> > >> You don't explain why this is a valid transformation. > >> > >> > >> > >> Is it a bug fix? > > > > No > > > >> > >> > >> > >> Can it never happen? > >> > >> > >> > >> What are the details and how did you determine this? > > > > I want to simplify the codes, when flushing the flow, previous codes don't > > count the flow, because we have set them(flow counter/ufid counter)to 0. > > Now don't set counter and count flow and ufid flow when deleting them, and > > I add BUG_ON to make sure other patch no bug when flushing flow. > > Add these details to your commit message, please. Hi David v2 are sent, please review http://patchwork.ozlabs.org/project/netdev/patch/20200824073602.70812-2-xiangxia.m.yue@gmail.com/ http://patchwork.ozlabs.org/project/netdev/patch/20200824073602.70812-3-xiangxia.m.yue@gmail.com/ http://patchwork.ozlabs.org/project/netdev/patch/20200824073602.70812-4-xiangxia.m.yue@gmail.com/
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)