| Message ID | 20240423124426.307018-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd, ic: Fix handling of ovn-appctl resume. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Tue, Apr 23, 2024 at 2:44 PM Xavier Simonart <xsimonar@redhat.com> wrote: > After ovn-appctl resume was issued for northd or ovn-ic, there was no > guarantee that northd or ovn-ic were waking up, potentially handling > changes received while they were paused.. > Usually, poll_block would be woken up by POLLHUP, but race conditions could > cause this not to happen. > ovn-controller is already properly handling the resume. > > This caused the following tests to fail sporadically: > - ovn-ic -- sync ISB status to INB > - propagate Port_Binding.up to NB and OVS. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > --- > ic/ovn-ic.c | 2 +- > northd/ovn-northd.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index e947323bf..be23f199d 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -2409,7 +2409,7 @@ ovn_ic_resume(struct unixctl_conn *conn, int argc > OVS_UNUSED, > { > struct ic_state *state = state_; > state->paused = false; > - > + poll_immediate_wake(); > unixctl_command_reply(conn, NULL); > } > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 3a5544b0c..d71114f35 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -1107,6 +1107,7 @@ ovn_northd_resume(struct unixctl_conn *conn, int > argc OVS_UNUSED, > { > struct northd_state *state = state_; > state->paused = false; > + poll_immediate_wake(); > > unixctl_command_reply(conn, NULL); > } > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On Fri, May 3, 2024 at 2:07 AM Ales Musil <amusil@redhat.com> wrote: > > On Tue, Apr 23, 2024 at 2:44 PM Xavier Simonart <xsimonar@redhat.com> wrote: > > > After ovn-appctl resume was issued for northd or ovn-ic, there was no > > guarantee that northd or ovn-ic were waking up, potentially handling > > changes received while they were paused.. > > Usually, poll_block would be woken up by POLLHUP, but race conditions could > > cause this not to happen. > > ovn-controller is already properly handling the resume. > > > > This caused the following tests to fail sporadically: > > - ovn-ic -- sync ISB status to INB > > - propagate Port_Binding.up to NB and OVS. > > > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > --- > > ic/ovn-ic.c | 2 +- > > northd/ovn-northd.c | 1 + > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index e947323bf..be23f199d 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -2409,7 +2409,7 @@ ovn_ic_resume(struct unixctl_conn *conn, int argc > > OVS_UNUSED, > > { > > struct ic_state *state = state_; > > state->paused = false; > > - > > + poll_immediate_wake(); > > unixctl_command_reply(conn, NULL); > > } > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 3a5544b0c..d71114f35 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -1107,6 +1107,7 @@ ovn_northd_resume(struct unixctl_conn *conn, int > > argc OVS_UNUSED, > > { > > struct northd_state *state = state_; > > state->paused = false; > > + poll_immediate_wake(); > > > > unixctl_command_reply(conn, NULL); > > } > > -- > > 2.31.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> Thanks. Applied to main and backported until branch-23.03. Numan > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com > <https://red.ht/sig> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c index e947323bf..be23f199d 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -2409,7 +2409,7 @@ ovn_ic_resume(struct unixctl_conn *conn, int argc OVS_UNUSED, { struct ic_state *state = state_; state->paused = false; - + poll_immediate_wake(); unixctl_command_reply(conn, NULL); } diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 3a5544b0c..d71114f35 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1107,6 +1107,7 @@ ovn_northd_resume(struct unixctl_conn *conn, int argc OVS_UNUSED, { struct northd_state *state = state_; state->paused = false; + poll_immediate_wake(); unixctl_command_reply(conn, NULL); }
After ovn-appctl resume was issued for northd or ovn-ic, there was no guarantee that northd or ovn-ic were waking up, potentially handling changes received while they were paused.. Usually, poll_block would be woken up by POLLHUP, but race conditions could cause this not to happen. ovn-controller is already properly handling the resume. This caused the following tests to fail sporadically: - ovn-ic -- sync ISB status to INB - propagate Port_Binding.up to NB and OVS. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- ic/ovn-ic.c | 2 +- northd/ovn-northd.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)