[ovs-dev] [PATCH 2/3] ovn-controller.c: Move the position of handling OVN-SB related settings.

Numan Siddique numans at ovn.org
Fri Jan 17 13:14:20 UTC 2020


On Wed, Jan 15, 2020 at 5:34 PM Flavio Fernandes <flavio at flaviof.com> wrote:
>
>
>
> > On Jan 14, 2020, at 5:34 PM, Han Zhou <hzhou at ovn.org> wrote:
> >
> > On Tue, Jan 14, 2020 at 1:24 PM Mark Michelson <mmichels at redhat.com> wrote:
> >>
> >> The commit message doesn't make much sense to me. The external-ids are
> >> set outside of ovn-controller, so the concept of them being handled in
> >> "the same iteration" or "the next one" only works if ovn-controller is
> >> setting them at some point in the loop.
> >>
> > Maybe the commit message is not clear enough. Let me explain with more
> > details.
> > In each iteration, the OVS IDL's data is updated AFTER the line:
> >    struct ovsdb_idl_txn *ovs_idl_txn = ovsdb_idl_loop_run(&ovs_idl_loop);
> >
> > Without this patch, it (the lines that are moved) applies the settings
> > before reading the new settings. So if a change is made to external-ids,
> > e.g. ovn-remote-db, the loop will be waked up because of the OVSDB RPC
> > messages, but it won't apply any of the new settings. The new settings will
> > be applied only if there is another event coming to wake the loop, e.g.
> > probe interval timeout. In my testing I saw the change took effect after 5
> > seconds when probe interval timed out. If probe was disabled, it would
> > never got applied unless a new change is made. I suspect the problem
> > reported here [0] may due to the same reason.
> >
> > [0]
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2020-January/049695.html
> >
> >> Couldn't this have a negative effect on the first iteration of the loop?
> >> If we don't set SSL parameters or the sb remote first, then we will have
> >> errors when attempting to connect to the southbound database.
> >>
> >
> > At the first iteration, it just make sure the OVS IDL data is refreshed
> > before setting the SSL parameters. We are still setting the parameters. The
> > patch doesn't skip anything.
> >
> >> On 1/13/20 5:52 PM, Han Zhou wrote:
> >>> Move the logic of handling OVN-SB related setting in external-ids
> >>> after the ovs_idl_loop run, so that any change in the external-ids
> >>> settings can take effect in the same iteration, without waiting for
> >>> the next one.
> >>>
> >>> Signed-off-by: Han Zhou <hzhou at ovn.org>

Acked-by: Numan Siddique <numans at ovn.org>

Numan

> >>> ---
> >>>  controller/ovn-controller.c | 8 ++++----
> >>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> >>> index 3d843f7..abb1b4c 100644
> >>> --- a/controller/ovn-controller.c
> >>> +++ b/controller/ovn-controller.c
> >>> @@ -2012,10 +2012,6 @@ main(int argc, char *argv[])
> >>>      while (!exiting) {
> >>>          engine_init_run();
> >>>
> >>> -        update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl);
> >>> -        update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
> >>> -
> > ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl));
> >>> -
> >>>          struct ovsdb_idl_txn *ovs_idl_txn =
> > ovsdb_idl_loop_run(&ovs_idl_loop);
> >>>          unsigned int new_ovs_cond_seqno
> >>>              = ovsdb_idl_get_condition_seqno(ovs_idl_loop.idl);
> >>> @@ -2027,6 +2023,10 @@ main(int argc, char *argv[])
> >>>              ovs_cond_seqno = new_ovs_cond_seqno;
> >>>          }
> >>>
> >>> +        update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl);
> >>> +        update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
> >>> +
> > ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl));
> >>> +
> >>>          struct ovsdb_idl_txn *ovnsb_idl_txn
> >>>              = ovsdb_idl_loop_run(&ovnsb_idl_loop);
> >>>          unsigned int new_ovnsb_cond_seqno
> >>>
> >>
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
> This change addresses the issue of setting the tunnel as Lars discovered.
> I needs to be back merged to 2.12 as well.
>
>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-January/049695.html <https://mail.openvswitch.org/pipermail/ovs-discuss/2020-January/049695.html>
> Tested-by: Flavio Fernandes <flavio at flaviof.com>
>
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list