On Fri, Jan 27, 2017 at 10:29 AM, Mickey Spiegel <[email protected]> wrote:
> Thanks for the review. > > On Fri, Jan 27, 2017 at 10:20 AM, Guru Shetty <[email protected]> wrote: > >> >> >> On 26 January 2017 at 01:20, Mickey Spiegel <[email protected]> >> wrote: >> >>> Currently in OVN, the "nat-addresses" in the "options" column of a >>> logical switch port of type "router" must be specified manually. >>> Typically the user would specify as "nat-addresses" all of the NAT >>> external IP addresses and load balancer IP addresses that have >>> already been specified separately on the router. >>> >>> This patch allows the logical switch port's "nat-addresses" to be >>> specified as the string "router". When ovn-northd sees this string, >>> it automatically copies the following into the southbound >>> Port_Binding's "nat-addresses" in the "options" column: >>> The options:router-port's MAC address. >>> Each NAT external IP address (of any NAT type) specified on the >>> logical router of options:router-port. >>> Each load balancer IP address specified on the logical router of >>> options:router-port. >>> This will cause the controller where the gateway router resides to >>> issue gratuitous ARPs for each NAT external IP address and for each >>> load balancer IP address specified on the gateway router. >>> >>> This patch is written as if it will be included in OVS 2.7. If it >>> is deferred to OVS 2.8, then the OVS version mentioned in ovn-nb.xml >>> will need to be updated from OVS 2.7 to OVS 2.8. >>> >>> Signed-off-by: Mickey Spiegel <[email protected]> >>> --- >>> ovn/northd/ovn-northd.c | 114 ++++++++++++++++++++++++++++++ >>> ++++++++---------- >>> ovn/ovn-nb.xml | 42 +++++++++++++++--- >>> tests/ovn.at | 60 +++++++++++++++++++++++++ >>> 3 files changed, 185 insertions(+), 31 deletions(-) >>> >>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c >>> index e24ff8f..efb8a74 100644 >>> --- a/ovn/northd/ovn-northd.c >>> +++ b/ovn/northd/ovn-northd.c >>> @@ -1436,6 +1436,86 @@ join_logical_ports(struct northd_context *ctx, >>> } >>> >>> static void >>> +ip_address_and_port_from_lb_key(const char *key, char **ip_address, >>> + uint16_t *port); >>> + >>> +static void >>> +get_router_load_balancer_ips(const struct ovn_datapath *od, >>> + struct sset *all_ips) >>> +{ >>> + if (!od->nbr) { >>> + return; >>> + } >>> + >>> + for (int i = 0; i < od->nbr->n_load_balancer; i++) { >>> + struct nbrec_load_balancer *lb = od->nbr->load_balancer[i]; >>> + struct smap *vips = &lb->vips; >>> + struct smap_node *node; >>> + >>> + SMAP_FOR_EACH (node, vips) { >>> + /* node->key contains IP:port or just IP. */ >>> + char *ip_address = NULL; >>> + uint16_t port; >>> + >>> + ip_address_and_port_from_lb_key(node->key, &ip_address, >>> &port); >>> + if (!ip_address) { >>> + continue; >>> + } >>> + >>> + if (!sset_contains(all_ips, ip_address)) { >>> + sset_add(all_ips, ip_address); >>> + } >>> + >>> + free(ip_address); >>> + } >>> + } >>> +} >>> + >>> +/* Returns a string consisting of the port's MAC address followed by the >>> + * external IP addresses of all NAT rules defined on that router and the >>> + * VIPs of all load balancers defined on that router. */ >>> >> A comment that the called has to free the returned string is useful. >> > > I can add that. > > >> >> Also, the load balancer IP address can also be the IP address of the >> router itself. For e.g: when it is ROUTER_IP:port. I guess, that is not a >> problem when a GARP is sent for the router IP too. >> > > If the router IP is used as a load balancer IP address or as a SNAT > address, then sending GARP for the router IP is a good thing. > I should clarify that statement. It is a good thing if the chassis changes, for example if doing simple high availability. The GARP packet will fix L2 learning. As I think about it, if anyone uses logical routers without NAT or load balancers, and the chassis changes, then GARP of the router IP would still be useful. I guess a user could always specify the router IP and MAC in "nat-addresses" to make that happen. I am not sure if anyone is deploying OVN that way. Mickey > Mickey > > >> >> Acked-by: Gurucharan Shetty <[email protected]> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
