[ovs-dev] [PATCH v6] ovn: Support for GARP for NAT IPs via localnet
Chandra S Vejendla
csvejend at us.ibm.com
Tue Aug 16 17:14:21 UTC 2016
In cases where a DNAT IP is moved to a new router or the SNAT IP is reused
with a new mac address, the NAT IPs become unreachable because the external
switches/routers have stale ARP entries. This commit
aims to fix the problem by sending GARPs for NAT IPs via locanet. There are
two parts to this patch.
[1] Adding the datapath of the l3 gateway port to local datapaths in
ovn-controller. This will result in creation of patch ports between
br-int and the physical bridge (that provides connectivity to local network
via localnet port) and will enable gateway router to have external
connectivity
[2] A new options key "nat-addresses" is added to the logical switch port of
type router, the logical switch that has this port is the one that provides
connectivity to local network via localnet port. The value for the key
"nat-addresses" is the MAC address of the port followed by a list of
SNAT & DNAT IPs. When ovn-controller sees a new IP in nat-addrress option,
it sends a GARP message for the IP via the localnet port. nat-addresses
option is added to the logical switch port of type router and not to the
logical router port, because the logical switch datapath has the localnet
port. Adding nat-addresses option to the router port will involve more
changes to get to the local net port.
Signed-off-by: Chandra Sekhar Vejendla <csvejend at us.ibm.com>
Acked-by: Ryan Moats <rmoats at us.ibm.com>
---
v5->v6:
Fixed style check error
Fixed some typos
Eloborated the commit meesage
v4->v5:
Fixed memory leak in ovn-northd.c by freeing laddrs
ovn/controller/binding.c | 9 ++-
ovn/controller/ovn-controller.8.xml | 20 +++++-
ovn/controller/patch.c | 8 ++-
ovn/controller/physical.c | 6 ++
ovn/controller/pinctrl.c | 134 +++++++++++++++++++++++++++++++-----
ovn/lib/ovn-util.c | 6 +-
ovn/lib/ovn-util.h | 2 +-
ovn/northd/ovn-northd.c | 14 ++++
ovn/ovn-nb.xml | 10 +++
ovn/ovn-sb.xml | 10 +++
tests/ovn.at | 49 +++++++++++++
11 files changed, 239 insertions(+), 29 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 3073727..bd73da8 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -232,8 +232,13 @@ consider_local_datapath(struct controller_ctx *ctx,
sset_add(all_lports, binding_rec->logical_port);
add_local_datapath(local_datapaths, binding_rec);
}
- } else if (chassis_rec && binding_rec->chassis == chassis_rec
- && strcmp(binding_rec->type, "l3gateway")) {
+ } else if (!strcmp(binding_rec->type, "l3gateway")) {
+ const char *chassis = smap_get(&binding_rec->options,
+ "l3gateway-chassis");
+ if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
+ add_local_datapath(local_datapaths, binding_rec);
+ }
+ } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
if (ctx->ovnsb_idl_txn) {
VLOG_INFO("Releasing lport %s from this chassis.",
binding_rec->logical_port);
diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml
index 345d104..559031f 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -232,7 +232,7 @@
<code>ovn-controller</code> to connect the integration bridge and
another bridge to implement a <code>l2gateway</code> logical port.
Its value is the name of the logical port with <code>type</code>
- set to <code>l3gateway</code> that the port implements. See
+ set to <code>l2gateway</code> that the port implements. See
<code>external_ids:ovn-bridge-mappings</code>, above, for more
information.
</p>
@@ -246,7 +246,23 @@
</dd>
<dt>
- <code>external_ids:ovn-logical-patch-port</code> in the
+ <code>external-ids:ovn-l3gateway-port</code> in the <code>Port</code>
+ table
+ </dt>
+
+ <dd>
+ <p>
+ This key identifies a patch port as one created by
+ <code>ovn-controller</code> to implement a <code>l3gateway</code>
+ logical port. Its value is the name of the logical port with type
+ set to <code>l3gateway</code>. This patch port is similar to
+ the OVN logical patch port, except that <code>l3gateway</code>
+ port can only be bound to a paticular chassis.
+ </p>
+ </dd>
+
+ <dt>
+ <code>external-ids:ovn-logical-patch-port</code> in the
<code>Port</code> table
</dt>
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 62d68d9..02bf450 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -345,12 +345,14 @@ add_logical_patch_ports(struct controller_ctx *ctx,
const struct sbrec_port_binding *binding;
SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
+ const char *patch_port_id = "ovn-logical-patch-port";
bool local_port = false;
if (!strcmp(binding->type, "l3gateway")) {
const char *chassis = smap_get(&binding->options,
"l3gateway-chassis");
if (chassis && !strcmp(local_chassis_id, chassis)) {
local_port = true;
+ patch_port_id = "ovn-l3gateway-port";
}
}
@@ -363,7 +365,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
char *src_name = patch_port_name(local, peer);
char *dst_name = patch_port_name(peer, local);
- create_patch_port(ctx, "ovn-logical-patch-port", local,
+ create_patch_port(ctx, patch_port_id, local,
br_int, src_name, br_int, dst_name,
existing_ports);
free(dst_name);
@@ -394,6 +396,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) {
if (smap_get(&port->external_ids, "ovn-localnet-port")
|| smap_get(&port->external_ids, "ovn-l2gateway-port")
+ || smap_get(&port->external_ids, "ovn-l3gateway-port")
|| smap_get(&port->external_ids, "ovn-logical-patch-port")) {
shash_add(&existing_ports, port->name, port);
}
@@ -402,7 +405,8 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
/* Create in the database any patch ports that should exist. Remove from
* 'existing_ports' any patch ports that do exist in the database and
* should be there. */
- add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, chassis_id);
+ add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths,
+ chassis_id);
add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports,
patched_datapaths);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0bd228c..23e3e2c 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -709,6 +709,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
"ovn-localnet-port");
const char *l2gateway = smap_get(&port_rec->external_ids,
"ovn-l2gateway-port");
+ const char *l3gateway = smap_get(&port_rec->external_ids,
+ "ovn-l3gateway-port");
const char *logpatch = smap_get(&port_rec->external_ids,
"ovn-logical-patch-port");
@@ -735,6 +737,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
/* L2 gateway patch ports can be handled just like VIFs. */
simap_put(&new_localvif_to_ofport, l2gateway, ofport);
break;
+ } else if (is_patch && l3gateway) {
+ /* L3 gateway patch ports can be handled just like VIFs. */
+ simap_put(&new_localvif_to_ofport, l3gateway, ofport);
+ break;
} else if (is_patch && logpatch) {
/* Logical patch ports can be handled just like VIFs. */
simap_put(&new_localvif_to_ofport, logpatch, ofport);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 8a759cd..8bbc42d 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1010,10 +1010,24 @@ destroy_send_garps(void)
shash_destroy_free_data(&send_garp_data);
}
+static void
+add_garp(const char *name, ofp_port_t ofport,
+ const struct eth_addr ea, ovs_be32 ip)
+{
+ 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;
+ 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, struct hmap *local_datapaths)
+ struct simap *localnet_ofports, struct hmap *local_datapaths,
+ struct shash *nat_addresses)
{
/* Find the localnet ofport to send this GARP. */
struct local_datapath *ld
@@ -1025,9 +1039,32 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
ld->localnet_port->logical_port));
- /* Update GARP if it exists. */
- struct garp_data *garp = shash_find_data(&send_garp_data,
- binding_rec->logical_port);
+ volatile struct garp_data *garp = NULL;
+ /* Update GARP for NAT IP if it exists. */
+ if (!strcmp(binding_rec->type, "l3gateway")) {
+ struct lport_addresses *laddrs = NULL;
+ laddrs = shash_find_data(nat_addresses, binding_rec->logical_port);
+ if (!laddrs) {
+ return;
+ }
+ int i;
+ for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
+ char *name = xasprintf("%s-%s", binding_rec->logical_port,
+ laddrs->ipv4_addrs[i].addr_s);
+ garp = shash_find_data(&send_garp_data, name);
+ if (garp) {
+ garp->ofport = ofport;
+ } else {
+ add_garp(name, ofport, laddrs->ea, laddrs->ipv4_addrs[i].addr);
+ }
+ free(name);
+ }
+ destroy_lport_addresses(laddrs);
+ return;
+ }
+
+ /* Update GARP for vif if it exists. */
+ garp = shash_find_data(&send_garp_data, binding_rec->logical_port);
if (garp) {
garp->ofport = ofport;
return;
@@ -1042,13 +1079,8 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
continue;
}
- struct garp_data *garp = xmalloc(sizeof *garp);
- garp->ea = laddrs.ea;
- garp->ipv4 = laddrs.ipv4_addrs[0].addr;
- garp->announce_time = time_msec() + 1000;
- garp->backoff = 1;
- garp->ofport = ofport;
- shash_add(&send_garp_data, binding_rec->logical_port, garp);
+ add_garp(binding_rec->logical_port, ofport,
+ laddrs.ea, laddrs.ipv4_addrs[0].addr);
destroy_lport_addresses(&laddrs);
break;
@@ -1107,14 +1139,15 @@ send_garp(struct garp_data *garp, long long int current_time)
return garp->announce_time;
}
-/* Get localnet vifs, and ofport for localnet patch ports. */
+/* Get localnet vifs, local l3gw ports and ofport for localnet patch ports. */
static void
-get_localnet_vifs(const struct ovsrec_bridge *br_int,
+get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
const char *this_chassis_id,
const struct lport_index *lports,
struct hmap *local_datapaths,
struct sset *localnet_vifs,
- struct simap *localnet_ofports)
+ struct simap *localnet_ofports,
+ struct sset *local_l3gw_ports)
{
for (int i = 0; i < br_int->n_ports; i++) {
const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -1128,6 +1161,8 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
}
const char *localnet = smap_get(&port_rec->external_ids,
"ovn-localnet-port");
+ const char *l3_gateway_port = smap_get(&port_rec->external_ids,
+ "ovn-l3gateway-port");
for (int j = 0; j < port_rec->n_interfaces; j++) {
const struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
if (!iface_rec->n_ofport) {
@@ -1141,6 +1176,10 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
simap_put(localnet_ofports, localnet, ofport);
continue;
}
+ if (l3_gateway_port) {
+ sset_add(local_l3gw_ports, l3_gateway_port);
+ continue;
+ }
const char *iface_id = smap_get(&iface_rec->external_ids,
"iface-id");
if (!iface_id) {
@@ -1162,6 +1201,41 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
}
static void
+get_nat_addresses_and_keys(struct sset *nat_address_keys,
+ struct sset *local_l3gw_ports,
+ const struct lport_index *lports,
+ struct shash *nat_addresses)
+{
+ const char *gw_port;
+ SSET_FOR_EACH(gw_port, local_l3gw_ports) {
+ const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
+ gw_port);
+ if (!pb) {
+ continue;
+ }
+ const char *nat_addresses_options = smap_get(&pb->options,
+ "nat-addresses");
+ if (!nat_addresses_options) {
+ continue;
+ }
+
+ struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
+ if (!extract_lsp_addresses(nat_addresses_options, laddrs)) {
+ free(laddrs);
+ continue;
+ }
+ int i;
+ for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
+ char *name = xasprintf("%s-%s", pb->logical_port,
+ laddrs->ipv4_addrs[i].addr_s);
+ sset_add(nat_address_keys, name);
+ free(name);
+ }
+ shash_add(nat_addresses, pb->logical_port, laddrs);
+ }
+}
+
+static void
send_garp_wait(void)
{
poll_timer_wait_until(send_garp_time);
@@ -1173,15 +1247,23 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
struct hmap *local_datapaths)
{
struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs);
+ struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports);
+ struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys);
struct simap localnet_ofports = SIMAP_INITIALIZER(&localnet_ofports);
+ struct shash nat_addresses;
- get_localnet_vifs(br_int, chassis_id, lports, local_datapaths,
- &localnet_vifs, &localnet_ofports);
+ shash_init(&nat_addresses);
- /* For deleted ports, remove from send_garp_data. */
+ get_localnet_vifs_l3gwports(br_int, chassis_id, lports, local_datapaths,
+ &localnet_vifs, &localnet_ofports, &local_l3gw_ports);
+
+ get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports,
+ &nat_addresses);
+ /* For deleted ports and deleted nat ips, remove from send_garp_data. */
struct shash_node *iter, *next;
SHASH_FOR_EACH_SAFE (iter, next, &send_garp_data) {
- if (!sset_contains(&localnet_vifs, iter->name)) {
+ if (!sset_contains(&localnet_vifs, iter->name) &&
+ !sset_contains(&nat_ip_keys, iter->name)) {
send_garp_delete(iter->name);
}
}
@@ -1192,7 +1274,19 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
iface_id);
if (pb) {
- send_garp_update(pb, &localnet_ofports, local_datapaths);
+ send_garp_update(pb, &localnet_ofports, local_datapaths,
+ &nat_addresses);
+ }
+ }
+
+ /* Update send_garp_data for nat-addresses. */
+ const char *gw_port;
+ SSET_FOR_EACH (gw_port, &local_l3gw_ports) {
+ const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
+ gw_port);
+ if (pb) {
+ send_garp_update(pb, &localnet_ofports, local_datapaths,
+ &nat_addresses);
}
}
@@ -1206,7 +1300,9 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
}
}
sset_destroy(&localnet_vifs);
+ sset_destroy(&local_l3gw_ports);
simap_destroy(&localnet_ofports);
+ shash_destroy_free_data(&nat_addresses);
}
static void
diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index ff0d204..94feadb 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -71,13 +71,13 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
*
* The caller must call destroy_lport_addresses(). */
bool
-extract_lsp_addresses(char *address, struct lport_addresses *laddrs)
+extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
{
memset(laddrs, 0, sizeof *laddrs);
- char *buf = address;
+ const char *buf = address;
int buf_index = 0;
- char *buf_end = buf + strlen(address);
+ const char *buf_end = buf + strlen(address);
if (!ovs_scan_len(buf, &buf_index, ETH_ADDR_SCAN_FMT,
ETH_ADDR_SCAN_ARGS(laddrs->ea))) {
laddrs->ea = eth_addr_zero;
diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index c2ac471..22bb06d 100644
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -53,7 +53,7 @@ struct lport_addresses {
};
-bool extract_lsp_addresses(char *address, struct lport_addresses *);
+bool extract_lsp_addresses(const char *address, struct lport_addresses *);
bool extract_lrp_networks(const struct nbrec_logical_router_port *,
struct lport_addresses *);
void destroy_lport_addresses(struct lport_addresses *);
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 8901f7b..30dfa6c 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1195,6 +1195,20 @@ ovn_port_update_sbrec(const struct ovn_port *op)
if (chassis) {
smap_add(&new, "l3gateway-chassis", chassis);
}
+
+ const char *nat_addresses = smap_get(&op->nbsp->options,
+ "nat-addresses");
+ if (nat_addresses) {
+ struct lport_addresses laddrs;
+ if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
+ static struct vlog_rate_limit rl =
+ VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
+ } else {
+ smap_add(&new, "nat-addresses", nat_addresses);
+ destroy_lport_addresses(&laddrs);
+ }
+ }
sbrec_port_binding_set_options(op->sb, &new);
smap_destroy(&new);
}
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 8fb5244..5719e74 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -225,6 +225,16 @@
table="Logical_Router_Port"/> to which this logical switch port is
connected.
</column>
+
+ <column name="options" key="nat-addresses">
+ MAC address of the <code>router-port</code> followed by a list of
+ SNAT and DNAT IP addresses. This is used to send gratuitous ARPs for
+ SNAT and DNAT IP addresses via <code>localnet</code> and is valid for
+ only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 158.36.44.22
+ 158.36.44.24</code>. This would result in generation of gratuitous
+ ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC
+ address of 80:fa:5b:06:72:b7.
+ </column>
</group>
<group title="Options for localnet ports">
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 1da309c..e119249 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1756,6 +1756,16 @@ tcp.flags = RST;
<column name="options" key="l3gateway-chassis">
The <code>chassis</code> in which the port resides.
</column>
+
+ <column name="options" key="nat-addresses">
+ MAC address of the <code>l3gateway</code> port followed by a list of
+ SNAT and DNAT IP addresses. This is used to send gratuitous ARPs for
+ SNAT and DNAT IP addresses via <code>localnet</code> and is valid for
+ only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 158.36.44.22
+ 158.36.44.24</code>. This would result in generation of gratuitous
+ ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC
+ address of 80:fa:5b:06:72:b7.
+ </column>
</group>
<group title="Localnet Options">
diff --git a/tests/ovn.at b/tests/ovn.at
index 6232214..216bb07 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -4916,3 +4916,52 @@ cat packets
OVN_CLEANUP([hv1])
AT_CLEANUP
+
+AT_SETUP([ovn -- send gratuitous arp for nat ips in localnet])
+AT_KEYWORDS([ovn])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+# Create logical switch
+ovn-nbctl ls-add ls0
+# Create gateway router
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+# Add router port to gateway router
+ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.1/24
+ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \
+ type=router options:router-port=lrp0-rp addresses='"f0:00:00:00:00:01"'
+# Add nat-address option
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="f0:00:00:00:00:01 192.168.0.2"
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl \
+ -- add-br br-phys \
+ -- add-br br-eth0
+
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=physnet1:br-eth0])
+AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+# Create a localnet port.
+AT_CHECK([ovn-nbctl lsp-add ls0 ln_port])
+AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown])
+AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet])
+AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1])
+
+
+# Wait for packet to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50])
+trim_zeros() {
+ sed 's/\(00\)\{1,\}$//'
+}
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros > packets
+expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002"
+echo $expected > expout
+AT_CHECK([sort packets], [0], [expout])
+cat packets
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
--
2.6.1
More information about the dev
mailing list