[ovs-dev] [PATCH] ovn-controller: Inject GARPs to logical switch pipeline to update neighbors
Daniel Alvarez
dalvarez at redhat.com
Mon Dec 3 17:54:13 UTC 2018
Prior to this patch, GARPs announcing NAT addresses or new VIFs
were sent out to localnet ofport through an output action.
This can lead to problems since local datapaths won't get those
GARPs and ovn-controller won't update MAC_Binding entries (as
upstream switch will not send back the GARP to this port hence
other logical routers won't update their neighbours).
This patch is changing the behavior so that GARPs get injected
to OVN pipeline of the external switch. This way, they'll get
broadcasted to local pipelines and also sent out to the external
network through the localnet port.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047604.html
Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
---
ovn/controller/pinctrl.c | 62 ++++++++++--------------
tests/ovn.at | 100 +++++++++++++++++++++++++++++++++++++++
2 files changed, 125 insertions(+), 37 deletions(-)
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 56539a891..3e8af41cc 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -2019,8 +2019,8 @@ struct garp_data {
ovs_be32 ipv4; /* Ipv4 address of port. */
long long int announce_time; /* Next announcement in ms. */
int backoff; /* Backoff for the next announcement. */
- ofp_port_t ofport; /* ofport used to output this GARP. */
- int tag; /* VLAN tag of this GARP packet, or -1. */
+ uint32_t dp_key; /* Datapath used to output this GARP. */
+ uint32_t port_key; /* Port to inject the GARP into. */
};
/* Contains GARPs to be sent. */
@@ -2043,37 +2043,24 @@ destroy_send_garps(void)
}
static void
-add_garp(const char *name, ofp_port_t ofport, int tag,
- const struct eth_addr ea, ovs_be32 ip)
+add_garp(const char *name, const struct eth_addr ea, ovs_be32 ip,
+ uint32_t dp_key, uint32_t port_key)
{
struct garp_data *garp = xmalloc(sizeof *garp);
garp->ea = ea;
garp->ipv4 = ip;
garp->announce_time = time_msec() + 1000;
garp->backoff = 1;
- garp->ofport = ofport;
- garp->tag = tag;
+ garp->dp_key = dp_key;
+ garp->port_key = port_key;
shash_add(&send_garp_data, name, garp);
}
/* Add or update a vif for which GARPs need to be announced. */
static void
send_garp_update(const struct sbrec_port_binding *binding_rec,
- struct simap *localnet_ofports,
- const struct hmap *local_datapaths,
struct shash *nat_addresses)
{
- /* Find the localnet ofport to send this GARP. */
- struct local_datapath *ld
- = get_local_datapath(local_datapaths,
- binding_rec->datapath->tunnel_key);
- if (!ld || !ld->localnet_port) {
- return;
- }
- ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
- ld->localnet_port->logical_port));
- int tag = ld->localnet_port->n_tag ? *ld->localnet_port->tag : -1;
-
volatile struct garp_data *garp = NULL;
/* Update GARP for NAT IP if it exists. Consider port bindings with type
* "l3gateway" for logical switch ports attached to gateway routers, and
@@ -2090,11 +2077,13 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
laddrs->ipv4_addrs[i].addr_s);
garp = shash_find_data(&send_garp_data, name);
if (garp) {
- garp->ofport = ofport;
- garp->tag = tag;
+ garp->dp_key = binding_rec->datapath->tunnel_key;
+ garp->port_key = binding_rec->tunnel_key;
} else {
- add_garp(name, ofport, tag, laddrs->ea,
- laddrs->ipv4_addrs[i].addr);
+ add_garp(name, laddrs->ea,
+ laddrs->ipv4_addrs[i].addr,
+ binding_rec->datapath->tunnel_key,
+ binding_rec->tunnel_key);
}
free(name);
}
@@ -2107,7 +2096,8 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
/* Update GARP for vif if it exists. */
garp = shash_find_data(&send_garp_data, binding_rec->logical_port);
if (garp) {
- garp->ofport = ofport;
+ garp->dp_key = binding_rec->datapath->tunnel_key;
+ garp->port_key = binding_rec->tunnel_key;
return;
}
@@ -2120,8 +2110,9 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
continue;
}
- add_garp(binding_rec->logical_port, ofport, tag,
- laddrs.ea, laddrs.ipv4_addrs[0].addr);
+ add_garp(binding_rec->logical_port,
+ laddrs.ea, laddrs.ipv4_addrs[0].addr,
+ binding_rec->datapath->tunnel_key, binding_rec->tunnel_key);
destroy_lport_addresses(&laddrs);
break;
@@ -2150,16 +2141,15 @@ send_garp(struct garp_data *garp, long long int current_time)
compose_arp(&packet, ARP_OP_REQUEST, garp->ea, eth_addr_zero,
true, garp->ipv4, garp->ipv4);
- /* Compose a GARP request packet's vlan if exist. */
- if (garp->tag >= 0) {
- eth_push_vlan(&packet, htons(ETH_TYPE_VLAN), htons(garp->tag));
- }
-
- /* Compose actions. The garp request is output on localnet ofport. */
+ /* Inject GARP request. */
uint64_t ofpacts_stub[4096 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
enum ofp_version version = rconn_get_version(swconn);
- ofpact_put_OUTPUT(&ofpacts)->port = garp->ofport;
+ put_load(garp->dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
+ put_load(garp->port_key, MFF_LOG_INPORT, 0, 32, &ofpacts);
+ struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
+ resubmit->in_port = OFPP_CONTROLLER;
+ resubmit->table_id = OFTABLE_LOG_INGRESS_PIPELINE;
struct ofputil_packet_out po = {
.packet = dp_packet_data(&packet),
@@ -2489,8 +2479,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
const struct sbrec_port_binding *pb = lport_lookup_by_name(
sbrec_port_binding_by_name, iface_id);
if (pb) {
- send_garp_update(pb, &localnet_ofports, local_datapaths,
- &nat_addresses);
+ send_garp_update(pb, &nat_addresses);
}
}
@@ -2500,8 +2489,7 @@ send_garp_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
const struct sbrec_port_binding *pb
= lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
if (pb) {
- send_garp_update(pb, &localnet_ofports, local_datapaths,
- &nat_addresses);
+ send_garp_update(pb, &nat_addresses);
}
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 2db3f675a..466bfc6ed 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11727,3 +11727,103 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
OVN_CLEANUP([hv1],[hv2])
AT_CLEANUP
+
+AT_SETUP([ovn -- neighbor update on same HV])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Logical network:
+# A public switch (pub) with a localnet port connected to two LRs (lr0 and lr1)
+# each with a distributed gateway port.
+# Two VMs: lp0 on sw0 connected to lr0
+# lp1 on sw1 connected to lr1
+#
+# This test adds a floating IP to each VM so when they are bound to the same
+# hypervisor, it checks that the GARP sent by ovn-controller causes the
+# MAC_Binding entries to be updated properly on each logical router.
+# It will also capture packets on the physical interface to make sure that the
+# GARPs have been sent out to the external network as well.
+
+# Create logical switches
+ovn-nbctl ls-add sw0
+ovn-nbctl ls-add sw1
+ovn-nbctl ls-add pub
+
+# Created localnet port on public switch
+ovn-nbctl lsp-add pub ln-pub
+ovn-nbctl lsp-set-type ln-pub localnet
+ovn-nbctl lsp-set-addresses ln-pub unknown
+ovn-nbctl lsp-set-options ln-pub network_name=phys
+
+# Create logical routers and connect them to public switch
+ovn-nbctl create Logical_Router name=lr0
+ovn-nbctl create Logical_Router name=lr1
+
+ovn-nbctl lrp-add lr0 lr0-pub f0:00:00:00:00:01 172.24.4.220/24
+ovn-nbctl lsp-add pub pub-lr0 -- set Logical_Switch_Port pub-lr0 \
+ type=router options:router-port=lr0-pub options:nat-addresses="router" addresses="router"
+ovn-nbctl lrp-add lr1 lr1-pub f0:00:00:00:01:01 172.24.4.221/24
+ovn-nbctl lsp-add pub pub-lr1 -- set Logical_Switch_Port pub-lr1 \
+ type=router options:router-port=lr1-pub options:nat-addresses="router" addresses="router"
+
+ovn-nbctl lrp-set-gateway-chassis lr0-pub hv1 10
+ovn-nbctl lrp-set-gateway-chassis lr1-pub hv1 10
+
+# Connect sw0 and sw1 to lr0 and lr1
+ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24
+ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 type=router \
+ options:router-port=lr0-sw0 addresses="router"
+ovn-nbctl lrp-add lr1 lr1-sw1 00:00:00:00:ff:02 20.0.0.254/24
+ovn-nbctl lsp-add sw1 sw1-lr1 -- set Logical_Switch_Port sw1-lr1 type=router \
+ options:router-port=lr1-sw1 addresses="router"
+
+
+# Add SNAT rules
+ovn-nbctl lr-nat-add lr0 snat 172.24.4.220 10.0.0.0/24
+ovn-nbctl lr-nat-add lr1 snat 172.24.4.221 20.0.0.0/24
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 172.24.4.1
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+
+ovs-vsctl add-port br-int vif0 -- set Interface vif0 external-ids:iface-id=lp0
+ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1
+
+ovn-nbctl lsp-add sw0 lp0
+ovn-nbctl lsp-add sw1 lp1
+ovn-nbctl lsp-set-addresses lp0 "50:54:00:00:00:01 10.0.0.10"
+ovn-nbctl lsp-set-addresses lp1 "50:54:00:00:00:02 20.0.0.10"
+
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp0` = xup])
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xup])
+
+# Create two floating IPs, one for each VIF
+ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10
+ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10
+
+# Check that the MAC_Binding entries have been properly created
+OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr0-pub" ip="172.24.4.200" | wc -l` -gt 0])
+OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding logical_port="lr1-pub" ip="172.24.4.100" | wc -l` -gt 0])
+
+# Check that the GARPs went also to the external physical network
+# Wait until at least 4 packets have arrived and copy them to a separate file as
+# more GARPs are expected in the capture in order to avoid race conditions.
+OVS_WAIT_UNTIL([test `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | wc -l` -gt 4])
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/br-phys-tx.pcap | head -n4 > hv1/br-phys-tx4.pcap
+
+# GARP for lp0 172.24.4.100 on lr0-pub MAC (f0:00:00:00:00:01)
+echo "fffffffffffff0000000000108060001080006040001f00000000001ac180464000000000000ac180464" > expout
+# GARP for 172.24.4.220 on lr0-pub (f0:00:00:00:00:01)
+echo "fffffffffffff0000000000108060001080006040001f00000000001ac1804dc000000000000ac1804dc" >> expout
+# GARP for lp1 172.24.4.200 on lr1-pub MAC (f0:00:00:00:01:01)
+echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804c8000000000000ac1804c8" >> expout
+# GARP for 172.24.4.221 on lr1-pub (f0:00:00:00:01:01)
+echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd000000000000ac1804dd" >> expout
+AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout])
+#OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
--
More information about the dev
mailing list