[ovs-dev] [PATCH ovn] northd: Change the default value of ignore_lsp_down to true.

Han Zhou hzhou at ovn.org
Mon Oct 4 17:36:27 UTC 2021


On Mon, Oct 4, 2021 at 3:10 AM Mark Gray <mark.d.gray at redhat.com> wrote:
>
> On 02/10/2021 08:11, Han Zhou wrote:
> > The current default behavior is that ARP responder flows for VIFs are
> > added by northd after the port-binding state is UP, which creates more
> > trouble than benefit in most use cases. To make the default behavior
> > desirable for majority of the use cases, set the option ignore_lsp_down
> > to true by default. This would help saving the control plane cost in
> > large scale environment, reduce the e2e latency for all flows to be
> > installed for a VIF, and making the VIF readiness checking more
convenient
> > in test cases and likely in CMS as well. User can still set it to false
> > in circumstances (if any) when this behavior is not desired.
> >
> > Signed-off-by: Han Zhou <hzhou at ovn.org>
>
> Thanks Han. This seems to be good to me but I think someone else who is
> more familiar with some of the original problems should give it a look
> over in case I am missing something.
>
> Acked-by: Mark D. Gray <mark.d.gray at redhat.com>

Thanks Mark. I couldn't find the source of the original problem that
required checking LSP state before adding the ARP responder flows. But let
me add Numan who was the author for this lsp status check back in 2015, and
could have better judgement regarding this default behavior change.
(b891c4c6a ovn-northd: Only add ARP reply flows for logical ports that are
up.)

Thanks,
Han
>
> > ---
> >  NEWS                 | 4 ++++
> >  northd/northd.c      | 2 +-
> >  northd/ovn_northd.dl | 2 +-
> >  ovn-nb.xml           | 2 +-
> >  tests/ovn-northd.at  | 3 ++-
> >  5 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 8a21c029e..348f3f548 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -12,6 +12,10 @@ OVN v21.09.0 - xx xxx xxxx
> >    - Allow static routes without nexthops.
> >    - Enabled logical dp groups as a default.  CMS should disable it if
not
> >      desired.
> > +  - Set ignore_lsp_down to true as default, so that ARP responder
flows are
> > +    installed together with other flows when a logical switch port is
created,
> > +    without having to wait for the port to be UP.  CMS should set it
to false
> > +    if not desired.
> >
> >  OVN v21.06.0 - 18 Jun 2021
> >  -------------------------
> > diff --git a/northd/northd.c b/northd/northd.c
> > index cf2467fe1..5ffd6b8eb 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -14304,7 +14304,7 @@ ovnnb_db_run(struct northd_context *ctx,
> >      controller_event_en = smap_get_bool(&nb->options,
> >                                          "controller_event", false);
> >      check_lsp_is_up = !smap_get_bool(&nb->options,
> > -                                     "ignore_lsp_down", false);
> > +                                     "ignore_lsp_down", true);
> >
> >      build_datapaths(ctx, datapaths, lr_list);
> >      build_ovn_lbs(ctx, datapaths, &lbs);
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index 22913f05a..7154fed13 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -753,7 +753,7 @@ Northd_Probe_Interval[interval] :-
> >  relation CheckLspIsUp[bool]
> >  CheckLspIsUp[check_lsp_is_up] :-
> >      nb in nb::NB_Global(),
> > -    var check_lsp_is_up = not
nb.options.get_bool_def(i"ignore_lsp_down", false).
> > +    var check_lsp_is_up = not
nb.options.get_bool_def(i"ignore_lsp_down", true).
> >  CheckLspIsUp[true] :-
> >      Unit(),
> >      not nb in nb::NB_Global().
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 390cc5a44..d8266ed4d 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -236,7 +236,7 @@
> >            resolved even before the relevant VM/container is running.
For
> >            environments where this is not an issue, setting it to
> >            <code>true</code> can reduce the load and latency of the
control
> > -          plane. The default value is <code>false</code>.
> > +          plane. The default value is <code>true</code>.
> >          </p>
> >        </column>
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index c5400d806..3eebb55b6 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -1918,6 +1918,7 @@ OVN_FOR_EACH_NORTHD([
> >  AT_SETUP([ignore_lsp_down])
> >  ovn_start
> >
> > +ovn-nbctl set NB_Global . options:ignore_lsp_down=false
> >  ovn-nbctl ls-add sw0
> >  ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1
"aa:aa:aa:aa:aa:aa 10.0.0.1"
> >
> > @@ -4922,7 +4923,7 @@ check ovn-nbctl lsp-add sw0 lsp0 \
> >
> >  check ovn-nbctl --wait=sb sync
> >  AT_CHECK([ovn-sbctl --columns=tags list logical_flow | grep lsp0 -c],
[0], [dnl
> > -9
> > +10
> >  ])
> >
> >  AT_CLEANUP
> >
>


More information about the dev mailing list