| Message ID | 20231023100423.3861347-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-ic: wakeup on ovsdb transaction failures | 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 | success | github build: passed |
Thanks Xavier, Acked-by: Mark Michelson <mmichels@redhat.com> On 10/23/23 06:04, Xavier Simonart wrote: > If an ovsdb transaction fails (e.g. adding the same tunnel_key as another > ovn-ic to the same datapath, for a different port (race condition)), > there was no guarentee that ovn-ic would wake up, and that state would stay > until ovn-ic wake up, for instance due to some ovn-ic-sb changes. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > --- > ic/ovn-ic.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index e2023c2ba..bb91bad11 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -2216,10 +2216,19 @@ main(int argc, char *argv[]) > ovn_db_run(&ctx); > } > > - ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > + int rc1 = ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > + int rc2 = ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > + int rc3 = ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > + int rc4 = ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > + if (!rc1 || !rc2 || !rc3 || !rc4) { > + VLOG_DBG(" a transaction failed in: %s %s %s %s", > + !rc1 ? "nb" : "", !rc2 ? "sb" : "", > + !rc3 ? "ic_nb" : "", rc4 ? "ic_sb" : ""); > + /* A transaction failed. Wake up immediately to give > + * opportunity to send the proper transaction > + */ > + poll_immediate_wake(); > + } > } else { > /* ovn-ic is paused > * - we still want to handle any db updates and update the
On Mon, Oct 23, 2023 at 12:05 PM Xavier Simonart <xsimonar@redhat.com> wrote: > > If an ovsdb transaction fails (e.g. adding the same tunnel_key as another > ovn-ic to the same datapath, for a different port (race condition)), > there was no guarentee that ovn-ic would wake up, and that state would stay > until ovn-ic wake up, for instance due to some ovn-ic-sb changes. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > --- > ic/ovn-ic.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index e2023c2ba..bb91bad11 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -2216,10 +2216,19 @@ main(int argc, char *argv[]) > ovn_db_run(&ctx); > } > > - ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > - ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > + int rc1 = ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > + int rc2 = ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > + int rc3 = ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > + int rc4 = ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > + if (!rc1 || !rc2 || !rc3 || !rc4) { > + VLOG_DBG(" a transaction failed in: %s %s %s %s", > + !rc1 ? "nb" : "", !rc2 ? "sb" : "", > + !rc3 ? "ic_nb" : "", rc4 ? "ic_sb" : ""); > + /* A transaction failed. Wake up immediately to give > + * opportunity to send the proper transaction > + */ > + poll_immediate_wake(); > + } > } else { > /* ovn-ic is paused > * - we still want to handle any db updates and update the > -- > 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, Nov 3, 2023 at 5:30 AM Ales Musil <amusil@redhat.com> wrote: > > On Mon, Oct 23, 2023 at 12:05 PM Xavier Simonart <xsimonar@redhat.com> wrote: > > > > If an ovsdb transaction fails (e.g. adding the same tunnel_key as another > > ovn-ic to the same datapath, for a different port (race condition)), > > there was no guarentee that ovn-ic would wake up, and that state would stay > > until ovn-ic wake up, for instance due to some ovn-ic-sb changes. > > > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > --- > > ic/ovn-ic.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index e2023c2ba..bb91bad11 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -2216,10 +2216,19 @@ main(int argc, char *argv[]) > > ovn_db_run(&ctx); > > } > > > > - ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > > - ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > > - ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > > - ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > > + int rc1 = ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > > + int rc2 = ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > > + int rc3 = ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); > > + int rc4 = ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); > > + if (!rc1 || !rc2 || !rc3 || !rc4) { > > + VLOG_DBG(" a transaction failed in: %s %s %s %s", > > + !rc1 ? "nb" : "", !rc2 ? "sb" : "", > > + !rc3 ? "ic_nb" : "", rc4 ? "ic_sb" : ""); > > + /* A transaction failed. Wake up immediately to give > > + * opportunity to send the proper transaction > > + */ > > + poll_immediate_wake(); > > + } > > } else { > > /* ovn-ic is paused > > * - we still want to handle any db updates and update the > > -- > > 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. Will backport to other branches once the CI passes. Numan > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA > > amusil@redhat.com > > _______________________________________________ > 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 e2023c2ba..bb91bad11 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -2216,10 +2216,19 @@ main(int argc, char *argv[]) ovn_db_run(&ctx); } - ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); - ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); - ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); - ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); + int rc1 = ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); + int rc2 = ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); + int rc3 = ovsdb_idl_loop_commit_and_wait(&ovninb_idl_loop); + int rc4 = ovsdb_idl_loop_commit_and_wait(&ovnisb_idl_loop); + if (!rc1 || !rc2 || !rc3 || !rc4) { + VLOG_DBG(" a transaction failed in: %s %s %s %s", + !rc1 ? "nb" : "", !rc2 ? "sb" : "", + !rc3 ? "ic_nb" : "", rc4 ? "ic_sb" : ""); + /* A transaction failed. Wake up immediately to give + * opportunity to send the proper transaction + */ + poll_immediate_wake(); + } } else { /* ovn-ic is paused * - we still want to handle any db updates and update the
If an ovsdb transaction fails (e.g. adding the same tunnel_key as another ovn-ic to the same datapath, for a different port (race condition)), there was no guarentee that ovn-ic would wake up, and that state would stay until ovn-ic wake up, for instance due to some ovn-ic-sb changes. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- ic/ovn-ic.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)