On 12/4/20 9:13 AM, Han Zhou wrote: > > > On Thu, Dec 3, 2020 at 6:49 AM Dumitru Ceara <dce...@redhat.com > <mailto:dce...@redhat.com>> wrote: >> >> The IDL might decide to resend pending monitor condition changes >> implicitly (one such case is at reconnect) in which case the client >> (ovn-controller) might end waiting for a condition seqno that has >> already been satisfied. >> >> In order to avoid this case, we now relax the seqno check in >> get_nb_cfg() and if the IDL condition seqno is greater or equal than >> what ovn-controller expected after changing monitor conditions it means >> that there are no in-flight pending changes. >> >> Fixes: 58e5d32b011b ("ovn-controller: Fix nb_cfg update with > monitor_cond_change in flight.") >> Signed-off-by: Dumitru Ceara <dce...@redhat.com > <mailto:dce...@redhat.com>> >> --- >> CI run with tests passing from the first run (no recheck): >> https://github.com/dceara/ovn/actions/runs/398575371 >> --- >> controller/ovn-controller.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 46589e4..a9415b2 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -824,8 +824,15 @@ get_nb_cfg(const struct sbrec_sb_global_table > *sb_global_table, >> /* Delay getting nb_cfg if there are monitor condition changes >> * in flight. It might be that those changes would instruct the >> * server to send updates that happened before SB_Global.nb_cfg. >> + * >> + * The IDL can decide to resend pending conditions upon reconnect in >> + * which case the expected_cond_seqno is not updated because the > client >> + * (ovn-controller) did not explicitly request it. That means > that we >> + * cannot just check for cond_seqno != expected_cond_seqno and we > also >> + * have to take into account potential unsigned int overflows. >> */ >> - if (cond_seqno != expected_cond_seqno) { >> + if (cond_seqno < expected_cond_seqno && > > If we consider overflows, should the above condition use "!=" ? >
Hi Han, Thanks for the review. Actually, after some more debugging, I think the real issue was due to an IDL bug for which I've just sent a patch: https://patchwork.ozlabs.org/project/openvswitch/patch/1607093681-17955-1-git-send-email-dce...@redhat.com/ I think we can discard the OVN patch (at least for now). Thanks, Dumitru >> + (cond_seqno != 0 || expected_cond_seqno != UINT_MAX)) { >> return nb_cfg; >> } >> >> -- >> 1.8.3.1 >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev