diff mbox series

[ovs-dev,v3,2/3] ovn: Send GARP for the router ports with reside-on-redirect-chassis options set

Message ID 20190701074320.4322-1-nusiddiq@redhat.com
State Accepted
Headers show
Series Handle GARPs for logical router port IPs | expand

Commit Message

Numan Siddique July 1, 2019, 7:43 a.m. UTC 
From: Numan Siddique <nusiddiq@redhat.com>

With the commit [1], the routing for the provider logical switches
connected to a router is centralized on the master gateway chassis
(if the option - reside-on-redirect-chassis) is set. When the
failover happens and a standby gateway chassis becomes master,
it should send GARPs for the router port macs. Without this, the
physical switch doesn't learn the new location of the router port macs
immediately and this could result in traffic disruption.

This patch addresses this issue so that the ovn-controller which claims the
distributed gatweway router port sends out the GARPs.

ovn-controller sends the GARPs if the Port_Binding.nat_addresses column
is set. This patch makes use of this column, instead of adding a new column
even though the name - nat_addresses seems a bit misnomer. The documentation is
updated to highlight the usage of this column.

This patch doesn't handle sending the GARPs for the gateway router port IPs.
This will be handled in a separate patch.

[1] - 85706c34d53d ("ovn: Avoid tunneling for VLAN packets redirected to a gateway chassis")

Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 ovn/northd/ovn-northd.c | 31 ++++++++++++++++++++++
 tests/ovn.at            | 58 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 2 deletions(-)

Comments

Dumitru Ceara July 1, 2019, 5:52 p.m. UTC | #1 
On Mon, Jul 1, 2019 at 9:45 AM <nusiddiq@redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq@redhat.com>
>
> With the commit [1], the routing for the provider logical switches
> connected to a router is centralized on the master gateway chassis
> (if the option - reside-on-redirect-chassis) is set. When the
> failover happens and a standby gateway chassis becomes master,
> it should send GARPs for the router port macs. Without this, the
> physical switch doesn't learn the new location of the router port macs
> immediately and this could result in traffic disruption.
>
> This patch addresses this issue so that the ovn-controller which claims the
> distributed gatweway router port sends out the GARPs.
>
> ovn-controller sends the GARPs if the Port_Binding.nat_addresses column
> is set. This patch makes use of this column, instead of adding a new column
> even though the name - nat_addresses seems a bit misnomer. The documentation is
> updated to highlight the usage of this column.
>
> This patch doesn't handle sending the GARPs for the gateway router port IPs.
> This will be handled in a separate patch.
>
> [1] - 85706c34d53d ("ovn: Avoid tunneling for VLAN packets redirected to a gateway chassis")
>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>

Looks good to me,
Acked-by: Dumitru Ceara <dceara@redhat.com>

