[ovs-dev] [PATCH ovn] ovn-northd: Optionally skip the check of lsp_is_up.

Numan Siddique numans at ovn.org
Tue Sep 8 12:29:40 UTC 2020


On Thu, Sep 3, 2020 at 7:44 AM Han Zhou <hzhou at ovn.org> wrote:
>
> The checking of lsp "up" status before adding ARP responder flows was
> added to avoid confusion in cases when a network admin sees ARP response
> for VM/containers that is not up on chassis yet. However, this check
> introduces an extra round of flow change in SB and triggers computes
> on hypervisors which is unnecessary cost in most cases, especially for
> large scale environment. To improve this, this patch provides an option
> check_lsp_is_up to disable the check (when setting to "false") for the
> use cases when ARP reponse for a port that is "down" isn't considered
> harmful.
>
> Signed-off-by: Han Zhou <hzhou at ovn.org>


Few minor comments.

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

Thanks
Numan

> ---
>  northd/ovn-northd.8.xml | 15 ++++++++++-----
>  northd/ovn-northd.c     |  7 ++++++-
>  ovn-nb.xml              | 18 ++++++++++++++++++
>  tests/ovn-northd.at     | 14 ++++++++++++++
>  4 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 989e364..df43eb6 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -763,9 +763,11 @@ output;
>
>          <p>
>            These flows are omitted for logical ports (other than router ports or
> -          <code>localport</code> ports) that are down, for logical ports of
> -          type <code>virtual</code> and for logical ports with 'unknown'
> -          address set.
> +          <code>localport</code> ports) that are down (unless <code>
> +          check_lsp_is_up</code> is configured as false in <code>options</code>
> +          column of <code>NB_Global</code> table of the <code>Northbound</code>
> +          database), for logical ports of type <code>virtual</code> and for
> +          logical ports with 'unknown' address set.
>          </p>
>        </li>
>
> @@ -812,8 +814,11 @@ nd_na_router {
>
>          <p>
>            These flows are omitted for logical ports (other than router ports or
> -          <code>localport</code> ports) that are down and for logical ports of
> -          type <code>virtual</code>.
> +          <code>localport</code> ports) that are down (unless <code>
> +          check_lsp_is_up</code> is configured as false in <code>options</code>
> +          column of <code>NB_Global</code> table of the <code>Northbound</code>
> +          database), for logical ports of type <code>virtual</code> and for
> +          logical ports with 'unknown' address set.
>          </p>
>        </li>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 7be0e85..291713e 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -86,6 +86,8 @@ static struct eth_addr mac_prefix;
>
>  static bool controller_event_en;
>
> +static bool check_lsp_is_up;
> +
>  /* MAC allocated for service monitor usage. Just one mac is allocated
>   * for this purpose and ovn-controller's on each chassis will make use
>   * of this mac when sending out the packets to monitor the services
> @@ -6739,7 +6741,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
>               *  - port type is router or
>               *  - port type is localport
>               */
> -            if (!lsp_is_up(op->nbsp) && strcmp(op->nbsp->type, "router") &&
> +            if (check_lsp_is_up &&
> +                !lsp_is_up(op->nbsp) && strcmp(op->nbsp->type, "router") &&
>                  strcmp(op->nbsp->type, "localport")) {
>                  continue;
>              }
> @@ -11722,6 +11725,8 @@ 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,
> +                                    "check_lsp_is_up", true);
>
>      build_datapaths(ctx, datapaths, lr_list);
>      build_ports(ctx, sbrec_chassis_by_name, datapaths, ports);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 1f2dbb9..4e34518 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -147,6 +147,24 @@
>          </p>
>        </column>
>
> +      <column name="options" key="check_lsp_is_up">

How about "ignore_lport_down" ? The default would be false. If you're
fine with this, then
you need to invert the logic in the code.

I don't have a very strong opinion. If you want to stick with
check_lsp_is_up, I'm fine with it.


> +        <p>
> +          If set to true, ARP/ND reply flows for logical switch ports will be
> +          installed only if the port is up, i.e. claimed by a Chassis. If set
> +          to false, these flows are installed regardless of the status of the
> +          port, which can result in a situation that ARP request to an IP is
> +          resolved even before the relevant VM/container is running. For
> +          environments where this is not an issue, setting it to
> +          <code>false</code> can reduce the load and latency of the control
> +          plane. The default value is <code>true</code>.
> +        </p>
> +
> +        <p>
> +          If the value is nonzero, then it will be forced to a value of
> +          at least 1000 ms.
> +        </p>

Looks like some copy/paste issue ? Because the value is a boolean :).

Thanks
Numan



> +      </column>
> +
>        <group title="Options for configuring interconnection route advertisement">
>          <p>
>            These options control how routes are advertised between OVN
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 8344c7f..c98df49 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1781,3 +1781,17 @@ AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0
>  ])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- disable check_lsp_is_up])
> +ovn_start
> +
> +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"
> +
> +ovn-nbctl --wait=sb sync
> +AT_CHECK([ovn-sbctl lflow-list | grep arp | grep 10\.0\.0\.1], [1], [ignore])
> +
> +ovn-nbctl --wait=sb set NB_Global . options:check_lsp_is_up=false
> +AT_CHECK([ovn-sbctl lflow-list | grep arp | grep 10\.0\.0\.1], [0], [ignore])

> +
> +AT_CLEANUP
> --
> 2.1.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list