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

Numan Siddique numans at ovn.org
Mon Oct 4 22:34:36 UTC 2021


On Mon, Oct 4, 2021 at 4:56 PM Mark Michelson <mmichels at redhat.com> wrote:
>
> On 10/4/21 4:10 PM, Han Zhou wrote:
> >
> >
> >
> > On Mon, Oct 4, 2021 at 11:30 AM Numan Siddique <numans at ovn.org
> > <mailto:numans at ovn.org>> wrote:
> >  >
> >  > On Mon, Oct 4, 2021 at 1:37 PM Han Zhou <hzhou at ovn.org
> > <mailto:hzhou at ovn.org>> wrote:
> >  > >
> >  > > On Mon, Oct 4, 2021 at 3:10 AM Mark Gray <mark.d.gray at redhat.com
> > <mailto: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 <mailto: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
> > <mailto: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.)
> >  > >
> >  >
> >  > I think I submitted the patch because, when a logical port P1 sends
> >  > arp for an IP of lport P2,
> >  > then it would get the arp response even if P2 is down  which at that
> >  > time I thought is not
> >  > reflecting the real case scenario (with the physical deployments).
> >  >
> >  > But I'm fine with this patch.
> >  >
> >  > Acked-by: Numan Siddique <numans at ovn.org <mailto:numans at ovn.org>>
> >
> > Thanks Numan and Mark. I noticed that I put the news update in the wrong
> > section. So I just did the minor change and applied the patch.
> >
> > However, I also noticed that in the NEWS file the release date of 21.09
> > is not updated:
> > OVN v21.09.0 - xx xxx xxxx
> >
> > I didn't correct that because it is better to be a separate patch. cc
> > @Mark Michelson <mailto:mmichels at redhat.com>
>
> One of these days I'll actually do this correctly :)
>
> The release date is correct in branch-21.09, but I did not update the
> release date in the main branch. I'll submit a patch that corrects this.
> Thanks for bringing it to my attention.

I think you just need to apply the patch 1 (this one -
https://github.com/ovn-org/ovn/commit/332946db2f579f4999ad9708cdc94ebcd78ff296)
to the main branch too.

Numan

>
> >
> >  >
> >  > Thanks
> >  > Numan
> >  >
> >  >
> >  > > Han
> >  > > >
> >  > > > > ---
> >  > > > >  NEWS                 | 4 ++++
> >  > > > >  northd/northd.c      | 2 +-
> >  > > > >  northd/ovn_northd.dl | 2 +-
> >  > > > >  ovn-nb.xml           | 2 +-
> >  > > > >  tests/ovn-northd.at <http://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 <http://ovn-northd.at>
> > b/tests/ovn-northd.at <http://ovn-northd.at>
> >  > > > > index c5400d806..3eebb55b6 100644
> >  > > > > --- a/tests/ovn-northd.at <http://ovn-northd.at>
> >  > > > > +++ b/tests/ovn-northd.at <http://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
> >  > > > >
> >  > > >
> >  > > _______________________________________________
> >  > > dev mailing list
> >  > > dev at openvswitch.org <mailto:dev at openvswitch.org>
> >  > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
> >  > >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list