[ovs-dev] [PATCH ovn v9 3/4] northd: Add options to automatically add routes for NATs and LBs.
Mark Michelson
mmichels at redhat.com
Wed Jun 30 23:56:31 UTC 2021
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 <mmichels at redhat.com>
---
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");
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
More information about the dev
mailing list