> ---
>  ovn/northd/ovn-northd.c | 31 ++++++++++++++++++++++
>  tests/ovn.at            | 58 +++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 87 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index c43adb51c..e0af234f8 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2522,7 +2522,38 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>                  free(nats[i]);
>              }
>              free(nats);
> +
> +            /* Add the router mac and IPv4 addresses to
> +             * Port_Binding.nat_addresses so that GARP is sent for these
> +             * IPs by the ovn-controller on which the distributed gateway
> +             * router port resides if:
> +             *
> +             * -  op->peer has 'reside-on-gateway-chassis' set and the
> +             *    the logical router datapath has distributed router port.
> +             *
> +             * Note: Port_Binding.nat_addresses column is also used for
> +             * sending the GARPs for the router port IPs.
> +             * */
> +            if (op->peer && op->peer->nbrp && op->peer->od->l3dgw_port &&
> +                op->peer->od->l3redirect_port &&
> +                smap_get_bool(&op->peer->nbrp->options,
> +                              "reside-on-redirect-chassis", false)) {
> +                struct ds garp_info = DS_EMPTY_INITIALIZER;
> +                ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
> +                for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
> +                     i++) {
> +                    ds_put_format(&garp_info, " %s",
> +                                  op->peer->lrp_networks.ipv4_addrs[i].addr_s);
> +                }
> +                ds_put_format(&garp_info, " is_chassis_resident(%s)",
> +                              op->peer->od->l3redirect_port->json_key);
> +
> +                sbrec_port_binding_update_nat_addresses_addvalue(
> +                    op->sb, ds_cstr(&garp_info));
> +                ds_destroy(&garp_info);
> +            }
>          }
> +
>          sbrec_port_binding_set_parent_port(op->sb, op->nbsp->parent_name);
>          sbrec_port_binding_set_tag(op->sb, op->nbsp->tag, op->nbsp->n_tag);
>          sbrec_port_binding_set_mac(op->sb, (const char **) op->nbsp->addresses,
> diff --git a/tests/ovn.at b/tests/ovn.at
> index daf85a55d..ea627e128 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9317,9 +9317,10 @@ ovn_start
>  # # (i.e 8.8.8.8 as destination ip).
>
>  # Physical network:
> -# # Three hypervisors hv[123].
> +# # Four hypervisors hv[1234].
>  # # hv1 hosts vif foo1.
>  # # hv2 is the "redirect-chassis" that hosts the distributed router gateway port.
> +# # Later to test GARPs for the router port - foo, hv2 and hv4 are added to the ha_chassis_group
>  # # hv3 hosts nexthop port vif outside1.
>  # # All other tests connect hypervisors to network n1 through br-phys for tunneling.
>  # # But in this test, hv1 won't connect to n1(and no br-phys in hv1), and
> @@ -9344,6 +9345,8 @@ ovs-vsctl \
>  start_daemon ovn-controller
>  ovs-vsctl -- add-port br-int hv1-vif1 -- \
>      set interface hv1-vif1 external-ids:iface-id=foo1 \
> +    options:tx_pcap=hv1/vif1-tx.pcap \
> +    options:rxq_pcap=hv1/vif1-rx.pcap \
>      ofport-request=1
>
>  sim_add hv2
> @@ -9363,6 +9366,12 @@ ovs-vsctl -- add-port br-int hv3-vif1 -- \
>      ofport-request=1
>  ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings="phys:br-phys"
>
> +sim_add hv4
> +as hv4
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.4
> +ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings="public:br-ex,phys:br-phys"
> +
>  # Create network n2 for vlan connectivity between hv1 and hv2
>  net_add n2
>
> @@ -9374,6 +9383,10 @@ as hv2
>  ovs-vsctl add-br br-ex
>  net_attach n2 br-ex
>
> +as hv4
> +ovs-vsctl add-br br-ex
> +net_attach n2 br-ex
> +
>  OVN_POPULATE_ARP
>
>  ovn-nbctl create Logical_Router name=R1
> @@ -9556,7 +9569,48 @@ $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/vif1-tx.pcap | grep ${foo1_ip}${
>  cat hv3-vif1.expected > expout
>  AT_CHECK([sort hv3-vif1], [0], [expout])
>
> -OVN_CLEANUP([hv1],[hv2],[hv3])
> +# Test the GARP for the router port ip - 192.168.1.1
> +ovn-nbctl --wait=sb ha-chassis-group-add hagrp1
> +
> +as hv1 reset_pcap_file hv1-vif1 hv1/vif1
> +as hv2 reset_pcap_file br-ex_n2 hv2/br-ex_n2
> +as hv4 reset_pcap_file br-ex_n2 hv4/br-ex_n2
> +
> +ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv2 30
> +ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv4 20
> +
> +hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp1`
> +ovn-nbctl remove logical_router_port alice options redirect-chassis
> +ovn-nbctl --wait=sb set logical_router_port alice ha_chassis_group=$hagrp1_uuid
> +
> +# When hv2 claims the gw router port cr-alice, it should send out
> +# GARP for 192.168.1.1 and it should be received by foo1 on hv1.
> +
> +# foo1 (on hv1) should receive GARP without VLAN tag
> +exp_garp_on_foo1="ffffffffffff00000101020308060001080006040001000001010203c0a80101000000000000c0a80101"
> +echo $exp_garp_on_foo1 > foo1.expout
> +
> +# ovn-controller on hv2 should send garp with VLAN tag
> +sent_garp="ffffffffffff0000010102038100000208060001080006040001000001010203c0a80101000000000000c0a80101"
> +echo $sent_garp > br-ex_n2.expout
> +
> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [foo1.expout])
> +OVN_CHECK_PACKETS([hv2/br-ex_n2-tx.pcap], [br-ex_n2.expout])
> +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv4/br-ex_n2-tx.pcap > empty
> +AT_CHECK([cat empty], [0], [])
> +
> +# Make hv4 master
> +as hv1 reset_pcap_file hv1-vif1 hv1/vif1
> +as hv2 reset_pcap_file br-ex_n2 hv2/br-ex_n2
> +as hv4 reset_pcap_file br-ex_n2 hv4/br-ex_n2
> +ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv4 40
> +
> +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [foo1.expout])
> +OVN_CHECK_PACKETS([hv4/br-ex_n2-tx.pcap], [br-ex_n2.expout])
> +$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-ex_n2-tx.pcap > empty
> +AT_CHECK([cat empty], [0], [])
> +
> +OVN_CLEANUP([hv1],[hv2],[hv3], [hv4])
>  AT_CLEANUP
>
>  AT_SETUP([ovn -- IPv6 ND Router Solicitation responder])
> --
> 2.21.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index c43adb51c..e0af234f8 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2522,7 +2522,38 @@  ovn_port_update_sbrec(struct northd_context *ctx,
                 free(nats[i]);
             }
             free(nats);
+
+            /* Add the router mac and IPv4 addresses to
+             * Port_Binding.nat_addresses so that GARP is sent for these
+             * IPs by the ovn-controller on which the distributed gateway
+             * router port resides if:
+             *
+             * -  op->peer has 'reside-on-gateway-chassis' set and the
+             *    the logical router datapath has distributed router port.
+             *
+             * Note: Port_Binding.nat_addresses column is also used for
+             * sending the GARPs for the router port IPs.
+             * */
+            if (op->peer && op->peer->nbrp && op->peer->od->l3dgw_port &&
+                op->peer->od->l3redirect_port &&
+                smap_get_bool(&op->peer->nbrp->options,
+                              "reside-on-redirect-chassis", false)) {
+                struct ds garp_info = DS_EMPTY_INITIALIZER;
+                ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
+                for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
+                     i++) {
+                    ds_put_format(&garp_info, " %s",
+                                  op->peer->lrp_networks.ipv4_addrs[i].addr_s);
+                }
+                ds_put_format(&garp_info, " is_chassis_resident(%s)",
+                              op->peer->od->l3redirect_port->json_key);
+
+                sbrec_port_binding_update_nat_addresses_addvalue(
+                    op->sb, ds_cstr(&garp_info));
+                ds_destroy(&garp_info);
+            }
         }
+
         sbrec_port_binding_set_parent_port(op->sb, op->nbsp->parent_name);
         sbrec_port_binding_set_tag(op->sb, op->nbsp->tag, op->nbsp->n_tag);
         sbrec_port_binding_set_mac(op->sb, (const char **) op->nbsp->addresses,
diff --git a/tests/ovn.at b/tests/ovn.at
index daf85a55d..ea627e128 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9317,9 +9317,10 @@  ovn_start
 # # (i.e 8.8.8.8 as destination ip).
 
 # Physical network:
-# # Three hypervisors hv[123].
+# # Four hypervisors hv[1234].
 # # hv1 hosts vif foo1.
 # # hv2 is the "redirect-chassis" that hosts the distributed router gateway port.
+# # Later to test GARPs for the router port - foo, hv2 and hv4 are added to the ha_chassis_group
 # # hv3 hosts nexthop port vif outside1.
 # # All other tests connect hypervisors to network n1 through br-phys for tunneling.
 # # But in this test, hv1 won't connect to n1(and no br-phys in hv1), and
@@ -9344,6 +9345,8 @@  ovs-vsctl \
 start_daemon ovn-controller
 ovs-vsctl -- add-port br-int hv1-vif1 -- \
     set interface hv1-vif1 external-ids:iface-id=foo1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
     ofport-request=1
 
 sim_add hv2
@@ -9363,6 +9366,12 @@  ovs-vsctl -- add-port br-int hv3-vif1 -- \
     ofport-request=1
 ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings="phys:br-phys"
 
+sim_add hv4
+as hv4
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.4
+ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings="public:br-ex,phys:br-phys"
+
 # Create network n2 for vlan connectivity between hv1 and hv2
 net_add n2
 
@@ -9374,6 +9383,10 @@  as hv2
 ovs-vsctl add-br br-ex
 net_attach n2 br-ex
 
+as hv4
+ovs-vsctl add-br br-ex
+net_attach n2 br-ex
+
 OVN_POPULATE_ARP
 
 ovn-nbctl create Logical_Router name=R1
@@ -9556,7 +9569,48 @@  $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/vif1-tx.pcap | grep ${foo1_ip}${
 cat hv3-vif1.expected > expout
 AT_CHECK([sort hv3-vif1], [0], [expout])
 
-OVN_CLEANUP([hv1],[hv2],[hv3])
+# Test the GARP for the router port ip - 192.168.1.1
+ovn-nbctl --wait=sb ha-chassis-group-add hagrp1
+
+as hv1 reset_pcap_file hv1-vif1 hv1/vif1
+as hv2 reset_pcap_file br-ex_n2 hv2/br-ex_n2
+as hv4 reset_pcap_file br-ex_n2 hv4/br-ex_n2
+
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv2 30
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv4 20
+
+hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp1`
+ovn-nbctl remove logical_router_port alice options redirect-chassis
+ovn-nbctl --wait=sb set logical_router_port alice ha_chassis_group=$hagrp1_uuid
+
+# When hv2 claims the gw router port cr-alice, it should send out
+# GARP for 192.168.1.1 and it should be received by foo1 on hv1.
+
+# foo1 (on hv1) should receive GARP without VLAN tag
+exp_garp_on_foo1="ffffffffffff00000101020308060001080006040001000001010203c0a80101000000000000c0a80101"
+echo $exp_garp_on_foo1 > foo1.expout
+
+# ovn-controller on hv2 should send garp with VLAN tag
+sent_garp="ffffffffffff0000010102038100000208060001080006040001000001010203c0a80101000000000000c0a80101"
+echo $sent_garp > br-ex_n2.expout
+
+OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [foo1.expout])
+OVN_CHECK_PACKETS([hv2/br-ex_n2-tx.pcap], [br-ex_n2.expout])
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv4/br-ex_n2-tx.pcap > empty
+AT_CHECK([cat empty], [0], [])
+
+# Make hv4 master
+as hv1 reset_pcap_file hv1-vif1 hv1/vif1
+as hv2 reset_pcap_file br-ex_n2 hv2/br-ex_n2
+as hv4 reset_pcap_file br-ex_n2 hv4/br-ex_n2
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 hv4 40
+
+OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [foo1.expout])
+OVN_CHECK_PACKETS([hv4/br-ex_n2-tx.pcap], [br-ex_n2.expout])
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-ex_n2-tx.pcap > empty
+AT_CHECK([cat empty], [0], [])
+
+OVN_CLEANUP([hv1],[hv2],[hv3], [hv4])
 AT_CLEANUP
 
 AT_SETUP([ovn -- IPv6 ND Router Solicitation responder])