[ovs-dev] [PATCH ovn v5 3/5] northd: Save all router addresses in Port_Bindings

Numan Siddique numans at ovn.org
Wed Apr 21 14:50:29 UTC 2021


On Tue, Apr 20, 2021 at 11:57 AM Mark Michelson <mmichels at redhat.com> wrote:
>
> northd currently stores logical switch port's configured "nat-addresses"
> in the southbound Port_Binding "nat_addresses" column. This allows for
> explicit configuration of which NAT addresses should be broadcast in
> gARP messages by OVN.
>
> This adds a similar column, "router_addresses" to the Port_Binding. The
> difference is that this column contains all NAT, load balancer, and
> router interface addresses, no matter the northbound configuration.
>
> This column will be used in an upcoming commit.
>
> Signed-off-by: Mark Michelson <mmichels at redhat.com>

Hi Mark,

Please see below for few comments.


> ---
>  northd/ovn-northd.c  | 222 +++++++++++++++++++++++++++++--------------
>  northd/ovn_northd.dl |  35 +++++--
>  ovn-sb.ovsschema     |   5 +-
>  ovn-sb.xml           |  20 ++++
>  tests/ovn-northd.at  | 206 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 408 insertions(+), 80 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d8ee65a5f..78ac696a5 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2486,7 +2486,7 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
>  {
>      size_t n_nats = 0;
>      struct eth_addr mac;
> -    if (!op->nbrp || !op->od || !op->od->nbr
> +    if (!op || !op->nbrp || !op->od || !op->od->nbr
>          || (!op->od->nbr->n_nat && !op->od->nbr->n_load_balancer)
>          || !eth_addr_from_string(op->nbrp->mac, &mac)) {
>          *n = n_nats;
> @@ -2878,6 +2878,132 @@ ovn_update_ipv6_prefix(struct hmap *ports)
>      }
>  }
>
> +static void
> +ovn_port_set_garp_addresses(const struct ovn_port *op, const char **nats,
> +                            size_t n_nats, const char *router_networks)
> +{
> +    bool peer_is_gateway = op->peer
> +                           && op->peer->od
> +                           && op->peer->od->nbr
> +                           && smap_get(&op->peer->od->nbr->options,
> +                                       "chassis");
> +
> +    bool peer_has_dist_gw_port = op->peer
> +                                 && op->peer->od
> +                                 && op->peer->od->l3redirect_port;
> +

I think caller of this function can pass the 'peer_is_gateway' and
'peer_has_dist_gw_port' bools
since these values are stored  before calling this function.


> +    const char *nat_addresses = smap_get(&op->nbsp->options,
> +                                         "nat-addresses");
> +    size_t n_sources = 0;
> +    const char **source = NULL;
> +    if (nat_addresses && (peer_is_gateway || peer_has_dist_gw_port)) {
> +        if (!strcmp(nat_addresses, "router")) {
> +            /* We size this to n_nats + 1 since it can at most include
> +             * all NAT addresses plus the router's networks.
> +             */
> +            source = nats;
> +            n_sources = n_nats;
> +        /* Only accept manual specification of ethernet address
> +         * followed by IPv4 addresses on type "l3gateway" ports. */
> +        } else if (peer_is_gateway) {
> +            struct lport_addresses laddrs;
> +            if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
> +                static struct vlog_rate_limit rl =
> +                    VLOG_RATE_LIMIT_INIT(1, 1);
> +                VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
> +            } else {
> +                destroy_lport_addresses(&laddrs);
> +                /* Allocate enough space for the nat_addresses plus
> +                 * the router networks
> +                 */
> +                source = &nat_addresses;
> +                n_sources = 1;
> +            }
> +        }
> +    }
> +
> +    size_t n_garps = 0;
> +    /* The "garps" array contains the addresses for which
> +     * ovn-controller should send GARPs. This is controlled
> +     * by the "nat-addresses" option on the logical switch.
> +     *
> +     * Size the garps array to one more than the number of NAT
> +     * sources so that we can fit the NATs and the router
> +     * networks.
> +     */
> +    char **garps = xcalloc(n_sources + 1, sizeof *garps);
> +    for (n_garps = 0; n_garps < n_sources; n_garps++) {
> +        garps[n_garps] = xstrdup(source[n_garps]);
> +    }
> +
> +    /* Add the router mac and IPv4 addresses to
> +     * Port_Binding.nat_addresses so that GARP is sent for these
> +     * IPs by the ovn-controller on which the distributed gateway
> +     * router port resides if:
> +     *
> +     * -  op->peer has 'reside-on-redirect-chassis' set and the
> +     *    the logical router datapath has distributed router port.
> +     *
> +     * -  op->peer is distributed gateway router port.
> +     *
> +     * -  op->peer's router is a gateway router and op has a localnet
> +     *    port.
> +     *
> +     * Note: Port_Binding.nat_addresses column is also used for
> +     * sending the GARPs for the router port IPs.
> +     * */
> +    bool add_router_port_garp = false;
> +    if (peer_has_dist_gw_port &&
> +        (smap_get_bool(&op->peer->nbrp->options,
> +                      "reside-on-redirect-chassis", false) ||
> +        op->peer == op->peer->od->l3dgw_port)) {
> +        add_router_port_garp = true;
> +    } else if (peer_is_gateway && op->od->n_localnet_ports) {
> +        add_router_port_garp = true;
> +    }
> +
> +    if (add_router_port_garp) {
> +        /* "garps" was allocated with enough space to hold
> +         * this address without reallocating.
> +         */
> +        garps[n_garps] = xstrdup(router_networks);
> +        n_garps++;
> +    }
> +
> +    sbrec_port_binding_set_nat_addresses(op->sb,
> +                                         (const char **) garps,
> +                                         n_garps);
> +    for (size_t i = 0; i < n_garps; i++) {
> +        free(garps[i]);
> +    }
> +    free(garps);
> +}
> +
> +static void
> +ovn_port_set_router_addresses(const struct ovn_port *op, const char **nats,
> +                              size_t n_nats, const char *router_networks)
> +{
> +    /* The total number of router addresses is the NATs plus the
> +     * router networks
> +     */
> +    size_t n_router_addresses = n_nats + 1;
> +    const char **router_addresses = xcalloc(n_router_addresses,
> +                                      sizeof *router_addresses);
> +
> +    for (size_t i = 0; i < n_nats; i++) {
> +        router_addresses[i] = nats[i];
> +    }
> +    router_addresses[n_nats] = router_networks;
> +
> +    /* All NAT addresses + the router address need to be added
> +     * to the port binding's router_addresses field, even if they
> +     * have not been configured on the switch port.
> +     */
> +    sbrec_port_binding_set_router_addresses(
> +        op->sb, (const char **) router_addresses, n_router_addresses);
> +    free(router_addresses);
> +}
> +
>  static void
>  ovn_port_update_sbrec(struct northd_context *ctx,
>                        struct ovsdb_idl_index *sbrec_chassis_by_name,
> @@ -3050,9 +3176,14 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>                  chassis = smap_get(&op->peer->od->nbr->options, "chassis");
>              }
>
> +            bool peer_is_gateway = !!chassis;
> +            bool peer_has_dist_gw_port = op->peer
> +                                         && op->peer->od
> +                                         && op->peer->od->l3redirect_port;
> +
>              /* A switch port connected to a gateway router is also of
>               * type "l3gateway". */
> -            if (chassis) {
> +            if (peer_is_gateway) {
>                  sbrec_port_binding_set_type(op->sb, "l3gateway");
>              } else {
>                  sbrec_port_binding_set_type(op->sb, "patch");
> @@ -3060,7 +3191,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>
>              const char *router_port = smap_get(&op->nbsp->options,
>                                                 "router-port");
> -            if (router_port || chassis) {
> +            if (router_port || peer_is_gateway) {
>                  struct smap new;
>                  smap_init(&new);
>                  if (router_port) {
> @@ -3075,84 +3206,35 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>                  sbrec_port_binding_set_options(op->sb, NULL);
>              }
>
> -            const char *nat_addresses = smap_get(&op->nbsp->options,
> -                                           "nat-addresses");
> -            size_t n_nats = 0;
> -            char **nats = NULL;
> -            if (nat_addresses && !strcmp(nat_addresses, "router")) {
> -                if (op->peer && op->peer->od
> -                    && (chassis || op->peer->od->l3redirect_port)) {
> -                    nats = get_nat_addresses(op->peer, &n_nats);
> -                }
> -            /* Only accept manual specification of ethernet address
> -             * followed by IPv4 addresses on type "l3gateway" ports. */
> -            } else if (nat_addresses && chassis) {
> -                struct lport_addresses laddrs;
> -                if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
> -                    static struct vlog_rate_limit rl =
> -                        VLOG_RATE_LIMIT_INIT(1, 1);
> -                    VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
> -                } else {
> -                    destroy_lport_addresses(&laddrs);
> -                    n_nats = 1;
> -                    nats = xcalloc(1, sizeof *nats);
> -                    nats[0] = xstrdup(nat_addresses);
> -                }
> -            }
> +            size_t n_nats;
> +            char **nats;
> +            nats = get_nat_addresses(op->peer, &n_nats);
>
> -            /* Add the router mac and IPv4 addresses to
> -             * Port_Binding.nat_addresses so that GARP is sent for these
> -             * IPs by the ovn-controller on which the distributed gateway
> -             * router port resides if:
> -             *
> -             * -  op->peer has 'reside-on-redirect-chassis' set and the
> -             *    the logical router datapath has distributed router port.
> -             *
> -             * -  op->peer is distributed gateway router port.
> -             *
> -             * -  op->peer's router is a gateway router and op has a localnet
> -             *    port.
> -             *
> -             * Note: Port_Binding.nat_addresses column is also used for
> -             * sending the GARPs for the router port IPs.
> -             * */
> -            bool add_router_port_garp = false;
> -            if (op->peer && op->peer->nbrp && op->peer->od->l3dgw_port &&
> -                op->peer->od->l3redirect_port &&
> -                (smap_get_bool(&op->peer->nbrp->options,
> -                              "reside-on-redirect-chassis", false) ||
> -                op->peer == op->peer->od->l3dgw_port)) {
> -                add_router_port_garp = true;
> -            } else if (chassis && op->od->n_localnet_ports) {
> -                add_router_port_garp = true;
> -            }
> -
> -            if (add_router_port_garp) {
> -                struct ds garp_info = DS_EMPTY_INITIALIZER;
> -                ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
> +            struct ds router_networks = DS_EMPTY_INITIALIZER;
> +            if (op->peer) {
> +                ds_put_format(&router_networks, "%s", op->peer->lrp_networks.ea_s);
>                  for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
>                       i++) {
> -                    ds_put_format(&garp_info, " %s",
> +                    ds_put_format(&router_networks, " %s",
>                                    op->peer->lrp_networks.ipv4_addrs[i].addr_s);
> -                }
>
> -                if (op->peer->od->l3redirect_port) {
> -                    ds_put_format(&garp_info, " is_chassis_resident(%s)",
> -                                  op->peer->od->l3redirect_port->json_key);
> +                    if (peer_has_dist_gw_port) {
> +                        ds_put_format(&router_networks, " is_chassis_resident(%s)",
> +                                      op->peer->od->l3redirect_port->json_key);
> +                    }
>                  }
> -
> -                n_nats++;
> -                nats = xrealloc(nats, (n_nats * sizeof *nats));
> -                nats[n_nats - 1] = ds_steal_cstr(&garp_info);
> -                ds_destroy(&garp_info);
>              }
> +            ovn_port_set_garp_addresses(op, (const char **) nats, n_nats,
> +                                        ds_cstr(&router_networks));
> +
> +            ovn_port_set_router_addresses(op, (const char **) nats, n_nats,
> +                                          ds_cstr(&router_networks));
>

If I understand correctly, the router_addresses column of the
port_binding is set
for all the logical switch ports of type 'router' right ?

Is that really required ?  Setting the router_addresses for the port
binding whose
peer is a distributed gateway port should be enough right ? So that
the ovn-controller
on the gateway chassis will create chassis mac bindings.

If you consider the below logical resources
-----
switch 199fa7b4-7bef-43ff-ba0b-63639ff20e4b (sw1)
    port sw1-port2
        addresses: ["40:54:00:00:00:04 20.0.0.4"]
    port sw1-port1
        addresses: ["40:54:00:00:00:03 20.0.0.3"]
    port sw1-lr0
        type: router
        addresses: ["00:00:00:00:ff:02"]
        router-port: lr0-sw1
switch c4d6c8a9-1bdd-440b-9e71-94dcf1dea37f (sw0)
    port sw0-port2
        addresses: ["50:54:00:00:00:04 10.0.0.4"]
    port sw0-port1
        addresses: ["50:54:00:00:00:03 10.0.0.3"]
    port sw0-lr0
        type: router
        addresses: ["00:00:00:00:ff:01"]
        router-port: lr0-sw0
switch ca698d26-fb3e-4e22-8105-16764eefb8f0 (public)
    port ln-public
        type: localnet
        addresses: ["unknown"]
    port public-lr0
        type: router
        router-port: lr0-public
router 8931a35d-62b8-4d15-aafd-87cdbb5a050c (lr0)
    port lr0-sw1
        mac: "00:00:00:00:ff:02"
        networks: ["20.0.0.1/24"]
    port lr0-sw0
        mac: "00:00:00:00:ff:01"
        networks: ["10.0.0.1/24"]
    port lr0-public
        mac: "00:00:20:20:12:13"
        networks: ["172.168.0.100/24"]
        gateway chassis: [chassis-1]
    nat 93376277-ad24-4e06-8a90-d2da881b1757
        external ip: "172.168.0.100"
        logical ip: "10.0.0.0/24"
        type: "snat"
    nat 980f9681-fd54-459d-85e5-f80a6fc44871
        external ip: "172.168.0.120"
        logical ip: "20.0.0.3"
        type: "dnat_and_snat"
    nat a6274b11-0e14-406b-acf5-c98a89d393bc
        external ip: "172.168.0.110"
        logical ip: "10.0.0.3"
        type: "dnat_and_snat"
    nat c02500f4-c612-4791-b119-25d051c5361e
        external ip: "172.168.0.100"
        logical ip: "20.0.0.0/24"
        type: "snat"

-----

I see the below o/p

----
$ovn-sbctl --columns router_addresses list port_Binding sw0-lr0
router_addresses    : ["00:00:00:00:ff:01 10.0.0.1
is_chassis_resident(\"cr-lr0-public\")", "00:00:00:00:ff:01
172.168.0.100 172.168.0.100 is_chassis_resident(\"cr-lr0-public\")",
"30:54:00:00:00:03 172.168.0.110 is_chassis_resident(\"sw0-port1\")",
"30:54:00:00:00:04 172.168.0.120 is_chassis_resident(\"sw1-port1\")"]

$ ovn-sbctl --columns router_addresses list port_Binding sw1-lr0
router_addresses    : ["00:00:00:00:ff:02 172.168.0.100 172.168.0.100
is_chassis_resident(\"cr-lr0-public\")", "00:00:00:00:ff:02 20.0.0.1
is_chassis_resident(\"cr-lr0-public\")", "30:54:00:00:00:03
172.168.0.110 is_chassis_resident(\"sw0-port1\")", "30:54:00:00:00:04
172.168.0.120 is_chassis_resident(\"sw1-port1\")"]

$ovn-sbctl --columns router_addresses list port_Binding public-lr0
router_addresses    : ["00:00:20:20:12:13 172.168.0.100
is_chassis_resident(\"cr-lr0-public\")", "30:54:00:00:00:03
172.168.0.110 is_chassis_resident(\"sw0-port1\")", "30:54:00:00:00:04
172.168.0.120 is_chassis_resident(\"sw1-port1\")"]

----

In my opinion storing the router_addresses for public-lr0 should be
enough.  What do you think ?




> -            sbrec_port_binding_set_nat_addresses(op->sb,
> -                                                 (const char **) nats, n_nats);
>              for (size_t i = 0; i < n_nats; i++) {
>                  free(nats[i]);
>              }
>              free(nats);
> +            ds_destroy(&router_networks);
>          }
>
>          sbrec_port_binding_set_parent_port(op->sb, op->nbsp->parent_name);
> @@ -14119,6 +14201,8 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
>      add_column_noalert(ovnsb_idl_loop.idl,
>                         &sbrec_port_binding_col_nat_addresses);
> +    add_column_noalert(ovnsb_idl_loop.idl,
> +                       &sbrec_port_binding_col_router_addresses);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl,
>                           &sbrec_port_binding_col_gateway_chassis);
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index afadac99a..2be6591e9 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -116,6 +116,7 @@ relation OutProxy_Port_Binding (
>      tag: Option<integer>,
>      mac: Set<string>,
>      nat_addresses: Set<string>,
> +    router_addresses: Set<string>,
>      external_ids: Map<string,string>
>  )
>
> @@ -131,6 +132,7 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
>                        .tag                = tag,
>                        .mac                = lsp.addresses,
>                        .nat_addresses      = set_empty(),
> +                      .router_addresses   = set_empty(),
>                        .external_ids       = eids) :-
>      sp in &SwitchPort(.lsp = lsp, .sw = &sw),
>      SwitchPortNewDynamicTag(lsp._uuid, opt_tag),
> @@ -169,6 +171,7 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
>                        .tag                = None,
>                        .mac                = lsp.addresses,
>                        .nat_addresses      = nat_addresses,
> +                      .router_addresses   = router_addresses,
>                        .external_ids       = eids) :-
>      &SwitchPort(.lsp = lsp, .sw = &sw, .peer = peer),
>      var eids = {
> @@ -194,13 +197,17 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
>              }
>          }
>      },
> -    var base_nat_addresses = {
> +    var all_nat_addresses = match (peer) {
> +        None -> { set_empty() },
> +        Some{rport} -> get_nat_addresses(deref(rport))
> +    },
> +    var garp_nat_addresses = {
>          match (lsp.options.get("nat-addresses")) {
>              None -> { set_empty() },
>              Some{"router"} -> match ((l3dgw_port, opt_chassis, peer)) {
>                                   (None, None, _) -> set_empty(),
>                                   (_, _, None) -> set_empty(),
> -                                 (_, _, Some{rport}) -> get_nat_addresses(deref(rport))
> +                                 (_, _, Some{rport}) -> all_nat_addresses
>                                },
>              Some{nat_addresses} -> {
>                  /* Only accept manual specification of ethernet address
> @@ -233,7 +240,11 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
>       * Note: Port_Binding.nat_addresses column is also used for
>       * sending the GARPs for the router port IPs.
>       * */
> -    var garp_nat_addresses = match (peer) {
> +    var router_interface_addresses = match (peer) {
> +        None -> set_empty(),
> +        Some{rport} -> set_singleton(get_router_interface_addresses(deref(rport)))
> +    },
> +    var garp_router_interface_addresses = match (peer) {
>          Some{rport} -> match (
>              (rport.lrp.options.get_bool_def("reside-on-redirect-chassis", false)
>               and l3dgw_port.is_some()) or
> @@ -241,11 +252,12 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
>              (rport.router.lr.options.contains_key("chassis") and
>               not sw.localnet_ports.is_empty())) {
>              false -> set_empty(),
> -            true -> set_singleton(get_garp_nat_addresses(deref(rport)))
> +            true -> router_interface_addresses
>          },
>          None -> set_empty()
>      },
> -    var nat_addresses = set_union(base_nat_addresses, garp_nat_addresses).
> +    var nat_addresses = set_union(garp_nat_addresses, garp_router_interface_addresses),
> +    var router_addresses = set_union(all_nat_addresses, router_interface_addresses).
>
>  /* Case 3: Port_Binding per logical router port */
>  OutProxy_Port_Binding(._uuid              = lrp._uuid,
> @@ -259,6 +271,7 @@ OutProxy_Port_Binding(._uuid              = lrp._uuid,
>                        .tag                = None, // always empty for router ports
>                        .mac                = set_singleton("${lrp.mac} ${lrp.networks.join(\" \")}"),
>                        .nat_addresses      = set_empty(),
> +                      .router_addresses   = set_empty(),
>                        .external_ids       = lrp.external_ids) :-
>      rp in &RouterPort(.lrp = lrp, .router = &router, .peer = peer),
>      RouterPortRAOptionsComplete(lrp._uuid, options0),
> @@ -400,15 +413,15 @@ function get_nat_addresses(rport: RouterPort): Set<string> =
>      }
>  }
>
> -function get_garp_nat_addresses(rport: RouterPort): string = {
> -    var garp_info = ["${rport.networks.ea}"];
> +function get_router_interface_addresses(rport: RouterPort): string = {
> +    var router_info = ["${rport.networks.ea}"];
>      for (ipv4_addr in rport.networks.ipv4_addrs) {
> -        garp_info.push("${ipv4_addr.addr}")
> +        router_info.push("${ipv4_addr.addr}")
>      };
>      if (rport.router.redirect_port_name != "") {
> -        garp_info.push("is_chassis_resident(${rport.router.redirect_port_name})")
> +        router_info.push("is_chassis_resident(${rport.router.redirect_port_name})")
>      };
> -    garp_info.join(" ")
> +    router_info.join(" ")
>  }
>
>  /* Extra options computed for router ports by the logical flow generation code */
> @@ -441,6 +454,7 @@ OutProxy_Port_Binding(// lrp._uuid is already in use; generate a new UUID by
>                        .tag                = None,  //always empty for router ports
>                        .mac                = set_singleton("${lrp.mac} ${lrp.networks.join(\" \")}"),
>                        .nat_addresses      = set_empty(),
> +                      .router_addresses   = set_empty(),
>                        .external_ids       = lrp.external_ids) :-
>      DistributedGatewayPort(lrp, lr_uuid),
>      LogicalRouterHAChassisGroup(lr_uuid, hacg_uuid),
> @@ -478,6 +492,7 @@ sb::Out_Port_Binding(._uuid              = pbinding._uuid,
>                      .tag                = pbinding.tag,
>                      .mac                = pbinding.mac,
>                      .nat_addresses      = pbinding.nat_addresses,
> +                    .router_addresses   = pbinding.router_addresses,
>                      .external_ids       = pbinding.external_ids,
>                      .up                 = Some{up}) :-
>      pbinding in OutProxy_Port_Binding(),
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 205a30a37..b879e6228 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
>      "version": "20.17.0",
> -    "cksum": "669123379 26536",
> +    "cksum": "367711378 26723",
>      "tables": {
>          "SB_Global": {
>              "columns": {
> @@ -225,6 +225,9 @@
>                  "nat_addresses": {"type": {"key": "string",
>                                             "min": 0,
>                                             "max": "unlimited"}},
> +                "router_addresses": {"type": {"key": "string",
> +                                              "min": 0,
> +                                              "max": "unlimited"}},
>                  "up": {"type": {"key": "boolean", "min": 0, "max": 1}},
>                  "external_ids": {"type": {"key": "string",
>                                   "value": "string",
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index b853a5031..610dca17c 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2994,6 +2994,16 @@ tcp.flags = RST;
>          158.36.44.24 with a MAC address of 80:fa:5b:06:72:b7 from the chassis
>          where the logical port "foo1" resides.
>        </column>
> +
> +      <column name="router_addresses">
> +        This is an exhaustive list of all NAT, load balancer, and interface
> +        addresses of the connected router port. The format of the entries is
> +        the same as the <ref column="nat_addresses"/> column. Whereas the
> +        <ref column="nat_addresses"/> column is used to determine the addresses
> +        for which ovn-controller will send gratuitous ARPs, the addresses in
> +        this column are used to pre-set <ref table="MAC_Binding"/>s to avoid
> +        the need for sending unnecessary ARPs.
> +      </column>
>      </group>
>
>      <group title="L3 Gateway Options">
> @@ -3024,6 +3034,16 @@ tcp.flags = RST;
>          158.36.44.22 and 158.36.44.24 with a MAC address of 80:fa:5b:06:72:b7.
>          This is used in OVS version 2.8 and later versions.
>        </column>
> +
> +      <column name="router_addresses">
> +        This is an exhaustive list of all NAT, load balancer, and interface
> +        addresses of the connected router port. The format of the entries is
> +        the same as the <ref column="nat_addresses"/> column. Whereas the
> +        <ref column="nat_addresses"/> column is used to determine the addresses
> +        for which ovn-controller will send gratuitous ARPs, the addresses in
> +        this column are used to pre-set <ref table="MAC_Binding"/>s to avoid
> +        the need for sending unnecessary ARPs.
> +      </column>
>      </group>
>
>      <group title="Localnet Options">
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
00> index eef360802..38f18a28e 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2974,3 +2974,209 @@ ovn-sbctl list FDB
>
>  AT_CLEANUP
>  ])
> +
> +# XXX This test currently only runs for ovn-northd.c. The test fails
> +# with ovn-northd-ddlog because of the section where 2 HA_Chassis_Groups
> +# are used by 2 routers. For some reason, this causes ovn-northd-ddlog
> +# to stop processing new changes to the northbound database and to
> +# seemingly infinitely loop. This issue has been reported, but there is
> +# currently no fix for it. Once this issue is fixed, we can run this
> +# test for both C and DDLog versions of northd.

I can see that the test case is failing if I enable the test for
ddlog.  Looks like there is some
bug in ovn-northd-ddlog. In the northd-ddlog logs, I see the below
messages flooded
--
2021-04-21T12:34:28.443Z|69065|jsonrpc|DBG|unix:/home/nusiddiq/workspace_cpp/ovn-org/ovn/_gcc_ddlog/tests/testsuite.dir/784/ovn-sb/ovn-sb.sock:
received reply,
result=[{"uuid":["uuid","7c9e809e-de09-3999-ba23-e72b6965061d"]},{"uuid":["uuid","dcc3a5c9-9101-12fe-a008-fa64449a1afa"]},{"uuid":["uuid","16475605-7497-5611-88cf-b7945bcf90b5"]},{"syntax":"\"16475605-7497-5611-88cf-b7945bcf90b5\"","details":"This
UUID would duplicate a UUID already present within the table or
deleted within the same transaction.","error":"duplicate
uuid"},null,null,null,null,null,null,null,null,null,null,null],
id=22987
---

Thanks
Numan


> +AT_SETUP([ovn -- Router Address Propagation])
> +ovn_start
> +
> +AS_BOX([Setting up the logical network])
> +
> +check ovn-nbctl ls-add sw
> +
> +check ovn-nbctl lr-add ro1
> +check ovn-nbctl lrp-add ro1 ro1-sw 00:00:00:00:00:01 10.0.0.1/24
> +check ovn-nbctl lsp-add sw sw-ro1
> +
> +check ovn-nbctl lr-add ro2
> +check ovn-nbctl lrp-add ro2 ro2-sw 00:00:00:00:00:02 20.0.0.1/24
> +check ovn-nbctl --wait=sb lsp-add sw sw-ro2
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 vm1
> +check ovn-nbctl lsp-set-addresses vm1 "00:00:00:00:01:02 192.168.1.2"
> +check ovn-nbctl lrp-add ro1 ro1-ls1 00:00:00:00:01:01 192.168.1.1/24
> +check ovn-nbctl lsp-add ls1 ls1-ro1
> +check ovn-nbctl lsp-set-type ls1-ro1 router
> +check ovn-nbctl lsp-set-addresses ls1-ro1 router
> +check ovn-nbctl lsp-set-options ls1-ro1 router-port=ro1-ls1
> +
> +check ovn-nbctl ls-add ls2
> +check ovn-nbctl lsp-add ls2 vm2
> +check ovn-nbctl lsp-set-addresses vm2 "00:00:00:00:02:02 192.168.2.2"
> +check ovn-nbctl lrp-add ro2 ro2-ls2 00:00:00:00:02:01 192.168.2.1/24
> +check ovn-nbctl lsp-add ls2 ls2-ro2
> +check ovn-nbctl lsp-set-type ls2-ro2 router
> +check ovn-nbctl lsp-set-addresses ls2-ro2 router
> +check ovn-nbctl lsp-set-options ls2-ro2 router-port=ro2-ls2
> +
> +check ovn-nbctl ha-chassis-group-add grp1
> +check ovn-nbctl ha-chassis-group-add-chassis grp1 hv1 100
> +grp1_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp1)
> +
> +check ovn-nbctl ha-chassis-group-add grp2
> +check ovn-nbctl ha-chassis-group-add-chassis grp2 hv2 100
> +grp2_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp2)
> +
> +AS_BOX([Checking that unconnected logical switch ports have no router_addresses])
> +
> +check_column "" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that connected logical switch ports have router_addresses])
> +
> +check ovn-nbctl lsp-set-type sw-ro1 router
> +check ovn-nbctl lsp-set-addresses sw-ro1 router
> +check ovn-nbctl lsp-set-options sw-ro1 router-port=ro1-sw
> +
> +check ovn-nbctl lsp-set-type sw-ro2 router
> +check ovn-nbctl lsp-set-addresses sw-ro2 router
> +check ovn-nbctl --wait=sb lsp-set-options sw-ro2 router-port=ro2-sw
> +
> +check_column "00:00:00:00:00:01 10.0.0.1" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that NAT addresses are propagated without is_chassis_resident for non-gateway routers])
> +
> +check ovn-nbctl lr-nat-add ro1 dnat 10.0.0.100 192.168.1.100
> +check ovn-nbctl lr-nat-add ro1 snat 10.0.0.200 192.168.1.200/30
> +
> +check ovn-nbctl lr-nat-add ro2 dnat 20.0.0.100 192.168.2.100
> +check ovn-nbctl --wait=sb lr-nat-add ro2 snat 20.0.0.200 192.168.2.200/30
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100 10.0.0.200" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100 20.0.0.200" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that NAT addresses are propagated with is_chassis_resident for gateway routers])
> +
> +check ovn-nbctl lrp-set-gateway-chassis ro1-sw hv1 100
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis ro2-sw hv2 100
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 10.0.0.200 is_chassis_resident(\"cr-ro1-sw\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 20.0.0.200 is_chassis_resident(\"cr-ro2-sw\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that NAT addresses are propagated without is_chassis_resident for routers with gateway chassis removed])
> +
> +check ovn-nbctl lrp-del-gateway-chassis ro1-sw hv1
> +check ovn-nbctl --wait=sb lrp-del-gateway-chassis ro2-sw hv2
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100 10.0.0.200" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100 20.0.0.200" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that NAT addresses are propagated with is_chassis_resident for routers with ha_chassis_group])
> +
> +check ovn-nbctl set logical_router_port ro1-sw ha_chassis_group="$grp1_uuid"
> +check ovn-nbctl --wait=sb set logical_router_port ro2-sw ha_chassis_group="$grp2_uuid"
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 10.0.0.200 is_chassis_resident(\"cr-ro1-sw\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 20.0.0.200 is_chassis_resident(\"cr-ro2-sw\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that NAT addresses are propagated without is_chassis_resident for routers with HA_Chassis_Group removed])
> +
> +check ovn-nbctl clear logical_router_port ro1-sw ha_chassis_group
> +check ovn-nbctl --wait=sb clear logical_router_port ro2-sw ha_chassis_group
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100 10.0.0.200" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100 20.0.0.200" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Floating IP NAT addresses are propagated without is_chassis_resident with no gateway port set])
> +
> +check ovn-nbctl lr-nat-del ro1
> +check ovn-nbctl lr-nat-del ro2
> +
> +check ovn-nbctl lr-nat-add ro1 dnat_and_snat 10.0.0.100 192.168.1.2 vm1 00:00:00:00:00:01
> +check ovn-nbctl lr-nat-add ro2 dnat_and_snat 20.0.0.100 192.168.2.2 vm2 00:00:00:00:00:02
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Floating IP NAT addresses are propagated with is_chassis_resident for gateway routers])
> +
> +check ovn-nbctl lrp-set-gateway-chassis ro1-sw hv1 100
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis ro2-sw hv2 100
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 is_chassis_resident(\"vm1\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 is_chassis_resident(\"vm2\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Floating IP NAT addresses are propagated without is_chassis_resident for routers with gateway chassis removed])
> +
> +check ovn-nbctl lrp-del-gateway-chassis ro1-sw hv1
> +check ovn-nbctl --wait=sb lrp-del-gateway-chassis ro2-sw hv2
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Floating IP NAT addresses are propagated with is_chassis_resident for routers with ha_chassis_group])
> +
> +grp1_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp1)
> +check ovn-nbctl set logical_router_port ro1-sw ha_chassis_group="$grp1_uuid"
> +
> +grp2_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp2)
> +check ovn-nbctl --wait=sb set logical_router_port ro2-sw ha_chassis_group="$grp2_uuid"
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 is_chassis_resident(\"vm1\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 is_chassis_resident(\"vm2\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Floating IP NAT addresses are propagated without is_chassis_resident for routers with HA_Chassis_Group removed])
> +
> +check ovn-nbctl clear logical_router_port ro1-sw ha_chassis_group
> +check ovn-nbctl --wait=sb clear logical_router_port ro2-sw ha_chassis_group
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Load Balancer VIP addresses are propagated without is_chassis_resident for routers with no gateway port])
> +
> +check ovn-nbctl lr-nat-del ro1
> +check ovn-nbctl lr-nat-del ro2
> +
> +check ovn-nbctl lb-add lb1 10.0.0.100 192.168.1.2
> +check ovn-nbctl lr-lb-add ro1 lb1
> +
> +check ovn-nbctl lb-add lb2 20.0.0.100 192.168.2.2
> +check ovn-nbctl lr-lb-add ro2 lb2
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Load Balancer VIP addresses are propagated with is_chassis_resident for gateway routers])
> +
> +check ovn-nbctl lrp-set-gateway-chassis ro1-sw hv1 100
> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis ro2-sw hv2 100
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 is_chassis_resident(\"cr-ro1-sw\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 is_chassis_resident(\"cr-ro2-sw\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Load Balancer VIP addresses are propagated without is_chassis_resident for routers with gateway chassis removed])
> +
> +check ovn-nbctl lrp-del-gateway-chassis ro1-sw hv1
> +check ovn-nbctl --wait=sb lrp-del-gateway-chassis ro2-sw hv2
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Load Balancer VIP addresses are propagated with is_chassis_resident for routers with ha_chassis_group])
> +
> +grp1_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp1)
> +check ovn-nbctl set logical_router_port ro1-sw ha_chassis_group="$grp1_uuid"
> +
> +grp2_uuid=$(ovn-nbctl --columns=_uuid --bare find HA_Chassis_group name=grp2)
> +check ovn-nbctl --wait=sb set logical_router_port ro2-sw ha_chassis_group="$grp2_uuid"
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 is_chassis_resident(\"cr-ro1-sw\") 00:00:00:00:00:01 10.0.0.100 is_chassis_resident(\"cr-ro1-sw\")" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 is_chassis_resident(\"cr-ro2-sw\") 00:00:00:00:00:02 20.0.0.100 is_chassis_resident(\"cr-ro2-sw\")" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AS_BOX([Checking that Load Balancer VIP addresses are propagated without is_chassis_resident for routers with HA_Chassis_Group removed])
> +
> +check ovn-nbctl clear logical_router_port ro1-sw ha_chassis_group
> +check ovn-nbctl --wait=sb clear logical_router_port ro2-sw ha_chassis_group
> +
> +check_column "00:00:00:00:00:01 10.0.0.1 00:00:00:00:00:01 10.0.0.100" Port_Binding router_addresses logical_port=sw-ro1
> +check_column "00:00:00:00:00:02 20.0.0.1 00:00:00:00:00:02 20.0.0.100" Port_Binding router_addresses logical_port=sw-ro2
> +
> +AT_CLEANUP
> --
> 2.29.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>



More information about the dev mailing list