[ovs-dev] [PATCH v4] ovn: Allow for automatic dynamic updates of IPAM

Mark Michelson mmichels at redhat.com
Fri Jul 20 12:43:28 UTC 2018


On 07/20/2018 08:06 AM, Jakub Sitnicki wrote:
> On Fri, Jul 13, 2018 at 06:45 PM GMT, Mark Michelson wrote:
>> OVN offers a method of IP address management that allows for an IPv4 subnet or
>> IPv6 prefix to be specified on a logical switch. Then by specifying a
>> switch port's address as "dynamic" or "<mac address> dynamic", OVN will
>> automatically assign addresses to the switch port.
>>
>> While this works great for initial assignment of addresses, addresses do
>> not automatically adjust when changes are made to the switch's
>> configuration. For instance:
>> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
>> changes, the affected switch ports are not updated.
>> * If a switch port with a static IP address is added to the switch, and
>> that address conflicts with a dynamically assigned IP address, the
>> dynamic address is not updated.
>> * If a MAC address switched from being statically assigned to
>> dynamically assigned, the MAC address would not be updated.
>> * If a statically assigned MAC address changed, then the IPv6 address
>> would not be updated.
>>
>> This patch solves all of the above issues by changing the algorithm for
>> IPAM assignment. There are essentially three steps.
>> 1) While joining logical ports, all statically-assigned addresses (i.e.
>> any ports without "dynamic" addresses) have their addresses registered
>> to IPAM. This gives them top priority.
>> 2) All logical ports with dynamic addresses are inspected. Any changes
>> that must be made to the addresses are collected to be made later. Any
>> addresses that do not require change are registered to IPAM. This allows
>> for previously assigned dynamic addresses to be kept.
>> 3) All gathered changes are enacted.
>>
>> The change contains new tests that ensure that dynamic addresses are
>> updated when appropriate.
>>
>> This patch also alters some existing IPAM tests. Those tests assumed
>> that dynamic addresses would not be updated automatically, so those
>> tests either had to be altered or removed.
>>
>> Signed-off-by: Mark Michelson <mmichels at redhat.com>
> 
> I've come up with a couple suggestions (see in-line) but otherwise LGTM:
> 
> Acked-by: Jakub Sitnicki <jkbs at redhat.com>

Thanks for the review Jakub! These all look like good suggestions, so 
I'll put up a new version with those suggestions.

