[ovs-dev] [PATCH] OVN: select a random mac_prefix if not provided
Mark Michelson
mmichels at redhat.com
Wed Feb 20 22:22:13 UTC 2019
Looks good to me. The only potential issue here is that this constitutes
a change in behavior with no configuration change. Is there a good place
to note this so that it is clear to people who upgrade? Perhaps a NEWS
entry?
On 2/20/19 10:53 AM, Lorenzo Bianconi wrote:
> Select a random IPAM mac_prefix if it has not been provided by the user.
> With this patch the admin can avoid to configure mac_prefix in order to
> avoid L2 address collisions if multiple OVN deployments share the same
> broadcast domain.
> Remove MAC_ADDR_PREFIX definitions/occurrences since now mac_prefix is
> always provided to ovn-northd
>
> Tested-by: Miguel Duarte de Mora Barroso <mdbarroso at redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> ovn/northd/ovn-northd.c | 30 +++++++++++-------------------
> tests/ovn.at | 3 +++
> 2 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 3569ea2be..2b7a3634e 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -62,7 +62,6 @@ static const char *ovnnb_db;
> static const char *ovnsb_db;
> static const char *unixctl_path;
>
> -#define MAC_ADDR_PREFIX 0x0A0000000000ULL
> #define MAC_ADDR_SPACE 0xffffff
>
> /* MAC address management (macam) table of "struct eth_addr"s, that holds the
> @@ -937,13 +936,8 @@ ipam_insert_mac(struct eth_addr *ea, bool check)
> }
>
> uint64_t mac64 = eth_addr_to_uint64(*ea);
> - uint64_t prefix;
> + uint64_t prefix = eth_addr_to_uint64(mac_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. */
> @@ -1056,11 +1050,7 @@ ipam_get_unused_mac(ovs_be32 ip)
> for (i = 0; i < MAC_ADDR_SPACE - 1; i++) {
> /* The tentative MAC's suffix will be in the interval (1, 0xfffffe). */
> mac_addr_suffix = ((base_addr + i) % (MAC_ADDR_SPACE - 1)) + 1;
> - 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;
> - }
> + mac64 = eth_addr_to_uint64(mac_prefix) | mac_addr_suffix;
> eth_addr_from_uint64(mac64, &mac);
> if (!ipam_is_duplicate_mac(&mac, mac64, true)) {
> break;
> @@ -1132,13 +1122,7 @@ dynamic_mac_changed(const char *lsp_addresses,
> }
>
> uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
> - uint64_t prefix;
> -
> - if (!eth_addr_is_zero(mac_prefix)) {
> - prefix = eth_addr_to_uint64(mac_prefix);
> - } else {
> - prefix = MAC_ADDR_PREFIX;
> - }
> + uint64_t prefix = eth_addr_to_uint64(mac_prefix);
>
> if ((mac64 ^ prefix) >> 24) {
> return DYNAMIC;
> @@ -7300,6 +7284,14 @@ ovnnb_db_run(struct northd_context *ctx,
> &addr.ea[0], &addr.ea[1], &addr.ea[2])) {
> mac_prefix = addr;
> }
> + } else {
> + eth_addr_random(&mac_prefix);
> + memset(&mac_prefix.ea[3], 0, 3);
> +
> + char *addr_prefix = xasprintf(ETH_ADDR_FMT, ETH_ADDR_ARGS(mac_prefix));
> + smap_add((struct smap *)&nb->options, "mac_prefix", addr_prefix);
> + nbrec_nb_global_set_options(nb, &nb->options);
> + free(addr_prefix)
> }
>
> cleanup_macam(&macam);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index cfdbf412c..7e69808ed 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5416,6 +5416,7 @@ ovn_start
>
> # Add a port to a switch that does not have a subnet set, then set the
> # subnet which should result in an address being allocated for the port.
> +ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="0a:00:00:00:00:00"
> ovn-nbctl ls-add sw0
> ovn-nbctl lsp-add sw0 p0 -- lsp-set-addresses p0 dynamic
> ovn-nbctl --wait=sb add Logical-Switch sw0 other_config subnet=192.168.1.0/24
> @@ -5801,6 +5802,7 @@ ovn_start
> ovn-nbctl lr-add R1
>
> # Test for a ping using dynamically allocated addresses.
> +ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="0a:00:00:00:00:00"
> ovn-nbctl ls-add foo -- add Logical_Switch foo other_config subnet=192.168.1.0/24
> ovn-nbctl ls-add alice -- add Logical_Switch alice other_config subnet=192.168.2.0/24
>
> @@ -12052,6 +12054,7 @@ AT_CLEANUP
> AT_SETUP([ovn -- ipam to non-ipam])
> ovn_start
>
> +ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="0a:00:00:00:00:00"
> ovn-nbctl ls-add sw0
> ovn-nbctl lsp-add sw0 p0 -- lsp-set-addresses p0 dynamic
> ovn-nbctl --wait=sb add Logical-Switch sw0 other_config subnet=192.168.1.0/24
>
More information about the dev
mailing list