From 7bacb98d2115f2f677402b0e8f00e3a9bbea48f3 Mon Sep 17 00:00:00 2001 From: Numan Siddique Date: Oct 13 2021 13:55:01 +0000 Subject: Backported the bug fix patches required for lb groups. Backported the patches: - 855fb576af2("northd: Fix typo, from destroy_router_enternal_ips to destroy_router_external_ips.") - beed00c9206("northd: Always generate valid load balancer address set names.") Signed-off-by: Numan Siddique --- diff --git a/0001-northd-Fix-typo-from-destroy_router_enternal_ips-to-.patch b/0001-northd-Fix-typo-from-destroy_router_enternal_ips-to-.patch new file mode 100644 index 0000000..e027643 --- /dev/null +++ b/0001-northd-Fix-typo-from-destroy_router_enternal_ips-to-.patch @@ -0,0 +1,38 @@ +From 855fb576af2ef24eff9d0ad19f015efb30634310 Mon Sep 17 00:00:00 2001 +From: Dumitru Ceara +Date: Fri, 8 Oct 2021 13:27:54 +0200 +Subject: [PATCH 1/2] northd: Fix typo, from destroy_router_enternal_ips to + destroy_router_external_ips. + +Signed-off-by: Dumitru Ceara +Signed-off-by: Numan Siddique +--- + northd/northd.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/northd/northd.c b/northd/northd.c +index e42795ca0..0d813d3b0 100644 +--- a/northd/northd.c ++++ b/northd/northd.c +@@ -818,7 +818,8 @@ init_router_external_ips(struct ovn_datapath *od) + } + } + +-static void destroy_router_enternal_ips(struct ovn_datapath *od) ++static void ++destroy_router_external_ips(struct ovn_datapath *od) + { + if (!od->nbr) { + return; +@@ -951,7 +952,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) + destroy_ipam_info(&od->ipam_info); + free(od->router_ports); + destroy_nat_entries(od); +- destroy_router_enternal_ips(od); ++ destroy_router_external_ips(od); + destroy_lb_for_datapath(od); + free(od->nat_entries); + free(od->localnet_ports); +-- +2.31.1 + diff --git a/0002-northd-Always-generate-valid-load-balancer-address-s.patch b/0002-northd-Always-generate-valid-load-balancer-address-s.patch new file mode 100644 index 0000000..0eee18f --- /dev/null +++ b/0002-northd-Always-generate-valid-load-balancer-address-s.patch @@ -0,0 +1,203 @@ +From beed00c9206d439c89da3bcaf924163abf606215 Mon Sep 17 00:00:00 2001 +From: Dumitru Ceara +Date: Fri, 8 Oct 2021 13:23:25 +0200 +Subject: [PATCH 2/2] northd: Always generate valid load balancer address set + names. + +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 +Signed-off-by: Numan Siddique +--- + 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 0d813d3b0..32ab3baf3 100644 +--- a/northd/northd.c ++++ b/northd/northd.c +@@ -879,6 +879,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: $. ++ */ ++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) + { +@@ -12010,7 +12041,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); +@@ -12026,7 +12057,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); +@@ -13683,7 +13714,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, +@@ -13693,7 +13724,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.31.1 + diff --git a/ovn.spec b/ovn.spec index 02da3d2..2ee12a6 100644 --- a/ovn.spec +++ b/ovn.spec @@ -43,7 +43,7 @@ Name: ovn Summary: Open Virtual Network support URL: http://www.openvswitch.org/ Version: 21.09.0 -Release: 2%{?commit0:.%{date}git%{shortcommit0}}%{?dist} +Release: 3%{?commit0:.%{date}git%{shortcommit0}}%{?dist} Obsoletes: openvswitch-ovn-common < %{?epoch_ovs:%{epoch_ovs}:}2.11.0-8 Provides: openvswitch-ovn-common = %{?epoch:%{epoch}:}%{version}-%{release} @@ -72,6 +72,8 @@ Source: https://www.openvswitch.org/releases/ovn-%{version}.tar.gz Source10: https://openvswitch.org/releases/openvswitch-%{ovsver}.tar.gz # ovn-patches +Patch001: 0001-northd-Fix-typo-from-destroy_router_enternal_ips-to-.patch +Patch002: 0002-northd-Always-generate-valid-load-balancer-address-s.patch # OpenvSwitch backports (400-) if required. # Address crpto policy for fedora @@ -439,6 +441,11 @@ fi %{_unitdir}/ovn-controller-vtep.service %changelog +* Wed Oct 13 2021 Numan Siddique - 21.09.0-3 +- Backported the patches: +- 855fb576af2("northd: Fix typo, from destroy_router_enternal_ips to destroy_router_external_ips.") +- beed00c9206("northd: Always generate valid load balancer address set names.") + * Wed Oct 06 2021 Numan Siddique - 21.09.0-2 - Synced with OVN upstream commit 98c6c04fb7cd731e26b96b53d59fe949b9ec06bc