On 11/19/20 7:00 PM, Ben Pfaff wrote: > On Thu, Nov 19, 2020 at 09:59:04AM -0800, Ben Pfaff wrote: >> On Thu, Nov 19, 2020 at 11:50:57AM +0100, Dumitru Ceara wrote: >>> On 11/19/20 5:15 AM, Ben Pfaff wrote: >>>> On Tue, Nov 17, 2020 at 05:50:20PM +0100, Dumitru Ceara wrote: >>>>> It is not correct for ovn-controller to pass the current SB_Global.nb_cfg >>>>> value to ofctrl_put() if there are pending changes to conditional >>>>> monitoring clauses (local or in flight). It might be that after the >>>>> monitor condition is acked by the SB, records that were added to the SB >>>>> before SB_Global.nb_cfg was set are now sent as updates to >>>>> ovn-controller. These should be first installed in OVS before >>>>> ovn-controller reports that it caught up with the current >>>>> SB_Global.nb_cfg value. >>>>> >>>>> Also, ofctrl_put should not advance cur_cfg if there are flow updates in >>>>> flight. >> >> ... >> >>>> ovn-controller has changed (advanced?) to the point that I have trouble >>>> understanding the code now. I'm going to assume that you understand it >>>> pretty well, but please allow me to ask a question here. Will this >>> >>> I'm doing my best but there are so many dependencies and side effects in >>> the code.. >> >> DDlog might actually help with that, eventually. It should allow us to >> program in the problem domain, rather than writing a lot of code to >> manage dependencies. >>
I tend to agree to this. Although it still seems like a long way until ovn-controller would use DDlog. >>>> always manage to come up to date with some version of the sb, or is >>>> there a chance that it never will report a consistent value if the sb >>>> keeps changing quickly? I wasn't able to figure that out with a quick >>>> look. >>>> >>> >>> Now, back to your question: >>> >>> TLDR: If ovn-controller has to continuously change its monitor >>> conditions I *think* there is a chance that it will never report that is >>> caught up with a specific SB seqno. However, I tried various scenarios >>> (with quick SB updates or claiming/releasing OVS interfaces in quick >>> succession) and I didn't manage to make "ovn-nbctl --wait=hv sync" block >>> indefinitely. >> >> That's good, empirically. >> >>> Moreover, I think the question is if it's correct for ovn-controller to >>> report it has caught up if there are unseen SB flows that correspond to >>> its currently requested but not acked monitor condition? >> >> I think the answer is "no", which means that the next question is, how >> do we implement it properly? I don't have an immediate answer. The >> database interfacing code is rather complex these days. So that means that even if in the theoretical situation I described above ovn-controller never reports that it caught up until all conditions are acked that's not a real problem. Which will be the new behavior if this patch is accepted. > > Oh, let me give this patch a conditional "ack". If you are pretty > confident that it makes things better, then it has my blessing: > Acked-by: Ben Pfaff <[email protected]> > Thanks! I'll send a new version next week. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
