17:20:33 <imaximets> #startmeeting ovn_community_development_discussion
17:20:33 <openstack> Meeting started Thu Aug 13 17:20:33 2020 UTC and is due to finish in 60 minutes.  The chair is imaximets. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:20:34 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:20:36 <openstack> The meeting name has been set to 'ovn_community_development_discussion'
17:21:02 <imaximets> AFAIK, mmichelson and dceara will not be here today.
17:21:16 <imaximets> numans, are you here?
17:22:51 <imaximets> OK.  I could start with a quick update.
17:23:37 <imaximets> This week I looked at issues with DB sizes.  Mostly SB DB size.
17:24:12 <imaximets> It seems like we creating lots of identical lflows for each logical datapath.
17:24:58 <imaximets> And that might be optimized by only having one lflow referencing all logical datapaths it should be applicable to.
17:25:35 <numans> imaximets, zhouhan_ Hi
17:25:41 <imaximets> I don't know how exactly and under which conditions this could be done, but I'm looking into that.
17:25:50 <numans> I'm late
17:26:07 <imaximets> That's it from my side.
17:26:21 <numans> imaximets, Thanks for looking into that.
17:26:34 <imaximets> zhouhan seems to have connection issues.
17:26:47 <numans> imaximets, yeah there are many flows which are repetitive
17:27:07 <numans> I can go real quick.
17:27:18 <imaximets> numans, It all yours. :)
17:27:28 <numans> I did some reviews.
17:27:29 <zhouhan> imaximets: sorry I was in readonly mode. I was asking if there is any example?
17:28:05 <imaximets> zhouhan, for example we had a lot of reject ACL flows.
17:28:18 * numans will contnue after this discussion.
17:28:46 <zhouhan> imaximets: with the optimization, do we still need datapath information in the flow?
17:29:38 <numans> zhouhan, what do you mean by "we still need datapath information ..."
17:29:45 <imaximets> zhouhan, I thought to have a new table, e.g. Datapath_Group with sets of logical datapaths and have a single reference to a set from the logical flow.
17:30:45 <zhouhan> numans: I mean, if the flows are common for all datapaths, then we can replace them with just one flow, removing the datapath match.
17:31:05 <numans> zhouhan, ok. That makes sense too.
17:31:45 <imaximets> zhouhan, I see, but we have switches and routers and flow might be only applicable for switches, but not routers.
17:32:03 <imaximets> at least.
17:32:21 <numans> imaximets, may be a new column option which says its for logical switches or for routers.
17:32:36 <zhouhan> imaximets: if that's the case, would it be better to add a datapath type, instead of creating groups?
17:33:41 <imaximets> zhouhan, that make sense.  Good point.  Need to explore usecases deeply to understand if it's possible/feasible to have smaller groups.
17:34:37 <imaximets> zhouhan, one more case is lfows for port group. e.g. port group specific ACLs.
17:35:06 <zhouhan> I think it is good to optimize such cases if it is low hanging fruit, but I would avoid heavy changes for that, because I think the number of datapaths is much smaller than the number of ports. The size of the flow table mainly determined by number of ports.
17:36:37 <zhouhan> imaximets: oh, I didn't notice that. If it is for all port groups, it may be straightforward to optimize, too.
17:37:04 <numans> zhouhan, in the case of ovn-k8s, where its switch per node, there could be significat flows if say number of computes is 100
17:37:25 <numans> ok.
17:38:01 <zhouhan> numans: even though, it is normal to have 10x more ports than number of computes, right?
17:38:42 <zhouhan> sometimes, even 100x
17:38:58 <numans> yeah.
17:39:49 <imaximets> zhouhan, ok.  we definitely still need to explore some usecases and see if it will have real benefits in real-world cases.  Work in progress. :)
17:40:11 <zhouhan> imaximets: sure, thanks!
17:40:35 <zhouhan> numans: please continue. I will update after you.
17:41:24 <numans> zhouhan, thanks.
17:42:13 <numans> So I did some reviews.
17:42:21 <numans> and a couple of small bug fix patches.
17:42:49 <numans> There is one issue reported by openshift on openstack scenario
17:43:13 <numans> the etcd cluster is having a downtime and a new leader is elected when some tests are run
17:43:34 <numans> and the leader change happens when ovn-controller program flows and it updates the conjunction ids of existing flows
17:43:46 <numans> it is for ACLs which results in conjunction
17:44:06 <numans> So I'm working on making conjunction ids persistent
17:44:28 <numans> so that when a port is added to a  port group or during a recompute, we use the same conjunction id.
17:44:46 <numans> It is not a big issue. But there is a very very small window for packet drops.
17:45:02 <zhouhan> numans: sorry, how does etcd cluster impact ovn-controller?
17:45:19 <numans> zhouhan, etcd cluster is running as application pods
17:46:01 <numans> so the etcd traffic gets disrupted when ovn-controller changes the conjunction id for ACL flows which allow traffic for these etcd ports
17:46:44 <numans> zhouhan, the CI test creates other pods and other ACLs and while processing those, ovn-controller is updating the existing flows
17:46:44 <zhouhan> I see, does it happen only during flow recompute
17:46:46 <zhouhan> ?
17:47:07 <numans> zhouhan, it also happens when a port is added to the port group
17:47:19 <zhouhan> ok
17:47:43 <numans> zhouhan, with the new I-P patches, the issue is not seen often
17:47:44 <zhouhan> I wonder is it a generic problem even without conjunction flows.
17:47:54 <numans> as we don't recompute enough now
17:48:08 <numans> but the issue is still seen when the CI is run with parallel=2 itseems.
17:48:24 <numans> I'm not sure what parallel=2 exactly mean, I assume more tests are run in parallle
17:48:28 <numans> parallel.
17:49:09 <zhouhan> i.e. problem when there are updates in OVS flows, groups, meters, is it possible to see transient traffic broke?
17:49:33 <numans> zhouhan, that's what I observed.
17:49:47 <zhouhan> or, is conjunction ID recompute is the only thing we worry about?
17:50:09 <numans> zhouhan, the issue is seen when the ACL is added like this -- "ip && .. inport == @pg1 && tcp.dst >=900 && tcp.dst <=901"
17:50:33 <numans> zhouhan, and the issue is not seen when 2 separate ACLs are added for these 2 tcp dst ports
17:50:51 <numans> so I think its happening when conjunction is involved
17:51:07 <zhouhan> numans: I understand that the problem you saw is related to conjunction. I was just thinking is there similar issue even without using conjunction.
17:51:24 <numans> zhouhan, I don't  think so.
17:51:37 <zhouhan> numans: that's great.
17:51:39 <numans> zhouhan, in the case of conjunction, we do FLOW_MOD.
17:51:49 <numans> zhouhan, in other cases, we don't do FLOW_MOD right ?
17:52:26 <numans> I think either the OF flow will be deleted and added again
17:52:30 <zhouhan> numans: I don't remember. Maybe we can check offline. I don't remember either is there any chance ct-zone-id, etc. could have similar issue.
17:52:40 <numans> or nothing happens.
17:52:41 <numans> zhouhan, ok
17:52:43 <numans> sounds good
17:52:57 <numans> may be we can discuss further when I submit the patch
17:53:12 <zhouhan> numans: yeah, sounds good
17:53:25 <numans> one point though - I'm planning to revisit the lflow expr patches which we had revertd earlier.
17:53:30 <numans> to solve this issue.
17:53:36 <numans> That's it from me.
17:53:52 <zhouhan> I can go quickly
17:54:44 <imaximets> zhouhan, sure.
17:54:53 <zhouhan> I found the root cause of the scale-test regression in 20.03 compared with 2.12. It has nothing to do with 20.03 OVN, but related to the upstream OVS.
17:55:27 <numans> great finding. I didn't see the patch closely though.
17:55:50 <zhouhan> It is a change in ovsdb IDL code that caused the problem. I reverted the patch and the performance is comparable with 2.12 now. The revert is merged by imaximets. Thanks imaximets for the review.
17:56:11 <imaximets> zhouhan, thanks for finding this!
17:56:22 <numans> zhouhan, its on the client IDL side right ?
17:56:39 <zhouhan> numans: yes
17:56:43 <numans> zhouhan, ok. cool.
17:57:44 <zhouhan> Now with this solved, I can compare 20.03 v.s. 20.06 more fairly, because northd doesn't appear to be the main bottleneck now.
17:58:24 <zhouhan> There is obvious latency reduce in 20.06, thanks to numans's I-P improvement for handling changes in local chassis.
17:59:26 <zhouhan> Now there are still bottlenecks in ovn-controller ofctrl_put() when number of ports is big enough. I am working on incrementally installing flows.
17:59:44 <numans> zhouhan, that's cool.
18:00:34 <numans> zhouhan, anilvenkata had a WIP patch to improve the ofctl_put. But he switched his focus else where. I'll just share the commit in his private branch.
18:00:49 <numans> please take a look in case if that interests you.
18:01:00 <zhouhan> numans: cool. Thanks a lot!
18:01:10 <zhouhan> While working on this, I also found a bug related to conjunction when I-P is involved. It was introduced by the patch that handling merging conjunction flows from different logical flows.
18:01:35 <numans> ok.
18:01:38 <zhouhan> I fixed this first, and working on the tests, hope to send a patch soon.
18:01:50 <numans> that's great. Looking forward to it.
18:02:24 <zhouhan> that's my update
18:02:41 <imaximets> zhouhan, Thanks!
18:02:50 <imaximets> Anyone else here?
18:04:38 <numans> zhouhan, I just need another minute to share the commit
18:05:03 <zhouhan> numans: no worries. We can share offline.
18:05:15 <numans> imaximets, I think we are done.
18:05:20 <numans> zhouhan, sounds good.
18:05:30 <imaximets> OK.  Let's call it. :)
18:05:32 <zhouhan> Let's end the meeting and discuss secrets to punish the people who don't join :)
18:05:35 <imaximets> Thanks everyone!
18:05:45 <imaximets> zhouhan, :)
18:05:48 <imaximets> #endmeeting