From 1a4e52ff46b5ed48260d930c3a604a351ae7591c Mon Sep 17 00:00:00 2001
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Fri, 25 Feb 2022 15:24:45 +0100
Subject: [PATCH] northd: introduce exclude-lb-vips-from-garp option for lsp
The CMS can configure the same lbs on multiple gw routers so the
same VIPs will be adverised by multiple routers through GARPs.
In order to fix the issue, introduce exclude-lb-vips-from-garp
option in the logical_switch_port table in order to advertise in
GARPs packets all SNAT/DNAT configured in the connected logical
router but skip all configured load_balancers if nat-addresses
option is set to router.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2054394
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2053013
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
northd/northd.c | 50 +++++++++++++++++++++++++--------------------
ovn-nb.xml | 9 ++++++++
tests/ovn-northd.at | 32 +++++++++++++++++++++++++++++
tests/ovn.at | 38 ++++++++++++++++++++++++++++++++++
4 files changed, 107 insertions(+), 22 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index c0ecf2346..a886d5953 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1641,13 +1641,13 @@ destroy_routable_addresses(struct ovn_port_routable_addresses *ra)
}
static char **get_nat_addresses(const struct ovn_port *op, size_t *n,
- bool routable_only);
+ bool routable_only, bool include_lb_ips);
static void
assign_routable_addresses(struct ovn_port *op)
{
size_t n;
- char **nats = get_nat_addresses(op, &n, true);
+ char **nats = get_nat_addresses(op, &n, true, true);
if (!nats) {
return;
@@ -2711,7 +2711,8 @@ join_logical_ports(struct northd_input *input_data,
* 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, bool routable_only)
+get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only,
+ bool include_lb_ips)
{
size_t n_nats = 0;
struct eth_addr mac;
@@ -2791,24 +2792,26 @@ get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only)
}
}
- const char *ip_address;
- 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 (include_lb_ips) {
+ const char *ip_address;
+ 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;
+ }
}
}
@@ -3376,7 +3379,10 @@ ovn_port_update_sbrec(struct northd_input *input_data,
if (nat_addresses && !strcmp(nat_addresses, "router")) {
if (op->peer && op->peer->od
&& (chassis || op->peer->od->n_l3dgw_ports)) {
- nats = get_nat_addresses(op->peer, &n_nats, false);
+ bool exclude_lb_vips = smap_get_bool(&op->nbsp->options,
+ "exclude-lb-vips-from-garp", false);
+ nats = get_nat_addresses(op->peer, &n_nats, false,
+ !exclude_lb_vips);
}
/* Only accept manual specification of ethernet address
* followed by IPv4 addresses on type "l3gateway" ports. */
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 6a6972856..93c7488a8 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -925,6 +925,15 @@
</dl>
</column>
+ <column name="options" key="exclude-lb-vips-from-garp">
+ If <ref column="options" key="nat-addresses"/> is set to
+ <code>router</code>, Gratuitous ARPs will be sent for all
+ SNAT and DNAT external IP addresses defined on the
+ <ref column="options" key="router-port"/>'s logical router,
+ using the <ref column="options" key="router-port"/>'s MAC address,
+ not cosidering configured load balancers.
+ </column>
+
<column name="options" key="arp_proxy">
Optional. A list of IPv4 addresses that this
logical switch <code>router</code> port will reply to ARP requests.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 652903761..947fd63f7 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -5890,3 +5890,35 @@ AT_CHECK([grep -e "(lr_in_ip_routing ).*outport" lr0flows | sed 's/table=../ta
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([check exclude-lb-vips-from-garp option])
+ovn_start
+
+ovn-nbctl lr-add R1
+ovn-nbctl set logical_router R1 options:chassis=hv1
+ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24
+
+ovn-nbctl ls-add S1
+ovn-nbctl lsp-add S1 S1-R1
+ovn-nbctl lsp-set-type S1-R1 router
+ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1 nat-addresses="router"
+
+ovn-nbctl lr-nat-add R1 snat 172.16.1.1 10.0.0.0/24
+ovn-nbctl lr-nat-add R1 dnat 172.16.1.2 10.0.0.1
+# Add load balancers
+ovn-nbctl lb-add lb0 172.16.1.10:80 10.0.0.1:80
+ovn-nbctl lr-lb-add R1 lb0
+ovn-nbctl lb-add lb1 172.16.1.10:8080 10.0.0.1:8080
+ovn-nbctl lr-lb-add R1 lb1
+
+AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses |grep -q 172.16.1.10], [0])
+
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1 nat-addresses="router" \
+ exclude-lb-vips-from-garp="true"
+
+AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses |grep -q 172.16.1.10], [1])
+
+AT_CLEANUP
+])
diff --git a/tests/ovn.at b/tests/ovn.at
index 957eb7850..2e5a798af 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8651,6 +8651,44 @@ expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80003000000
echo $expected >> expout
AT_CHECK([sort packets], [0], [expout])
+# Temporarily remove nat-addresses option to avoid race conditions
+# due to GARP backoff
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses=""
+
+reset_pcap_file() {
+ local iface=$1
+ local pcap_file=$2
+ ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
+options:rxq_pcap=dummy-rx.pcap
+ rm -f ${pcap_file}*.pcap
+ ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
+options:rxq_pcap=${pcap_file}-rx.pcap
+}
+
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+
+# Re-add nat-addresses option
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" exclude-lb-vips-from-garp="true"
+
+# Wait for packets to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 250])
+trim_zeros() {
+ sed 's/\(00\)\{1,\}$//'
+}
+
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros > packets
+g0="fffffffffffff0000000000108060001080006040001f00000000001c0a80001000000000000c0a80001"
+echo $g0 > expout
+g1="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002"
+echo $g1 >> expout
+
+grep $g0 packets | head -1 > exp
+grep $g1 packets | head -1 >> exp
+AT_CHECK([cat exp], [0], [expout])
+
+g3="fffffffffffff0000000000108060001080006040001f00000000001c0a80003000000000000c0a80003"
+AT_CHECK([grep -q $g3 packets], [1])
+
OVN_CLEANUP([hv1])
AT_CLEANUP
--
2.34.1