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 <[email protected]>
(cherry picked from commit beed00c9206d439c89da3bcaf924163abf606215)
---
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 c1d83548e..d237f693d 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)
{
@@ -12040,7 +12071,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);
@@ -12056,7 +12087,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);
@@ -13687,7 +13718,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,
@@ -13697,7 +13728,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 166723502..3f21edee9 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; };)
])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
