[ovs-dev] [PATCH v12 6/6] ovn: specify options:nat-addresses as "router"

[Mickey Spiegel]

On Fri, Jan 27, 2017 at 10:29 AM, Mickey Spiegel <mickeys.dev at gmail.com>
wrote:

> Thanks for the review.
>
> On Fri, Jan 27, 2017 at 10:20 AM, Guru Shetty <guru at ovn.org> wrote:
>
>>
>>
>> On 26 January 2017 at 01:20, Mickey Spiegel <mickeys.dev at gmail.com>
>> 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 <mickeys.dev at gmail.com>
>>> ---
>>>  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 <guru at ovn.org>
>>
>