[ovs-dev] [PATCH v2] OVN: introduce mac_prefix support to IPAM

[Mark Michelson]

Looks good, Lorenzo,

Just to reaaffirm my previous ack:

Acked-by: Mark Michelson <mmichels at redhat.com>

On 10/26/2018 12:20 PM, Lorenzo Bianconi wrote:
> Add the possibility to specify a given mac address prefix for
> dynamically generated mac address. Mac address prefix can be
> specified in nbdb NB_Global table, options:mac_prefix=<mac_prefix>
> This patch fix a possible issue of L2 address duplication if
> multiple OVN deployments share a single broadcast domain
> 
> Acked-by: Mark Michelson <mmichels at redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> Changes since v1:
> - use a global definition for mac_prefix
> ---
>   ovn/northd/ovn-northd.c | 37 ++++++++++++++++++++++++++++++++++---
>   ovn/ovn-nb.xml          |  5 +++++
>   tests/ovn.at            | 17 +++++++++++++++++
>   3 files changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 439651f80..816c72311 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -68,6 +68,7 @@ static const char *unixctl_path;
>   /* MAC address management (macam) table of "struct eth_addr"s, that holds the
>    * MAC addresses allocated by the OVN ipam module. */
>   static struct hmap macam = HMAP_INITIALIZER(&macam);
> +static struct eth_addr mac_prefix;
>   
>   #define MAX_OVN_TAGS 4096
>   
> @@ -922,10 +923,17 @@ ipam_insert_mac(struct eth_addr *ea, bool check)
>       }
>   
>       uint64_t mac64 = eth_addr_to_uint64(*ea);
> +    uint64_t prefix;
> +
> +    if (!eth_addr_is_zero(mac_prefix)) {
> +        prefix = eth_addr_to_uint64(mac_prefix);
> +    } else {
> +        prefix = MAC_ADDR_PREFIX;
> +    }
>       /* If the new MAC was not assigned by this address management system or
>        * check is true and the new MAC is a duplicate, do not insert it into the
>        * macam hmap. */
> -    if (((mac64 ^ MAC_ADDR_PREFIX) >> 24)
> +    if (((mac64 ^ prefix) >> 24)
>           || (check && ipam_is_duplicate_mac(ea, mac64, true))) {
>           return;
>       }
> @@ -1036,7 +1044,11 @@ ipam_get_unused_mac(void)
>       for (i = 0; i < MAC_ADDR_SPACE - 1; i++) {
>           /* The tentative MAC's suffix will be in the interval (1, 0xfffffe). */
>           mac_addr_suffix = ((last_mac + i) % (MAC_ADDR_SPACE - 1)) + 1;
> -        mac64 = MAC_ADDR_PREFIX | mac_addr_suffix;
> +        if (!eth_addr_is_zero(mac_prefix)) {
> +            mac64 =  eth_addr_to_uint64(mac_prefix) | mac_addr_suffix;
> +        } else {
> +            mac64 = MAC_ADDR_PREFIX | mac_addr_suffix;
> +        }
>           eth_addr_from_uint64(mac64, &mac);
>           if (!ipam_is_duplicate_mac(&mac, mac64, false)) {
>               last_mac = mac_addr_suffix;
> @@ -1107,7 +1119,15 @@ dynamic_mac_changed(const char *lsp_addresses,
>      }
>   
>      uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
> -   if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
> +   uint64_t prefix;
> +
> +   if (!eth_addr_is_zero(mac_prefix)) {
> +       prefix = eth_addr_to_uint64(mac_prefix);
> +   } else {
> +       prefix = MAC_ADDR_PREFIX;
> +   }
> +
> +   if ((mac64 ^ prefix) >> 24) {
>          return DYNAMIC;
>      } else {
>          return NONE;
> @@ -7141,6 +7161,17 @@ ovnnb_db_run(struct northd_context *ctx,
>       sbrec_sb_global_set_options(sb, &nb->options);
>       sb_loop->next_cfg = nb->nb_cfg;
>   
> +    const char *mac_addr_prefix = smap_get(&nb->options, "mac_prefix");
> +    if (mac_addr_prefix) {
> +        struct eth_addr addr;
> +
> +        memset(&addr, 0, sizeof addr);
> +        if (ovs_scan(mac_addr_prefix, "%"SCNx8":%"SCNx8":%"SCNx8,
> +                     &addr.ea[0], &addr.ea[1], &addr.ea[2])) {
> +            mac_prefix = addr;
> +        }
> +    }
> +
>       cleanup_macam(&macam);
>   }
>   
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index c0739fe57..f309b3b86 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -102,6 +102,11 @@
>             tunnel interfaces.
>           </column>
>         </group>
> +
> +      <column name="options" key="mac_prefix">
> +        Configure a given OUI to be used as prefix when L2 address is
> +        dynamically assigned, e.g. <code>00:11:22</code>
> +      </column>
>       </group>
>   
>       <group title="Connection Options">
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 8825beca3..e512f94aa 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5616,6 +5616,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p41 dynamic_addresses], [0],
>            ["f0:00:00:00:10:2b 192.168.1.3"
>   ])
>   
> +# define a mac address prefix
> +ovn-nbctl ls-add sw6
> +ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="00:11:22:33:44:55"
> +ovn-nbctl --wait=sb set Logical-Switch sw6 other_config:subnet=192.168.100.0/24
> +for n in $(seq 1 3); do
> +    ovn-nbctl --wait=sb lsp-add sw6 "p5$n" -- lsp-set-addresses "p5$n" dynamic
> +done
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p51 dynamic_addresses], [0],
> +    ["00:11:22:00:00:4d 192.168.100.2"
> +])
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p52 dynamic_addresses], [0],
> +    ["00:11:22:00:00:4e 192.168.100.3"
> +])
> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 dynamic_addresses], [0],
> +    ["00:11:22:00:00:4f 192.168.100.4"
> +])
> +
>   as ovn-sb
>   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>   
>