[ovs-dev] [PATCH ovn] northd: Always generate valid load balancer address set names.
Dumitru Ceara
dceara at redhat.com
Fri Oct 8 11:23:25 UTC 2021
Address set names have a fixed format ([a-zA-Z_.][a-zA-Z_.0-9]*) while
logical router names are free form. This means we cannot directly embed
the logical router name inside the address set name.
To make sure that address set names are unique and valid use instead
the Datapath_Binding.tunnel_key value which is guaranteed to be unique
across logical routers.
Fixes: c1e3896c0a39 ("northd: Use address sets for ARP responder flows for VIPs.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
northd/northd.c | 39 +++++++++++++++++++++++++++++++++++----
tests/ovn-northd.at | 27 +++++++++++++++------------
2 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index e42795ca0..c331f5fb1 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -878,6 +878,37 @@ lr_has_lb_vip(struct ovn_datapath *od)
return false;
}
+/* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*)
+ * for the router's load balancer VIP address set, combining the logical
+ * router's datapath tunnel key and address family.
+ *
+ * Also prefixes the name with 'prefix'.
+ */
+static char *
+lr_lb_address_set_name_(const struct ovn_datapath *od, const char *prefix,
+ int addr_family)
+{
+ ovs_assert(od->nbr);
+ return xasprintf("%s_rtr_lb_%"PRIu32"_ip%s", prefix, od->tunnel_key,
+ addr_family == AF_INET ? "4" : "6");
+}
+
+/* Builds the router's load balancer VIP address set name. */
+static char *
+lr_lb_address_set_name(const struct ovn_datapath *od, int addr_family)
+{
+ return lr_lb_address_set_name_(od, "", addr_family);
+}
+
+/* Builds a string that refers to the the router's load balancer VIP address
+ * set name, that is: $<address_set_name>.
+ */
+static char *
+lr_lb_address_set_ref(const struct ovn_datapath *od, int addr_family)
+{
+ return lr_lb_address_set_name_(od, "$", addr_family);
+}
+
static void
init_lb_for_datapath(struct ovn_datapath *od)
{
@@ -12009,7 +12040,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
}
/* Create a single ARP rule for all IPs that are used as VIPs. */
- char *lb_ips_v4_as = xasprintf("$%s_lb_ip4", op->od->nbr->name);
+ char *lb_ips_v4_as = lr_lb_address_set_ref(op->od, AF_INET);
build_lrouter_arp_flow(op->od, op, lb_ips_v4_as,
REG_INPORT_ETH_ADDR,
match, false, 90, NULL, lflows);
@@ -12025,7 +12056,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
}
/* Create a single ND rule for all IPs that are used as VIPs. */
- char *lb_ips_v6_as = xasprintf("$%s_lb_ip6", op->od->nbr->name);
+ char *lb_ips_v6_as = lr_lb_address_set_ref(op->od, AF_INET6);
build_lrouter_nd_flow(op->od, op, "nd_na", lb_ips_v6_as, NULL,
REG_INPORT_ETH_ADDR, match, false, 90,
NULL, lflows, meter_groups);
@@ -13682,7 +13713,7 @@ sync_address_sets(struct northd_context *ctx, struct hmap *datapaths)
}
if (sset_count(&od->lb_ips_v4)) {
- char *ipv4_addrs_name = xasprintf("%s_lb_ip4", od->nbr->name);
+ char *ipv4_addrs_name = lr_lb_address_set_name(od, AF_INET);
const char **ipv4_addrs = sset_array(&od->lb_ips_v4);
sync_address_set(ctx, ipv4_addrs_name, ipv4_addrs,
@@ -13692,7 +13723,7 @@ sync_address_sets(struct northd_context *ctx, struct hmap *datapaths)
}
if (sset_count(&od->lb_ips_v6)) {
- char *ipv6_addrs_name = xasprintf("%s_lb_ip6", od->nbr->name);
+ char *ipv6_addrs_name = lr_lb_address_set_name(od, AF_INET6);
const char **ipv6_addrs = sset_array(&od->lb_ips_v6);
sync_address_set(ctx, ipv6_addrs_name, ipv6_addrs,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index a31a5046f..8b9049899 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1701,10 +1701,13 @@ ovn-nbctl lr-lb-add lr lb4
ovn-nbctl lr-lb-add lr lb5
ovn-nbctl --wait=sb sync
+lr_key=$(fetch_column sb:datapath_binding tunnel_key external_ids:name=lr)
+lb_as_v4="_rtr_lb_${lr_key}_ip4"
+lb_as_v6="_rtr_lb_${lr_key}_ip6"
# Check generated VIP address sets.
-check_column '192.168.2.1 192.168.2.4 192.168.2.5 192.168.2.6' Address_Set addresses name=lr_lb_ip4
-check_column 'fe80::200:ff:fe00:101 fe80::200:ff:fe00:102' Address_Set addresses name=lr_lb_ip6
+check_column '192.168.2.1 192.168.2.4 192.168.2.5 192.168.2.6' Address_Set addresses name=${lb_as_v4}
+check_column 'fe80::200:ff:fe00:101 fe80::200:ff:fe00:102' Address_Set addresses name=${lb_as_v6}
# Ingress router port ETH address is stored in lr_in_admission.
AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_admission.*xreg0\[[0..47\]]" | sort], [0], [dnl
@@ -1723,7 +1726,7 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
])
# Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input.
-AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
+AT_CHECK_UNQUOTED([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
@@ -1737,7 +1740,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
@@ -1746,10 +1749,10 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp" && nd_ns && nd.target == $lr_lb_ip6), dnl
+match=(inport == "lrp" && nd_ns && nd.target == \$${lb_as_v6}), dnl
action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
@@ -1758,7 +1761,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100), dnl
action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == $lr_lb_ip6), dnl
+match=(inport == "lrp-public" && nd_ns && nd.target == \$${lb_as_v6}), dnl
action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
])
@@ -1792,7 +1795,7 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
# Ingress router port is used for ARP reply/NA in lr_in_ip_input.
# xxreg0[0..47] is used unless external_mac is set.
# Priority 90 flows (per router).
-AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
+AT_CHECK_UNQUOTED([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
@@ -1806,7 +1809,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl
+match=(inport == "lrp" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl
@@ -1815,10 +1818,10 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl
action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp" && nd_ns && nd.target == $lr_lb_ip6), dnl
+match=(inport == "lrp" && nd_ns && nd.target == \$${lb_as_v6}), dnl
action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == $lr_lb_ip4 && is_chassis_resident("cr-lrp-public")), dnl
+match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == \$${lb_as_v4} && is_chassis_resident("cr-lrp-public")), dnl
action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
table=3 (lr_in_ip_input ), priority=90 , dnl
match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl
@@ -1827,7 +1830,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */
match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
table=3 (lr_in_ip_input ), priority=90 , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == $lr_lb_ip6 && is_chassis_resident("cr-lrp-public")), dnl
+match=(inport == "lrp-public" && nd_ns && nd.target == \$${lb_as_v6} && is_chassis_resident("cr-lrp-public")), dnl
action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
])
--
2.27.0
More information about the dev
mailing list