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

Numan Siddique nusiddiq at redhat.com
Mon Jul 1 08:43:52 UTC 2019


On Fri, Jun 28, 2019 at 2:18 PM Dumitru Ceara <dceara at redhat.com> wrote:

> On Fri, Jun 14, 2019 at 2:38 PM <nusiddiq at redhat.com> wrote:
> >
> > From: Numan Siddique <nusiddiq at 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 at redhat.com>
>
> Hi Numan,
>
> A few comments inline.
>
> Thanks,
> Dumitru
>
> > ---
> >  ovn/northd/ovn-northd.c | 30 +++++++++++++++++++++
> >  tests/ovn.at            | 58 +++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 86 insertions(+), 2 deletions(-)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index d0a77293a..26ad3583a 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -2518,6 +2518,36 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >                  }
> >              }
> >
> > +            /* 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
> gateeway
>
> typo: s/gateeway/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);
> > +
> > +                n_nats++;
> > +                nats = xrealloc(nats, (n_nats * sizeof *nats));
> > +                nats[n_nats - 1] = ds_steal_cstr(&garp_info);
> > +            }
> > +
> >              sbrec_port_binding_set_nat_addresses(op->sb,
> >                                                   (const char **) nats,
> n_nats);
>
> Can't we avoid xrealloc above by moving the GARP for router port IP
> code after sbrec_port_binding_set_nat_addresses and call
> sbrec_port_binding_update_nat_addresses_addvalue(op->sb,
> ds_cstr(&garp_info)) if we need GARP for the router port? Then we'd
> also need to ds_destroy(&garp_info).
>
>
I never knew there were *add_value/ delvalue variants.

I have used this function in v3.

Thanks
Numan


> >              for (size_t i = 0; i < n_nats; i++) {
> > 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 at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list