[ovs-dev,ovn,1/7] ovn-northd: Support optionally avoid static neighbor flows in routers.

Support option:dynamic_neigh_routers for logical routers, so that in
particular use cases static neighbor flows are not prepopulated IP
addresses belonging to neighbor router ports, to avoid flow exploding
problem reported for ovn-kubernetes large scale setup.

Reported-by: Girish Moodalbail <gmoodalbail@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 northd/ovn-northd.8.xml |  5 ++++-
 northd/ovn-northd.c     |  6 ++++++
 ovn-nb.xml              | 13 +++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 84 characters long (recommended limit is 79)
#61 FILE: ovn-nb.xml:1849:
      <column name="options" key="dynamic_neigh_routers" type='{"type": "boolean"}'>

Lines checked: 79, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

On Thu, Jul 23, 2020 at 10:57 AM Han Zhou <hzhou@ovn.org> wrote:

> Support option:dynamic_neigh_routers for logical routers, so that in
> particular use cases static neighbor flows are not prepopulated IP
> addresses belonging to neighbor router ports, to avoid flow exploding
> problem reported for ovn-kubernetes large scale setup.
>
> Reported-by: Girish Moodalbail <gmoodalbail@gmail.com>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>

Hi Han,

The patch LGTM. Can you please add few tests in ovn-northd.at and make sure
that the
lflows are not programmed by ovn-northd when  options:dynamic_neigh_routers
is set to true.

Thanks
Numan

 northd/ovn-northd.8.xml |  5 ++++-
>  northd/ovn-northd.c     |  6 ++++++
>  ovn-nb.xml              | 13 +++++++++++++
>  3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index eb2514f..9b1bcec 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2701,7 +2701,10 @@ outport = <var>P</var>;
>            <code>Logical_Switch_Port</code> table.  For router ports
>            connected to other logical routers, MAC bindings can be known
>            statically from the <code>mac</code> and <code>networks</code>
> -          column in the <code>Logical_Router_Port</code> table.
> +          column in the <code>Logical_Router_Port</code> table.  (Note:
> the
> +          flow is NOT installed for the IP addresses that belong to a
> neighbor
> +          logical router port if the current router has the
> +          <code>options:dynamic_neigh_routers</code> set to
> <code>true</code>)
>          </p>
>
>          <p>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 1921982..4e11a25 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -10202,6 +10202,12 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>                  continue;
>              }
>
> +            if (peer->od->nbr &&
> +                smap_get_bool(&peer->od->nbr->options,
> +                              "dynamic_neigh_routers", false)) {
> +                continue;
> +            }
> +
>              for (size_t i = 0; i < op->od->n_router_ports; i++) {
>                  const char *router_port_name = smap_get(
>
>  &op->od->router_ports[i]->nbsp->options,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index db5908c..e8450aa 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1846,6 +1846,19 @@
>            connected to the logical router. Default: False.
>          </p>
>        </column>
> +      <column name="options" key="dynamic_neigh_routers" type='{"type":
> "boolean"}'>
> +        <p>
> +          If set to <code>true</code>, the router will resolve neighbor
> +          routers' MAC addresses only by dynamic ARP/ND, instead of
> +          prepopulating static mappings for all neighbor routers in the
> ARP/ND
> +          Resolution stage.  This reduces number of flows, but requires
> ARP/ND
> +          messages to resolve the IP-MAC bindings when needed.  It is
> +          <code>false</code> by default.  It is recommended to set to
> +          <code>true</code> when a large number of logical routers are
> +          connected to the same logical switch but most of them never
> need to
> +          send traffic between each other.
> +        </p>
> +      </column>
>      </group>
>
>      <group title="Common Columns">
> --
> 2.1.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>