On Wed, Jul 27, 2016 at 02:15:54AM -0700, Chandra S Vejendla wrote:
> 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
>
> A new options key "nat-addresses" is added to the logical switch port of
> type router. The value for the key "nat-addresses" is the MAC address of the
> port followed by a list of SNAT & DNAT IPs.
>
> Signed-off-by: Chandra Sekhar Vejendla <[email protected]>
Here are some suggested changes to fold in, to improve style and reduce
redundancy.
ovn-sb.xml should document the new option (that it copies from the NB
database).
I didn't spot any actual problems, but I don't feel like I did a good
review of this either. I'd appreciate some comments from people who
better understand how NAT and OVN interact. Justin? Guru?
Thanks,
Ben.
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index ffe7f1e..8a904a3 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -709,6 +709,19 @@ 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,
@@ -738,24 +751,12 @@ send_garp_update(const struct sbrec_port_binding
*binding_rec,
char *name = xasprintf("%s-%s", binding_rec->logical_port,
laddrs->ipv4_addrs[i].addr_s);
garp = shash_find_data(&send_garp_data, name);
- free(name);
-
if (garp) {
garp->ofport = ofport;
- continue;
- }
- else {
- struct garp_data *garp = xmalloc(sizeof *garp);
- garp->ea = laddrs->ea;
- garp->ipv4 = laddrs->ipv4_addrs[i].addr;
- garp->announce_time = time_msec() + 1000;
- garp->backoff = 1;
- garp->ofport = ofport;
- char *name = xasprintf("%s-%s", binding_rec->logical_port,
- laddrs->ipv4_addrs[i].addr_s);
- shash_add(&send_garp_data, name, garp);
- free(name);
+ } else {
+ add_garp(name, ofport, laddrs->ea, laddrs->ipv4_addrs[i].addr);
}
+ free(name);
}
destroy_lport_addresses(laddrs);
return;
@@ -777,13 +778,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;
@@ -922,8 +918,8 @@ get_nat_addresses_and_keys(struct sset *nat_address_keys,
continue;
}
- struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
- if (!extract_lsp_addresses((char *)nat_addresses_options, laddrs)) {
+ struct lport_addresses laddrs = xmalloc(sizeof *laddrs);
+ if (!extract_lsp_addresses(nat_addresses_options, laddrs)) {
free(laddrs);
continue;
}
diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index de54624..e4e3f51 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -72,13 +72,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 e9f3ec2..7056781 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/ovn-nb.xml b/ovn/ovn-nb.xml
index 77ed3f3..59c9276 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -213,12 +213,12 @@
<column name="options" key="nat-addresses">
MAC address of the <code>router-port</code> followed by a list of
- SNAT and DNAT IP adddresses. This is used to send gratutious arps
- for SNAT and DNAT IP addresses via <code>localnet</code> and is
- valid for only l3 gateway ports. An examples for value is
- "80:fa:5b:06:72:b7 158.36.44.22 158.36.44.24". This would result
- in generation of gratutious arps for IP addresses 158.36.44.22
- and 158.36.44.24 with a mac address of 80:fa:5b:06:72:b7.
+ SNAT and DNAT IP adddresses. 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>
diff --git a/tests/ovn.at b/tests/ovn.at
index 44af6a3..ec63330 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3819,7 +3819,7 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
-AT_SETUP([ovn -- send gratutious arp for nat ips in localnet])
+AT_SETUP([ovn -- send gratuitous arp for nat ips in localnet])
AT_KEYWORDS([ovn])
AT_SKIP_IF([test $HAVE_PYTHON = no])
ovn_start
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
