On 12 August 2016 at 12:28, Chandra S Vejendla <csvej...@us.ibm.com> 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 <csvej...@us.ibm.com>
> Acked-by: Ryan Moats <rmo...@us.ibm.com>
>
Thanks, it took me a while to wrap my head around the design, so thanks for
the help over the IRC.
This patch actually does 2 things.
1. Adds localnet support for l3gateway
2. Adds support for GARP for NAT addresses.
You should ideally have split this into 2 patches.
For the first part (localnet support), please expand on the commit message
to make it easy to understand. That way, when someone 'git annotate's a few
months down the line and finds this commit, they understand why whats being
done.
In the larger picture, why are we doing this on a switch's port? Can you
explain the rationale behind it (in the commit message)? We could have done
the same thing as you are doing on the router's port itself, isn't it?
.......snip......
-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 861f872..61a9705 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1186,6 +1186,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)) {
>
The above creates a memory leak. I do not see a corresponding destroy_lport_
addresses()
> + static struct vlog_rate_limit rl =
> + VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
> + }
> + else {
>
The else condition needs to move up one line (CodingStyle violation).
> + smap_add(&new, "nat-addresses", nat_addresses);
> + }
> + }
> sbrec_port_binding_set_options(op->sb, &new);
> smap_destroy(&new);
> }
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 4ce295a..3fd8690 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 adddresses. This is used to send gratuitous
> ARPs for
>
s/adddresses/addresses
> + 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 13c9526..a65abbe 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1650,6 +1650,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 adddresses. This is used to send gratuitous
> ARPs for
>
s/adddresses/addresses
> + 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>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
