On Fri, Feb 25, 2022 at 9:25 AM Lorenzo Bianconi
<[email protected]> wrote:
>
> 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 <[email protected]>
Thanks. I applied to the main branch and also backported to branch-21.12.
Numan
> ---
> Changes since v2:
> - add exclude-lb-vips-from-garp option instead of nat-only-router one
> - add ovn-northd unit-test
>
> Changes since v1:
> - improve commit message
> - rebase on top of ovn master
> ---
> NEWS | 2 ++
> northd/northd.c | 50 +++++++++++++++++++++++++--------------------
> ovn-nb.xml | 9 ++++++++
> tests/ovn-northd.at | 32 +++++++++++++++++++++++++++++
> tests/ovn.at | 38 ++++++++++++++++++++++++++++++++++
> 5 files changed, 109 insertions(+), 22 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a309fe8df..ebc6ec353 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -12,6 +12,8 @@ Post v21.12.0
> ovn-controller-vtep will not process any DB changes.
> - When configured to log packtes matching ACLs, log the direction (logical
> pipeline) too.
> + - Introduce exclude-lb-vips-from-garp in logical_switch_port column in
> order
> + to not advertise lbs VIPs in GARPs sent by the logical router.
>
> OVN v21.12.0 - 22 Dec 2021
> --------------------------
> diff --git a/northd/northd.c b/northd/northd.c
> index 219398f96..dc7f5d122 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;
> + }
> }
> }
>
> @@ -3377,7 +3380,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 642e91845..9af1ea2eb 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -931,6 +931,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 170d015b2..106e08d76 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -5966,3 +5966,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 fa0e0a4a2..c1c06a5e4 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.35.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