> 
>> ---
>> v3->v4:
>>   Print warning when multiple dynamic addresses are configured on a
>>   switch port. Ensure that dynamic addresses beyond the first on a switch
>>   port are ignored. Found by Ben Pfaff.
>>
>> v2->v3:
>>   Fixed a checkpatch problem (line too long)
>>
>> v1->v2:
>>   Rebased
>> ---
>>   ovn/northd/ovn-northd.c | 385 ++++++++++++++++++++++++++++++++++--------------
>>   tests/ovn.at            |  97 +++++++++---
>>   2 files changed, 350 insertions(+), 132 deletions(-)
>>
>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>> index 04a072ba8..b75febb44 100644
>> --- a/ovn/northd/ovn-northd.c
>> +++ b/ovn/northd/ovn-northd.c
>> @@ -985,9 +985,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct ovn_port *op)
>>           for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
>>               ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
>>           }
>> -        if (op->nbsp->dynamic_addresses) {
>> -            ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
>> -        }
>>       } else if (op->nbrp) {
>>           struct lport_addresses lrp_networks;
>>           if (!extract_lrp_networks(op->nbrp, &lrp_networks)) {
>> @@ -1060,64 +1057,255 @@ ipam_get_unused_ip(struct ovn_datapath *od)
>>       return od->ipam_info.start_ipv4 + new_ip_index;
>>   }
>>
>> +enum dynamic_update_type {
>> +    NONE,    /* No change to the address */
>> +    REMOVE,  /* Address is no longer dynamic */
>> +    STATIC,  /* Use static address (MAC only) */
>> +    DYNAMIC, /* Assign a new dynamic address */
>> +};
>> +
>> +struct dynamic_address_update {
>> +    struct ovs_list node;       /* In build_ipam()'s list of updates. */
>> +
>> +    struct ovn_port *op;
>> +
>> +    struct lport_addresses current_addresses;
>> +    struct eth_addr static_mac;
>> +    enum dynamic_update_type mac;
>> +    enum dynamic_update_type ipv4;
>> +    enum dynamic_update_type ipv6;
>> +};
>> +
>> +static enum dynamic_update_type
>> +dynamic_mac_changed(const char *lsp_addresses,
>> +                    struct dynamic_address_update *update)
>> +{
>> +   struct eth_addr ea;
>> +
>> +   if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
>> +       if (eth_addr_equals(ea, update->current_addresses.ea)) {
>> +           return NONE;
>> +       } else {
>> +           /* MAC is still static, but it has changed */
>> +           update->static_mac = ea;
>> +           return STATIC;
>> +       }
>> +   }
>> +
>> +   uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
>> +   if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
>> +       return DYNAMIC;
>> +   } else {
>> +       return NONE;
>> +   }
>> +}
>> +
>> +static enum dynamic_update_type
>> +dynamic_ip4_changed(struct dynamic_address_update *update)
>> +{
>> +    bool dynamic_ip4 = update->op->od->ipam_info.allocated_ipv4s != NULL;
>> +
>> +    if (!dynamic_ip4) {
>> +        if (update->current_addresses.n_ipv4_addrs) {
>> +            return REMOVE;
>> +        } else {
>> +            return NONE;
>> +        }
>> +    }
>> +
>> +    if (!update->current_addresses.n_ipv4_addrs) {
>> +        /* IPv4 was previously static but now is dynamic */
>> +        return DYNAMIC;
>> +    }
>> +
>> +    uint32_t ip4 = ntohl(update->current_addresses.ipv4_addrs[0].addr);
>> +    if (ip4 < update->op->od->ipam_info.start_ipv4) {
>> +        return DYNAMIC;
>> +    }
>> +
>> +    uint32_t index = ip4 - update->op->od->ipam_info.start_ipv4;
>> +    if (index > update->op->od->ipam_info.total_ipv4s ||
>> +        bitmap_is_set(update->op->od->ipam_info.allocated_ipv4s, index)) {
>> +        /* Previously assigned dynamic IPv4 address can no longer be used.
>> +         * It's either outside the subnet, conflicts with an excluded IP,
>> +         * or conflicts with a statically-assigned address on the switch
>> +         */
>> +        return DYNAMIC;
>> +    } else {
>> +        return NONE;
>> +    }
>> +}
> 
> Looks like you could improve readability a bit with a local pointer to
> update->op->od->ipam_info, and maybe even also another one to
> update->current_addresses.
> 
>> +
>> +static enum dynamic_update_type
>> +dynamic_ip6_changed(struct dynamic_address_update *update)
>> +{
>> +    bool dynamic_ip6 = update->op->od->ipam_info.ipv6_prefix_set;
>> +
>> +    if (!dynamic_ip6) {
>> +        if (update->current_addresses.n_ipv6_addrs) {
>> +            /* IPv6 was dynamic but now is not */
>> +            return REMOVE;
>> +        } else {
>> +            /* IPv6 has never been dynamic */
>> +            return NONE;
>> +        }
>> +    }
>> +
>> +    if (update->mac != NONE) {
>> +        /* IPv6 address is based on MAC, so if MAC has been updated,
>> +         * then we have to update IPv6 address too.
>> +         */
>> +        return DYNAMIC;
>> +    }
>> +
>> +    if (!update->current_addresses.n_ipv6_addrs) {
>> +        /* IPv6 was previously static but now is dynamic */
>> +        return DYNAMIC;
>> +    }
>> +
>> +    struct in6_addr masked = ipv6_addr_bitand(
>> +        &update->current_addresses.ipv6_addrs[0].addr,
>> +        &update->op->od->ipam_info.ipv6_prefix);
>> +    if (!IN6_ARE_ADDR_EQUAL(&masked, &update->op->od->ipam_info.ipv6_prefix)) {
>> +        return DYNAMIC;
>> +    }
>> +
>> +    return NONE;
>> +}
>> +
>> +/* Check previously assigned dynamic addresses for validity. This will
>> + * check if the assigned addresses need to change. For addresses that
>> + * do not need to be updated, go ahead and insert them into IPAM. This
>> + * way, their addresses will be claimed and cannot be assigned elsewhere
>> + * later.
>> + *
>> + * Returns true if any changes to dynamic addresses are required
>> + */
>>   static bool
>> -ipam_allocate_addresses(struct ovn_datapath *od, struct ovn_port *op,
>> -                        const char *addrspec)
>> +dynamic_addresses_check_for_updates(const char *lsp_addrs,
>> +                                    struct dynamic_address_update *update)
>>   {
>> -    if (!op->nbsp) {
>> +    update->mac = dynamic_mac_changed(lsp_addrs, update);
>> +    update->ipv4 = dynamic_ip4_changed(update);
>> +    update->ipv6 = dynamic_ip6_changed(update);
>> +    if (update->mac == NONE) {
>> +        ipam_insert_mac(&update->current_addresses.ea, false);
>> +    }
>> +    if (update->ipv4 == NONE && update->current_addresses.n_ipv4_addrs) {
>> +        ipam_insert_ip(update->op->od,
>> +                       ntohl(update->current_addresses.ipv4_addrs[0].addr));
>> +    }
>> +    if (update->mac == NONE &&
>> +        update->ipv4 == NONE &&
>> +        update->ipv6 == NONE) {
>>           return false;
>> +    } else {
>> +        return true;
>>       }
>> +}
> 
> The side effect of reserving unchanged addresses might be surprising.
> 
> Have you considered splitting it into two steps:
> 
>      any_changed = dynamic_addresses_check_for_updates(addresses, update);
>      ipam_insert_unchanged_addresses(update);
>      if (any_changed) {
>          /* Queue update */
>      } else {
>          /* No changes */
>      }
> 
>> +
>> +static void
>> +set_lsp_dynamic_addresses(const char *dynamic_addresses, struct ovn_port *op)
>> +{
>> +    extract_lsp_addresses(dynamic_addresses, &op->lsp_addrs[op->n_lsp_addrs]);
>> +    op->n_lsp_addrs++;
>> +}
>>
>> -    /* Get or generate MAC address. */
>> +/* Determines which components (MAC, IPv4, and IPv6) of dynamic
>> + * addresses need to be assigned. This is used exclusively for
>> + * ports that do not have dynamic addresses already assigned.
>> + */
>> +static void
>> +set_dynamic_updates(const char *addrspec,
>> +                    struct dynamic_address_update *update)
>> +{
>>       struct eth_addr mac;
>> -    bool dynamic_mac;
>>       int n = 0;
>>       if (ovs_scan(addrspec, ETH_ADDR_SCAN_FMT" dynamic%n",
>>                    ETH_ADDR_SCAN_ARGS(mac), &n)
>>           && addrspec[n] == '\0') {
>> -        dynamic_mac = false;
>> +        update->mac = STATIC;
>> +        update->static_mac = mac;
>>       } else {
>> -        uint64_t mac64 = ipam_get_unused_mac();
>> -        if (!mac64) {
>> -            return false;
>> -        }
>> -        eth_addr_from_uint64(mac64, &mac);
>> -        dynamic_mac = true;
>> +        update->mac = DYNAMIC;
>>       }
>> -
>> -    /* Generate IPv4 address, if desirable. */
>> -    bool dynamic_ip4 = od->ipam_info.allocated_ipv4s != NULL;
>> -    uint32_t ip4 = dynamic_ip4 ? ipam_get_unused_ip(od) : 0;
>> -
>> -    /* Generate IPv6 address, if desirable. */
>> -    bool dynamic_ip6 = od->ipam_info.ipv6_prefix_set;
>> -    struct in6_addr ip6;
>> -    if (dynamic_ip6) {
>> -        in6_generate_eui64(mac, &od->ipam_info.ipv6_prefix, &ip6);
>> +    if (update->op->od->ipam_info.allocated_ipv4s) {
>> +        update->ipv4 = DYNAMIC;
>> +    } else {
>> +        update->ipv4 = NONE;
>> +    }
>> +    if (update->op->od->ipam_info.ipv6_prefix_set) {
>> +        update->ipv6 = DYNAMIC;
>> +    } else {
>> +        update->ipv6 = NONE;
>>       }
>> +}
>>
>> -    /* If we didn't generate anything, bail out. */
>> -    if (!dynamic_ip4 && !dynamic_ip6) {
>> -        return false;
>> +static void
>> +update_dynamic_addresses(struct ovn_datapath *od,
>> +                         struct dynamic_address_update *update)
>> +{
>> +    struct eth_addr mac;
>> +    switch (update->mac) {
>> +    case NONE:
>> +        mac = update->current_addresses.ea;
>> +        break;
>> +    case REMOVE:
>> +        ovs_assert(0);
> 
> I think OVS_NOT_REACHED() is what we're using for impossible branches.
> Can't find any ovs_assert(0) in the codebase.
> 
>> +    case STATIC:
>> +        mac = update->static_mac;
>> +        break;
>> +    case DYNAMIC:
>> +        eth_addr_from_uint64(ipam_get_unused_mac(), &mac);
>> +        break;
>> +    }
>> +
>> +    ovs_be32 ip4 = 0;
>> +    switch (update->ipv4) {
>> +    case NONE:
>> +        if (update->current_addresses.n_ipv4_addrs) {
>> +            ip4 = update->current_addresses.ipv4_addrs[0].addr;
>> +        }
>> +        break;
>> +    case REMOVE:
>> +        break;
>> +    case STATIC:
>> +        ovs_assert(0);
>> +    case DYNAMIC:
>> +        ip4 = htonl(ipam_get_unused_ip(od));
>> +    }
>> +
>> +    struct in6_addr ip6 = in6addr_any;
>> +    switch (update->ipv6) {
>> +    case NONE:
>> +        if (update->current_addresses.n_ipv6_addrs) {
>> +            ip6 = update->current_addresses.ipv6_addrs[0].addr;
>> +        }
>> +        break;
>> +    case REMOVE:
>> +        break;
>> +    case STATIC:
>> +        ovs_assert(0);
>> +    case DYNAMIC:
>> +        in6_generate_eui64(mac, &od->ipam_info.ipv6_prefix, &ip6);
>> +        break;
>>       }
>>
>> -    /* Save the dynamic addresses. */
>>       struct ds new_addr = DS_EMPTY_INITIALIZER;
>>       ds_put_format(&new_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
>> -    if (dynamic_ip4 && ip4) {
>> -        ipam_insert_ip(od, ip4);
>> -        ds_put_format(&new_addr, " "IP_FMT, IP_ARGS(htonl(ip4)));
>> +    if (ip4) {
>> +        ipam_insert_ip(od, ntohl(ip4));
>> +        ds_put_format(&new_addr, " "IP_FMT, IP_ARGS(ip4));
>>       }
>> -    if (dynamic_ip6) {
>> +    if (!IN6_ARE_ADDR_EQUAL(&ip6, &in6addr_any)) {
>>           char ip6_s[INET6_ADDRSTRLEN + 1];
>>           ipv6_string_mapped(ip6_s, &ip6);
>>           ds_put_format(&new_addr, " %s", ip6_s);
>>       }
>> -    ipam_insert_mac(&mac, !dynamic_mac);
>> -    nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp,
>> +    nbrec_logical_switch_port_set_dynamic_addresses(update->op->nbsp,
>>                                                       ds_cstr(&new_addr));
>> +    set_lsp_dynamic_addresses(ds_cstr(&new_addr), update->op);
>>       ds_destroy(&new_addr);
>> -    return true;
>>   }
>>
>>   static void
>> @@ -1133,48 +1321,80 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
>>        * ports that have the "dynamic" keyword in their addresses column. */
>>       struct ovn_datapath *od;
>>       HMAP_FOR_EACH (od, key_node, datapaths) {
>> -        if (!od->nbs || (!od->ipam_info.allocated_ipv4s &&
>> -                         !od->ipam_info.ipv6_prefix_set)) {
>> +        if (!od->nbs) {
>>               continue;
>>           }
>>
>> -        struct ovn_port *op;
>> +        struct ovs_list updates;
>> +        ovs_list_init(&updates);
>>           for (size_t i = 0; i < od->nbs->n_ports; i++) {
>> -            const struct nbrec_logical_switch_port *nbsp =
>> -                od->nbs->ports[i];
>> +            const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
>>
>> -            if (!nbsp) {
>> +            if (!od->ipam_info.allocated_ipv4s &&
>> +                !od->ipam_info.ipv6_prefix_set) {
>> +                if (nbsp->dynamic_addresses) {
>> +                    nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
>> +                                                                    NULL);
>> +                }
>>                   continue;
>>               }
>>
>> -            op = ovn_port_find(ports, nbsp->name);
>> -            if (!op || (op->nbsp && op->peer)) {
>> +            struct ovn_port *op = ovn_port_find(ports, nbsp->name);
>> +            if (!op || op->nbsp != nbsp || op->peer) {
>>                   /* Do not allocate addresses for logical switch ports that
>>                    * have a peer. */
>>                   continue;
>>               }
>>
>> +            int num_dynamic_addresses = 0;
>>               for (size_t j = 0; j < nbsp->n_addresses; j++) {
>> -                if (is_dynamic_lsp_address(nbsp->addresses[j])
>> -                    && !nbsp->dynamic_addresses) {
>> -                    if (!ipam_allocate_addresses(od, op, nbsp->addresses[j])
>> -                        || !extract_lsp_addresses(nbsp->dynamic_addresses,
>> -                                        &op->lsp_addrs[op->n_lsp_addrs])) {
>> -                        static struct vlog_rate_limit rl
>> -                            = VLOG_RATE_LIMIT_INIT(1, 1);
>> -                        VLOG_INFO_RL(&rl, "Failed to allocate address.");
>> +                if (!is_dynamic_lsp_address(nbsp->addresses[j])) {
>> +                    continue;
>> +                }
>> +                if (num_dynamic_addresses) {
>> +                    static struct vlog_rate_limit rl
>> +                        = VLOG_RATE_LIMIT_INIT(1, 1);
>> +                    VLOG_WARN_RL(&rl, "More than one dynamic address "
>> +                                 "configured for logical switch port '%s'",
>> +                                 nbsp->name);
>> +                    continue;
>> +                }
>> +                num_dynamic_addresses++;
>> +                struct dynamic_address_update *update
>> +                    = xzalloc(sizeof *update);
>> +                update->op = op;
>> +                if (nbsp->dynamic_addresses) {
>> +                    extract_lsp_addresses(nbsp->dynamic_addresses,
>> +                                          &update->current_addresses);
>> +                    if (dynamic_addresses_check_for_updates(nbsp->addresses[j],
>> +                                                            update)) {
>> +                        ovs_list_push_back(&updates, &update->node);
>>                       } else {
>> -                        op->n_lsp_addrs++;
>> +                        /* No changes to dynamic addresses */
>> +                        set_lsp_dynamic_addresses(nbsp->dynamic_addresses, op);
>> +                        destroy_lport_addresses(&update->current_addresses);
>> +                        free(update);
>>                       }
>> -                    break;
>> +                } else {
>> +                    set_dynamic_updates(nbsp->addresses[j], update);
>> +                    ovs_list_push_back(&updates, &update->node);
>>                   }
>>               }
>>
>>               if (!nbsp->n_addresses && nbsp->dynamic_addresses) {
>> -                nbrec_logical_switch_port_set_dynamic_addresses(op->nbsp,
>> -                                                                NULL);
>> +                nbrec_logical_switch_port_set_dynamic_addresses(nbsp, NULL);
>>               }
>>           }
>> +
>> +        /* After retaining all unchanged dynamic addresses, now assign
>> +         * new ones.
>> +         */
>> +        struct dynamic_address_update *update;
>> +        LIST_FOR_EACH_POP (update, node, &updates) {
>> +            update_dynamic_addresses(od, update);
>> +            destroy_lport_addresses(&update->current_addresses);
>> +            free(update);
>> +        }
>>       }
>>   }
>>   
>> @@ -1277,41 +1497,6 @@ tag_alloc_create_new_tag(struct hmap *tag_alloc_table,
>>   }
>>   
>>
>> -/*
>> - * This function checks if the MAC in "address" parameter (if present) is
>> - * different from the one stored in Logical_Switch_Port.dynamic_addresses
>> - * and updates it.
>> - */
>> -static void
>> -check_and_update_mac_in_dynamic_addresses(
>> -    const char *address,
>> -    const struct nbrec_logical_switch_port *nbsp)
>> -{
>> -    if (!nbsp->dynamic_addresses) {
>> -        return;
>> -    }
>> -    int buf_index = 0;
>> -    struct eth_addr ea;
>> -    if (!ovs_scan_len(address, &buf_index,
>> -                      ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
>> -        return;
>> -    }
>> -
>> -    struct eth_addr present_ea;
>> -    buf_index = 0;
>> -    if (ovs_scan_len(nbsp->dynamic_addresses, &buf_index,
>> -                     ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(present_ea))
>> -        && !eth_addr_equals(ea, present_ea)) {
>> -        /* MAC address has changed. Update it */
>> -        char *new_addr =  xasprintf(
>> -            ETH_ADDR_FMT"%s", ETH_ADDR_ARGS(ea),
>> -            &nbsp->dynamic_addresses[buf_index]);
>> -        nbrec_logical_switch_port_set_dynamic_addresses(
>> -            nbsp, new_addr);
>> -        free(new_addr);
>> -    }
>> -}
>> -
>>   static void
>>   join_logical_ports(struct northd_context *ctx,
>>                      struct hmap *datapaths, struct hmap *ports,
>> @@ -1379,23 +1564,7 @@ join_logical_ports(struct northd_context *ctx,
>>                           continue;
>>                       }
>>                       if (is_dynamic_lsp_address(nbsp->addresses[j])) {
>> -                        if (nbsp->dynamic_addresses) {
>> -                            check_and_update_mac_in_dynamic_addresses(
>> -                                nbsp->addresses[j], nbsp);
>> -                            if (!extract_lsp_addresses(nbsp->dynamic_addresses,
>> -                                            &op->lsp_addrs[op->n_lsp_addrs])) {
>> -                                static struct vlog_rate_limit rl
>> -                                    = VLOG_RATE_LIMIT_INIT(1, 1);
>> -                                VLOG_INFO_RL(&rl, "invalid syntax '%s' in "
>> -                                                  "logical switch port "
>> -                                                  "dynamic_addresses. No "
>> -                                                  "MAC address found",
>> -                                                  op->nbsp->dynamic_addresses);
>> -                                continue;
>> -                            }
>> -                        } else {
>> -                            continue;
>> -                        }
>> +                        continue;
>>                       } else if (!extract_lsp_addresses(nbsp->addresses[j],
>>                                              &op->lsp_addrs[op->n_lsp_addrs])) {
>>                           static struct vlog_rate_limit rl
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index d1a8967dd..e80c965a3 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -5140,7 +5140,7 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p19 dynamic_addresses], [0],
>>
>>   # Change a port's address to test for multiple ip's for a single address entry
>>   # and addresses set by the user.
>> -ovn-nbctl lsp-set-addresses p0 "0a:00:00:00:00:15 192.168.1.12 192.168.1.14"
>> +ovn-nbctl lsp-set-addresses p0 "0a:00:00:00:00:15 192.168.1.2 192.168.1.12 192.168.1.14"
>>   ovn-nbctl --wait=sb lsp-add sw0 p20 -- lsp-set-addresses p20 dynamic
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p20 dynamic_addresses], [0],
>>        ["0a:00:00:00:00:16 192.168.1.13"
>> @@ -5208,7 +5208,6 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
>>   # Update the static MAC address with dynamically allocated IP and check
>>   # if the MAC address is updated in 'Logical_Switch_Port.dynamic_adddresses'
>>   ovn-nbctl --wait=sb lsp-set-addresses p31 "fe:dc:ba:98:76:55 dynamic"
>> -ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses
>>
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
>>        ["fe:dc:ba:98:76:55 192.168.1.18"
>> @@ -5216,7 +5215,7 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
>>
>>   ovn-nbctl --wait=sb lsp-set-addresses p31 "dynamic"
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p31 dynamic_addresses], [0],
>> -     ["fe:dc:ba:98:76:55 192.168.1.18"
>> +     ["0a:00:00:00:00:21 192.168.1.18"
>>   ])
>>
>>   ovn-nbctl --wait=sb lsp-set-addresses p31 "fe:dc:ba:98:76:56 dynamic"
>> @@ -5233,21 +5232,21 @@ ovn-nbctl --wait=sb lsp-add sw0 p32 -- lsp-set-addresses p32 \
>>   "dynamic"
>>   # 192.168.1.20 should be assigned as 192.168.1.19 is excluded.
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p32 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:21 192.168.1.20"
>> +     ["0a:00:00:00:00:22 192.168.1.20"
>>   ])
>>
>>   ovn-nbctl --wait=sb lsp-add sw0 p33 -- lsp-set-addresses p33 \
>>   "dynamic"
>>   # 192.168.1.22 should be assigned as 192.168.1.21 is excluded.
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p33 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:22 192.168.1.22"
>> +     ["0a:00:00:00:00:23 192.168.1.22"
>>   ])
>>
>>   ovn-nbctl --wait=sb lsp-add sw0 p34 -- lsp-set-addresses p34 \
>>   "dynamic"
>>   # 192.168.1.51 should be assigned as 192.168.1.23-192.168.1.50 is excluded.
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p34 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:23 192.168.1.51"
>> +     ["0a:00:00:00:00:24 192.168.1.51"
>>   ])
>>
>>   # Now clear the exclude_ips list. 192.168.1.19 should be assigned.
>> @@ -5255,7 +5254,7 @@ ovn-nbctl --wait=sb set Logical-switch sw0 other_config:exclude_ips="invalid"
>>   ovn-nbctl --wait=sb lsp-add sw0 p35 -- lsp-set-addresses p35 \
>>   "dynamic"
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p35 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:24 192.168.1.19"
>> +     ["0a:00:00:00:00:25 192.168.1.19"
>>   ])
>>
>>   # Set invalid data in exclude_ips list. It should be ignored.
>> @@ -5264,7 +5263,7 @@ ovn-nbctl --wait=sb lsp-add sw0 p36 -- lsp-set-addresses p36 \
>>   "dynamic"
>>   # 192.168.1.21 should be assigned as that's the next free one.
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p36 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:25 192.168.1.21"
>> +     ["0a:00:00:00:00:26 192.168.1.21"
>>   ])
>>
>>   # Clear the dynamic addresses assignment request.
>> @@ -5281,16 +5280,17 @@ ovn-nbctl --wait=sb lsp-add sw0 p37 -- lsp-set-addresses p37 \
>>   # With prefix aef0 and mac 0a:00:00:00:00:26, the dynamic IPv6 should be
>>   # - aef0::800:ff:fe00:26 (EUI64)
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p37 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:26 192.168.1.21 aef0::800:ff:fe00:26"
>> +     ["0a:00:00:00:00:27 192.168.1.21 aef0::800:ff:fe00:27"
>>   ])
>>
>>   ovn-nbctl --wait=sb ls-add sw4
>> -ovn-nbctl --wait=sb set Logical-switch sw4 other_config:ipv6_prefix="bef0::"
>> +ovn-nbctl --wait=sb set Logical-switch sw4 other_config:ipv6_prefix="bef0::" \
>> +-- set Logical-switch sw4 other_config:subnet=192.168.2.0/30
>>   ovn-nbctl --wait=sb lsp-add sw4 p38 -- lsp-set-addresses p38 \
>>   "dynamic"
>>
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p38 dynamic_addresses], [0],
>> -     ["0a:00:00:00:00:27 bef0::800:ff:fe00:27"
>> +     ["0a:00:00:00:00:28 192.168.2.2 bef0::800:ff:fe00:28"
>>   ])
>>
>>   ovn-nbctl --wait=sb lsp-add sw4 p39 -- lsp-set-addresses p39 \
>> @@ -5300,29 +5300,78 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p39 dynamic_addresses], [0],
>>        ["f0:00:00:00:10:12 bef0::f200:ff:fe00:1012"
>>   ])
>>
>> -# Clear the other_config for sw4. No dynamic ip should be assigned.
>> -ovn-nbctl --wait=sb clear Logical-switch sw4 other_config
>> +# Test the case where IPv4 addresses are exhausted and IPv6 prefix is set
>> +# p40 should not have an IPv4 address since the pool is exhausted
>>   ovn-nbctl --wait=sb lsp-add sw4 p40 -- lsp-set-addresses p40 \
>>   "dynamic"
>> -
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p40 dynamic_addresses], [0],
>> +         ["0a:00:00:00:00:29 bef0::800:ff:fe00:29"
>> +])
>> +
>> +# Test dynamic changes on switch ports.
>> +#
>> +ovn-nbctl --wait=sb ls-add sw5
>> +ovn-nbctl --wait=sb lsp-add sw5 p41 -- lsp-set-addresses p41 \
>> +"dynamic"
>> +# p41 will start with nothing
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>>            [[[]]
>>   ])
>>
>> -# Test the case where IPv4 addresses are exhausted and IPv6 prefix is set
>> -ovn-nbctl --wait=sb set Logical-switch sw4 other_config:subnet=192.168.2.0/30 \
>> --- set Logical-switch sw4 other_config:ipv6_prefix="bef0::"
>> +# Set a subnet. Now p41 should have an ipv4 address, too
>> +ovn-nbctl --wait=sb add Logical-Switch sw5 other_config subnet=192.168.1.0/24
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["0a:00:00:00:00:2a 192.168.1.2"
>> +])
>>
>> -# Now p40 should be assigned with dynamic addresses.
>> -AT_CHECK([ovn-nbctl get Logical-Switch-Port p40 dynamic_addresses], [0],
>> -         ["0a:00:00:00:00:28 192.168.2.2 bef0::800:ff:fe00:28"
>> +# Clear the other_config. The IPv4 address should be gone
>> +ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         [[[]]
>>   ])
>>
>> -ovn-nbctl --wait=sb lsp-add sw4 p41 -- lsp-set-addresses p41 \
>> -"dynamic"
>> -# p41 should not have IPv4 address (as the pool is exhausted).
>> +# Set an IPv6 prefix. Now p41 should have an IPv6 address.
>> +ovn-nbctl --wait=sb set Logical-Switch sw5 other_config:ipv6_prefix="aef0::"
>>   AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> -         ["0a:00:00:00:00:29 bef0::800:ff:fe00:29"
>> +         ["0a:00:00:00:00:2b aef0::800:ff:fe00:2b"
>> +])
>> +
>> +# Change the MAC address to a static one. The IPv6 address should update.
>> +ovn-nbctl --wait=sb lsp-set-addresses p41 "f0:00:00:00:10:2b dynamic"
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["f0:00:00:00:10:2b aef0::f200:ff:fe00:102b"
>> +])
>> +
>> +# Change the IPv6 prefix. The IPv6 address should update.
>> +ovn-nbctl --wait=sb set Logical-Switch sw5 other_config:ipv6_prefix="bef0::"
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["f0:00:00:00:10:2b bef0::f200:ff:fe00:102b"
>> +])
>> +
>> +# Clear the other_config. The IPv6 address should be gone
>> +ovn-nbctl --wait=sb clear Logical-Switch sw5 other_config
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         [[[]]
>> +])
>> +
>> +# Set the subnet again. Now p41 should get the IPv4 address again.
>> +ovn-nbctl --wait=sb add Logical-Switch sw5 other_config subnet=192.168.1.0/24
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["f0:00:00:00:10:2b 192.168.1.2"
>> +])
>> +
>> +# Add another port with a conflicting static IPv4 address. p41 should update.
>> +ovn-nbctl --wait=sb lsp-add sw5 p42 -- lsp-set-addresses p42 \
>> +"f0:00:00:00:10:2c 192.168.1.2"
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["f0:00:00:00:10:2b 192.168.1.3"
>> +])
>> +
>> +# Add an excluded IP address that conflicts with p41. p41 should update.
>> +ovn-nbctl --wait=sb add Logical-Switch sw5 other_config \
>> +exclude_ips="192.168.1.3"
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>> +         ["f0:00:00:00:10:2b 192.168.1.4"
>>   ])
>>
>>   as ovn-sb



More information about the dev mailing list