On Wed, Jul 7, 2021 at 1:35 PM Numan Siddique <[email protected]> wrote:
>
> On Wed, Jun 30, 2021 at 7:57 PM Mark Michelson <[email protected]> wrote:
> >
> > Load_Balancer and NAT entries have a new option, "add_route" that can be
> > set to automatically add routes to those addresses to neighbor routers,
> > therefore eliminating the need to create static routes.
> >
> > Signed-off-by: Mark Michelson <[email protected]>
>
> Acked-by: Numan Siddique <[email protected]>

There's also a couple of checkpatch errors which need fixing before applying.

Numan

>
> There is one small comment which you can choose to ignore.  Please see below.
>
> Numan
>
> > ---
> >  northd/ovn-northd.8.xml |  7 ++++-
> >  northd/ovn-northd.c     | 57 +++++++++++++++++++++++++++++++++--------
> >  northd/ovn_northd.dl    | 23 ++++++++++++-----
> >  ovn-nb.xml              | 33 +++++++++++++++++++++++-
> >  tests/ovn-nbctl.at      |  3 +++
> >  tests/ovn-northd.at     | 40 ++++++++++++++++++++++++++---
> >  utilities/ovn-nbctl.c   | 25 +++++++++++++-----
> >  7 files changed, 158 insertions(+), 30 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index b5c961e89..beaf5a183 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -3539,7 +3539,12 @@ outport = <var>P</var>
> >            <ref column="external_mac" table="NAT" db="OVN_Northbound"/> 
> > column
> >            of <ref table="NAT" db="OVN_Northbound"/> table for of type
> >            <code>dnat_and_snat</code>, otherwise the Ethernet address of the
> > -          distributed logical router port.
> > +          distributed logical router port. Note that if the
> > +          <ref column="external_ip" table="NAT" db="OVN_Northbound"/> is 
> > not
> > +          within a subnet on the owning logical router, then OVN will only
> > +          create ARP resolution flows if the <ref 
> > column="options:add_route"/>
> > +          is set to <code>true</code>. Otherwise, no ARP resolution flows
> > +          will be added.
> >          </p>
> >
> >          <p>
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 58132bc5c..f6fad281b 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -662,8 +662,14 @@ struct ovn_datapath {
> >      struct lport_addresses dnat_force_snat_addrs;
> >      struct lport_addresses lb_force_snat_addrs;
> >      bool lb_force_snat_router_ip;
> > +    /* The "routable" ssets are subsets of the load balancer
> > +     * IPs for which IP routes and ARP resolution flows are automatically
> > +     * added
> > +     */
> >      struct sset lb_ips_v4;
> > +    struct sset lb_ips_v4_routable;
> >      struct sset lb_ips_v6;
> > +    struct sset lb_ips_v6_routable;
> >
> >      struct ovn_port **localnet_ports;
> >      size_t n_localnet_ports;
> > @@ -834,7 +840,9 @@ static void
> >  init_lb_ips(struct ovn_datapath *od)
> >  {
> >      sset_init(&od->lb_ips_v4);
> > +    sset_init(&od->lb_ips_v4_routable);
> >      sset_init(&od->lb_ips_v6);
> > +    sset_init(&od->lb_ips_v6_routable);
> >  }
> >
> >  static void
> > @@ -845,7 +853,9 @@ destroy_lb_ips(struct ovn_datapath *od)
> >      }
> >
> >      sset_destroy(&od->lb_ips_v4);
> > +    sset_destroy(&od->lb_ips_v4_routable);
> >      sset_destroy(&od->lb_ips_v6);
> > +    sset_destroy(&od->lb_ips_v6_routable);
> >  }
> >
> >  /* A group of logical router datapaths which are connected - either
> > @@ -1475,13 +1485,14 @@ destroy_routable_addresses(struct 
> > ovn_port_routable_addresses *ra)
> >      free(ra->laddrs);
> >  }
> >
> > -static char **get_nat_addresses(const struct ovn_port *op, size_t *n);
> > +static char **get_nat_addresses(const struct ovn_port *op, size_t *n,
> > +                                bool routable_only);
> >
> >  static void
> >  assign_routable_addresses(struct ovn_port *op)
> >  {
> >      size_t n;
> > -    char **nats = get_nat_addresses(op, &n);
> > +    char **nats = get_nat_addresses(op, &n, true);
> >
> >      if (!nats) {
> >          return;
> > @@ -2541,7 +2552,7 @@ join_logical_ports(struct northd_context *ctx,
> >   * The caller must free each of the n returned strings with free(),
> >   * and must free the returned array when it is no longer needed. */
> >  static char **
> > -get_nat_addresses(const struct ovn_port *op, size_t *n)
> > +get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only)
> >  {
> >      size_t n_nats = 0;
> >      struct eth_addr mac;
> > @@ -2564,6 +2575,12 @@ get_nat_addresses(const struct ovn_port *op, size_t 
> > *n)
> >          const struct nbrec_nat *nat = op->od->nbr->nat[i];
> >          ovs_be32 ip, mask;
> >
> > +        if (routable_only &&
> > +            (!strcmp(nat->type, "snat") ||
> > +             !smap_get_bool(&nat->options, "add_route", false))) {
> > +            continue;
> > +        }
> > +
> >          char *error = ip_parse_masked(nat->external_ip, &ip, &mask);
> >          if (error || mask != OVS_BE32_MAX) {
> >              free(error);
> > @@ -2615,13 +2632,24 @@ get_nat_addresses(const struct ovn_port *op, size_t 
> > *n)
> >      }
> >
> >      const char *ip_address;
> > -    SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) {
> > -        ds_put_format(&c_addresses, " %s", ip_address);
> > -        central_ip_address = true;
> > -    }
> > -    SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
> > -        ds_put_format(&c_addresses, " %s", ip_address);
> > -        central_ip_address = true;
> > +    if (routable_only) {
> > +        SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4_routable) {
> > +            ds_put_format(&c_addresses, " %s", ip_address);
> > +            central_ip_address = true;
> > +        }
> > +        SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6_routable) {
> > +            ds_put_format(&c_addresses, " %s", ip_address);
> > +            central_ip_address = true;
> > +        }
> > +    } else {
> > +        SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) {
> > +            ds_put_format(&c_addresses, " %s", ip_address);
> > +            central_ip_address = true;
> > +        }
> > +        SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
> > +            ds_put_format(&c_addresses, " %s", ip_address);
> > +            central_ip_address = true;
> > +        }
> >      }
> >
> >      if (central_ip_address) {
> > @@ -3133,7 +3161,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >              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);
> > +                    nats = get_nat_addresses(op->peer, &n_nats, false);
> >                  }
> >              /* Only accept manual specification of ethernet address
> >               * followed by IPv4 addresses on type "l3gateway" ports. */
> > @@ -3606,11 +3634,18 @@ build_lrouter_lbs(struct hmap *datapaths, struct 
> > hmap *lbs)
> >                  ovn_northd_lb_find(lbs,
> >                                     
> > &od->nbr->load_balancer[i]->header_.uuid);
> >              const char *ip_address;
> > +            bool is_routable = smap_get_bool(&lb->nlb->options, 
> > "add_route", false);
> >              SSET_FOR_EACH (ip_address, &lb->ips_v4) {
> >                  sset_add(&od->lb_ips_v4, ip_address);
> > +                if (is_routable) {
> > +                    sset_add(&od->lb_ips_v4_routable, ip_address);
> > +                }
> >              }
> >              SSET_FOR_EACH (ip_address, &lb->ips_v6) {
> >                  sset_add(&od->lb_ips_v6, ip_address);
> > +                if (is_routable) {
> > +                    sset_add(&od->lb_ips_v6_routable, ip_address);
> > +                }
> >              }
> >          }
> >      }
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index 35d40ff5a..c9ee742d1 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -201,7 +201,7 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
> >              Some{"router"} -> match ((l3dgw_port, opt_chassis, peer)) {
> >                                   (None, None, _) -> set_empty(),
> >                                   (_, _, None) -> set_empty(),
> > -                                 (_, _, Some{rport}) -> 
> > get_nat_addresses(rport)
> > +                                 (_, _, Some{rport}) -> 
> > get_nat_addresses(rport, false)
> >                                },
> >              Some{nat_addresses} -> {
> >                  /* Only accept manual specification of ethernet address
> > @@ -298,12 +298,16 @@ OutProxy_Port_Binding(._uuid              = lrp._uuid,
> >      }.
> >  /*
> >  */
> > -function get_router_load_balancer_ips(router: Intern<Router>) :
> > +function get_router_load_balancer_ips(router: Intern<Router>,
> > +                                      routable_only: bool) :
> >      (Set<string>, Set<string>) =
> >  {
> >      var all_ips_v4 = set_empty();
> >      var all_ips_v6 = set_empty();
> >      for (lb in router.lbs) {
> > +        if (routable_only and not lb.options.get_bool_def("add_route", 
> > false)) {
> > +            continue;
> > +        };
> >          for (kv in lb.vips) {
> >              (var vip, _) = kv;
> >              /* node->key contains IP:port or just IP. */
> > @@ -325,7 +329,7 @@ function get_router_load_balancer_ips(router: 
> > Intern<Router>) :
> >   * external IP addresses of all NAT rules defined on that router, and all
> >   * of the IP addresses used in load balancer VIPs defined on that router.
> >   */
> > -function get_nat_addresses(rport: Intern<RouterPort>): Set<string> =
> > +function get_nat_addresses(rport: Intern<RouterPort>, routable_only: 
> > bool): Set<string> =
> >  {
> >      var addresses = set_empty();
> >      var has_redirect = rport.router.l3dgw_port.is_some();
> > @@ -337,6 +341,11 @@ function get_nat_addresses(rport: Intern<RouterPort>): 
> > Set<string> =
> >
> >              /* Get NAT IP addresses. */
> >              for (nat in rport.router.nats) {
> > +                if (routable_only and
> > +                    (nat.nat.__type == "snat" or
> > +                     not nat.nat.options.get_bool_def("add_route", 
> > false))) {
> > +                    continue;
> > +                };
> >                  /* Determine whether this NAT rule satisfies the 
> > conditions for
> >                   * distributed NAT processing. */
> >                  if (has_redirect and nat.nat.__type == "dnat_and_snat" and
> > @@ -379,7 +388,7 @@ function get_nat_addresses(rport: Intern<RouterPort>): 
> > Set<string> =
> >              };
> >
> >              /* A set to hold all load-balancer vips. */
> > -            (var all_ips_v4, var all_ips_v6) = 
> > get_router_load_balancer_ips(rport.router);
> > +            (var all_ips_v4, var all_ips_v6) = 
> > get_router_load_balancer_ips(rport.router, routable_only);
> >
> >              for (ip_address in set_union(all_ips_v4, all_ips_v6)) {
> >                  c_addresses = c_addresses ++ " ${ip_address}";
> > @@ -4105,7 +4114,7 @@ function get_arp_forward_ips(rp: Intern<RouterPort>): 
> > (Set<string>, Set<string>)
> >      var all_ips_v6 = set_empty();
> >
> >      (var lb_ips_v4, var lb_ips_v6)
> > -        = get_router_load_balancer_ips(rp.router);
> > +        = get_router_load_balancer_ips(rp.router, false);
> >      for (a in lb_ips_v4) {
> >          /* Check if the ovn port has a network configured on which we could
> >           * expect ARP requests for the LB VIP.
> > @@ -5090,7 +5099,7 @@ var residence_check = match (is_redirect) {
> >      true -> Some{"is_chassis_resident(${router.redirect_port_name})"},
> >      false -> None
> >  } in {
> > -    (var all_ips_v4, _) = get_router_load_balancer_ips(router) in {
> > +    (var all_ips_v4, _) = get_router_load_balancer_ips(router, false) in {
> >          if (not all_ips_v4.is_empty()) {
> >              LogicalRouterArpFlow(.lr = router,
> >                                   .lrp = Some{lrp},
> > @@ -6504,7 +6513,7 @@ relation RouterPortRoutableAddresses(
> >      addresses: Set<lport_addresses>)
> >  RouterPortRoutableAddresses(port.lrp._uuid, addresses) :-
> >      port in &RouterPort(.is_redirect = true),
> > -    var addresses = get_nat_addresses(port).filter_map(extract_addresses),
> > +    var addresses = get_nat_addresses(port, 
> > true).filter_map(extract_addresses),
> >      addresses != set_empty().
> >
> >  /* Return a vector of pairs (1, set[0]), ... (n, set[n - 1]). */
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 36a77097c..ad6deb059 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -1715,6 +1715,18 @@
> >          option, the force_snat_for_lb option configured for the router
> >          pipeline will not be applied for this load balancer.
> >        </column>
> > +
> > +      <column name="options" key="add_route">
> > +        If set to <code>true</code>, then neighbor routers will have 
> > logical
> > +        flows added that will allow for routing to the VIP IP. It also will
> > +        have ARP resolution logical flows added. By setting this option, it
> > +        means there is no reason to create a
> > +        <ref table="Logical_Router_Static_Route"/> from neighbor routers to
> > +        this NAT address. It also means that no ARP request is required for
> > +        neighbor routers to learn the IP-MAC mapping for this VIP IP. For
> > +        more information about what flows are added for IP routes, please
> > +        see the <code>ovn-northd</code> manpage section on IP Routing.
> > +      </column>
> >      </group>
> >    </table>
> >
> > @@ -2059,7 +2071,13 @@
> >            <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.
> > +          send traffic between each other. By default, ovn-northd does not
> > +          create mappings to NAT and load balancer addresess. However, for 
> > NAT
> > +          and load balancer addresses that have the <code>add_route</code>
> > +          option added, ovn-northd will create logical flows that map NAT 
> > and
> > +          load balancer IP addresses to the appropriate MAC address. 
> > Setting
> > +          <var>dynamic_neigh_routers</var> to <code>true</code> will 
> > prevent
> > +          the automatic creation of these logical flows.
> >          </p>
> >        </column>
> >        <column name="options" key="always_learn_from_arp_request" 
> > type='{"type": "boolean"}'>
> > @@ -3032,6 +3050,19 @@
> >        tracking state or not.
> >      </column>
> >
> > +    <column name="options" key="add_route">
> > +      If set to <code>true</code>, then neighbor routers will have logical
> > +      flows added that will allow for routing to the NAT address. It also 
> > will
> > +      have ARP resolution logical flows added. By setting this option, it 
> > means
> > +      there is no reason to create a <ref 
> > table="Logical_Router_Static_Route"/>
> > +      from neighbor routers to this NAT address. It also means that no ARP
> > +      request is required for neighbor routers to learn the IP-MAC mapping 
> > for
> > +      this NAT address. This option only applies to NATs of type
> > +      <code>dnat</code> and <code>dnat_and_snat</code>. For more 
> > information
> > +      about what flows are added for IP routes, please see the
> > +      <code>ovn-northd</code> manpage section on IP Routing.
> > +    </column>
> > +
> >      <group title="Common Columns">
> >        <column name="external_ids">
> >          See <em>External IDs</em> at the beginning of this document.
> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> > index 1058d418a..828777b82 100644
> > --- a/tests/ovn-nbctl.at
> > +++ b/tests/ovn-nbctl.at
> > @@ -465,6 +465,9 @@ AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
> >  AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.2 lp0 
> > 00:00:00:01:02], [1], [],
> >  [ovn-nbctl: invalid mac address 00:00:00:01:02.
> >  ])
> > +AT_CHECK([ovn-nbctl --add-route lr-nat-add lr0 snat 30.0.0.2 192.168.1.2], 
> > [1], [],
> > +[ovn-nbctl: routes cannot be added for snat types.
> > +])
> >
> >  dnl Add snat and dnat
> >  AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24])
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index feb38ea5e..0c8a09ced 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -3887,11 +3887,20 @@ check ovn-nbctl lr-nat-add ro2 dnat 20.0.0.100 
> > 192.168.2.100
> >
> >  check_lflows 0
> >
> > -AS_BOX([Checking that NAT flows are installed for gateway routers])
> > +AS_BOX([Checking that non-routable NAT flows are not installed 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_lflows 0
> > +
> > +AS_BOX([Checking that routable NAT flows are installed when gateway 
> > chassis exists])
> > +
> > +check ovn-nbctl lr-nat-del ro1
> > +check ovn-nbctl lr-nat-del ro2
> > +check ovn-nbctl --add-route lr-nat-add ro1 dnat 10.0.0.100 192.168.1.100
> > +check ovn-nbctl --add-route lr-nat-add ro2 dnat 20.0.0.100 192.168.2.100
> > +
> >  check_lflows 1
> >
> >  AS_BOX([Checking that NAT flows are not installed for routers with gateway 
> > chassis removed])
> > @@ -3925,11 +3934,20 @@ check ovn-nbctl lr-nat-add ro2 dnat_and_snat 
> > 20.0.0.100 192.168.2.2 vm2 00:00:00
> >
> >  check_lflows 0
> >
> > -AS_BOX([Checking that Floating IP NAT flows are installed for gateway 
> > routers])
> > +AS_BOX([Checking that non-routable Floating IP NAT flows are not installed 
> > 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_lflows 0
> > +
> > +AS_BOX([Checking that routable Floating IP NAT flows are installed for 
> > gateway routers])
> > +check ovn-nbctl lr-nat-del ro1
> > +check ovn-nbctl lr-nat-del ro2
> > +
> > +check ovn-nbctl --add-route 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 --add-route lr-nat-add ro2 dnat_and_snat 20.0.0.100 
> > 192.168.2.2 vm2 00:00:00:00:00:02
> > +
> >  check_lflows 1
> >
> >  AS_BOX([Checking that Floating IP NAT flows are not installed for routers 
> > with gateway chassis removed])
> > @@ -3965,15 +3983,29 @@ 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 ovn-nbctl --wait=sb lr-lb-add ro2 lb2
> >
> >  check_lflows 0
> >
> > -AS_BOX([Checking that Load Balancer VIP flows are installed for gateway 
> > routers])
> > +AS_BOX([Checking that non-routable Load Balancer VIP flows are not 
> > installed 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_lflows 0
> > +
> > +AS_BOX([Checking that routable Load Balancer VIP flows are installed for 
> > gateway routers])
> > +
> > +check ovn-nbctl lr-lb-del ro1 lb1
> > +check ovn-nbctl lr-lb-del ro2 lb2
> > +check ovn-nbctl lb-del lb1
> > +check ovn-nbctl lb-del lb2
> > +
> > +check ovn-nbctl --add-route lb-add lb1 10.0.0.100 192.168.1.2
> > +check ovn-nbctl --add-route lb-add lb2 20.0.0.100 192.168.2.2
> > +check ovn-nbctl lr-lb-add ro1 lb1
> > +check ovn-nbctl --wait=sb lr-lb-add ro2 lb2
> > +
> >  check_lflows 1
> >
> >  AS_BOX([Checking that Load Balancer VIP flows are not installed for 
> > routers with gateway chassis removed])
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index dc13fa9ca..f4a859495 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -367,6 +367,7 @@ Policy commands:\n\
> >  NAT commands:\n\
> >    [--stateless]\n\
> >    [--portrange]\n\
> > +  [--add-route]\n\
> >    lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT 
> > EXTERNAL_MAC]\n\
> >                              [EXTERNAL_PORT_RANGE]\n\
> >                              add a NAT to ROUTER\n\
> > @@ -2703,6 +2704,7 @@ nbctl_lb_add(struct ctl_context *ctx)
> >      bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != 
> > NULL;
> >      bool empty_backend_rej = shash_find(&ctx->options, "--reject") != NULL;
> >      bool empty_backend_event = shash_find(&ctx->options, "--event") != 
> > NULL;
> > +    bool add_route = shash_find(&ctx->options, "--add-route") != NULL;
> >
> >      if (empty_backend_event && empty_backend_rej) {
> >              ctl_error(ctx,
> > @@ -2822,14 +2824,18 @@ nbctl_lb_add(struct ctl_context *ctx)
> >      smap_add(CONST_CAST(struct smap *, &lb->vips),
> >              lb_vip_normalized, ds_cstr(&lb_ips_new));
> >      nbrec_load_balancer_set_vips(lb, &lb->vips);
> > +    struct smap options = SMAP_INITIALIZER(&options);
> >      if (empty_backend_rej) {
> > -        const struct smap options = SMAP_CONST1(&options, "reject", 
> > "true");
> > -        nbrec_load_balancer_set_options(lb, &options);
> > +        smap_add(&options, "reject", "true");
> >      }
> >      if (empty_backend_event) {
> > -        const struct smap options = SMAP_CONST1(&options, "event", "true");
> > -        nbrec_load_balancer_set_options(lb, &options);
> > +        smap_add(&options, "event", "true");
> > +    }
> > +    if (add_route) {
> > +        smap_add(&options, "add_route", "true");
> >      }
> > +    nbrec_load_balancer_set_options(lb, &options);
> > +    smap_destroy(&options);
> >  out:
> >      ds_destroy(&lb_ips_new);
> >
> > @@ -4400,6 +4406,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >      char *new_logical_ip = NULL;
> >      char *new_external_ip = NULL;
> >      bool is_portrange = shash_find(&ctx->options, "--portrange") != NULL;
> > +    bool add_route = shash_find(&ctx->options, "--add-route") != NULL;
> >
> >      char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
> >      if (error) {
> > @@ -4516,6 +4523,11 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >      }
> >
> >      int is_snat = !strcmp("snat", nat_type);
> > +
> > +    if (is_snat && add_route) {
> > +        ctl_error(ctx, "routes cannot be added for snat types.");
> > +        goto cleanup;
> > +    }
> >      for (size_t i = 0; i < lr->n_nat; i++) {
> >          const struct nbrec_nat *nat = lr->nat[i];
> >
> > @@ -4596,6 +4608,7 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >      }
> >
> >      smap_add(&nat_options, "stateless", stateless ? "true":"false");
> > +    smap_add(&nat_options, "add_route", add_route ? "true": "false");
>
> We can add the option "add_route=true" only if "--add-route" flag is set.
> ovn-northd anyway defaults to false if not set.  This would save some
> NB DB space.
>
> Thanks
> Numan
>
> >      nbrec_nat_set_options(nat, &nat_options);
> >
> >      smap_destroy(&nat_options);
> > @@ -6714,7 +6727,7 @@ static const struct ctl_command_syntax 
> > nbctl_commands[] = {
> >        "ROUTER TYPE EXTERNAL_IP LOGICAL_IP"
> >        "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]",
> >        nbctl_pre_lr_nat_add, nbctl_lr_nat_add,
> > -      NULL, "--may-exist,--stateless,--portrange", RW },
> > +      NULL, "--may-exist,--stateless,--portrange,--add-route", RW },
> >      { "lr-nat-del", 1, 3, "ROUTER [TYPE [IP]]",
> >        nbctl_pre_lr_nat_del, nbctl_lr_nat_del, NULL, "--if-exists", RW },
> >      { "lr-nat-list", 1, 1, "ROUTER", nbctl_pre_lr_nat_list,
> > @@ -6725,7 +6738,7 @@ static const struct ctl_command_syntax 
> > nbctl_commands[] = {
> >      /* load balancer commands. */
> >      { "lb-add", 3, 4, "LB VIP[:PORT] IP[:PORT]... [PROTOCOL]",
> >        nbctl_pre_lb_add, nbctl_lb_add, NULL,
> > -      "--may-exist,--add-duplicate,--reject,--event", RW },
> > +      "--may-exist,--add-duplicate,--reject,--event,--add-route", RW },
> >      { "lb-del", 1, 2, "LB [VIP]", nbctl_pre_lb_del, nbctl_lb_del, NULL,
> >          "--if-exists", RW },
> >      { "lb-list", 0, 1, "[LB]", nbctl_pre_lb_list, nbctl_lb_list, NULL, "", 
> > RO },
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to