Hi Lorenzo, Lorenzo Bianconi <[email protected]> writes:
> 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 > > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- Does it make sense for the following failure to be associated with this patch: 2697: ovn -- ensure one gw controller restart in HA doesn't bounce the master FAILED (ovn.at:9568) I'm about to enable reporting 'make distcheck' errors in the bot, and don't want it to be spamming with a known issue. Bala is currently working on factoring out upstream errors, so that will have to come later. > ovn/northd/ovn-northd.c | 76 ++++++++++++++++++++++++++++++++--------- > ovn/ovn-nb.xml | 5 +++ > tests/ovn.at | 17 +++++++++ > 3 files changed, 81 insertions(+), 17 deletions(-) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 439651f80..3e8a4a276 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -404,6 +404,9 @@ struct ipam_info { > unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */ > bool ipv6_prefix_set; > struct in6_addr ipv6_prefix; > + bool mac_prefix_set; > + struct eth_addr mac_prefix; > + > }; > > /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or > @@ -534,7 +537,8 @@ lrouter_is_enabled(const struct nbrec_logical_router > *lrouter) > } > > static void > -init_ipam_info_for_datapath(struct ovn_datapath *od) > +init_ipam_info_for_datapath(struct ovn_datapath *od, > + struct northd_context *ctx) > { > if (!od->nbs) { > return; > @@ -543,11 +547,27 @@ init_ipam_info_for_datapath(struct ovn_datapath *od) > const char *subnet_str = smap_get(&od->nbs->other_config, "subnet"); > const char *ipv6_prefix = smap_get(&od->nbs->other_config, > "ipv6_prefix"); > > + const struct nbrec_nb_global *nb = nbrec_nb_global_first(ctx->ovnnb_idl); > + const char *mac_prefix = smap_get(&nb->options, "mac_prefix"); > + > if (ipv6_prefix) { > od->ipam_info.ipv6_prefix_set = ipv6_parse( > ipv6_prefix, &od->ipam_info.ipv6_prefix); > } > > + if (mac_prefix) { > + struct eth_addr addr; > + > + memset(&addr, 0, sizeof addr); > + if (ovs_scan(mac_prefix, "%"SCNx8":%"SCNx8":%"SCNx8, > + &addr.ea[0], &addr.ea[1], &addr.ea[2])) { > + od->ipam_info.mac_prefix_set = true; > + od->ipam_info.mac_prefix = addr; > + } else { > + od->ipam_info.mac_prefix_set = false; > + } > + } > + > if (!subnet_str) { > return; > } > @@ -703,7 +723,7 @@ join_datapaths(struct northd_context *ctx, struct hmap > *datapaths, > ovs_list_push_back(nb_only, &od->list); > } > > - init_ipam_info_for_datapath(od); > + init_ipam_info_for_datapath(od, ctx); > } > > const struct nbrec_logical_router *nbr; > @@ -915,17 +935,24 @@ ipam_is_duplicate_mac(struct eth_addr *ea, uint64_t > mac64, bool warn) > } > > static void > -ipam_insert_mac(struct eth_addr *ea, bool check) > +ipam_insert_mac(struct ovn_datapath *od, struct eth_addr *ea, bool check) > { > if (!ea) { > return; > } > > uint64_t mac64 = eth_addr_to_uint64(*ea); > + uint64_t prefix; > + > + if (od->ipam_info.mac_prefix_set) { > + prefix = eth_addr_to_uint64(od->ipam_info.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; > } > @@ -970,7 +997,7 @@ ipam_insert_lsp_addresses(struct ovn_datapath *od, struct > ovn_port *op, > VLOG_WARN_RL(&rl, "Extract addresses failed."); > return; > } > - ipam_insert_mac(&laddrs.ea, true); > + ipam_insert_mac(od, &laddrs.ea, true); > > /* IP is only added to IPAM if the switch's subnet option > * is set, whereas MAC is always added to MACAM. */ > @@ -1007,7 +1034,7 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct > ovn_port *op) > VLOG_WARN_RL(&rl, "Extract addresses failed."); > return; > } > - ipam_insert_mac(&lrp_networks.ea, true); > + ipam_insert_mac(od, &lrp_networks.ea, true); > > if (!op->peer || !op->peer->nbsp || !op->peer->od || > !op->peer->od->nbs > || !smap_get(&op->peer->od->nbs->other_config, "subnet")) { > @@ -1025,7 +1052,7 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct > ovn_port *op) > } > > static uint64_t > -ipam_get_unused_mac(void) > +ipam_get_unused_mac(struct ovn_datapath *od) > { > /* Stores the suffix of the most recently ipam-allocated MAC address. */ > static uint32_t last_mac; > @@ -1036,7 +1063,12 @@ 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 (od->ipam_info.mac_prefix_set) { > + mac64 = eth_addr_to_uint64(od->ipam_info.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; > @@ -1091,7 +1123,7 @@ struct dynamic_address_update { > }; > > static enum dynamic_update_type > -dynamic_mac_changed(const char *lsp_addresses, > +dynamic_mac_changed(struct ovn_datapath *od, const char *lsp_addresses, > struct dynamic_address_update *update) > { > struct eth_addr ea; > @@ -1107,7 +1139,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 (od->ipam_info.mac_prefix_set) { > + prefix = eth_addr_to_uint64(od->ipam_info.mac_prefix); > + } else { > + prefix = MAC_ADDR_PREFIX; > + } > + > + if ((mac64 ^ prefix) >> 24) { > return DYNAMIC; > } else { > return NONE; > @@ -1195,10 +1235,11 @@ dynamic_ip6_changed(struct dynamic_address_update > *update) > * Returns true if any changes to dynamic addresses are required > */ > static bool > -dynamic_addresses_check_for_updates(const char *lsp_addrs, > +dynamic_addresses_check_for_updates(struct ovn_datapath *od, > + const char *lsp_addrs, > struct dynamic_address_update *update) > { > - update->mac = dynamic_mac_changed(lsp_addrs, update); > + update->mac = dynamic_mac_changed(od, lsp_addrs, update); > update->ipv4 = dynamic_ip4_changed(update); > update->ipv6 = dynamic_ip6_changed(update); > if (update->mac == NONE && > @@ -1215,10 +1256,11 @@ dynamic_addresses_check_for_updates(const char > *lsp_addrs, > * elsewhere later. > */ > static void > -update_unchanged_dynamic_addresses(struct dynamic_address_update *update) > +update_unchanged_dynamic_addresses(struct ovn_datapath *od, > + struct dynamic_address_update *update) > { > if (update->mac == NONE) { > - ipam_insert_mac(&update->current_addresses.ea, false); > + ipam_insert_mac(od, &update->current_addresses.ea, false); > } > if (update->ipv4 == NONE && update->current_addresses.n_ipv4_addrs) { > ipam_insert_ip(update->op->od, > @@ -1278,7 +1320,7 @@ update_dynamic_addresses(struct ovn_datapath *od, > mac = update->static_mac; > break; > case DYNAMIC: > - eth_addr_from_uint64(ipam_get_unused_mac(), &mac); > + eth_addr_from_uint64(ipam_get_unused_mac(od), &mac); > break; > } > > @@ -1390,8 +1432,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports) > extract_lsp_addresses(nbsp->dynamic_addresses, > &update->current_addresses); > any_changed = dynamic_addresses_check_for_updates( > - nbsp->addresses[j], update); > - update_unchanged_dynamic_addresses(update); > + od, nbsp->addresses[j], update); > + update_unchanged_dynamic_addresses(od, update); > if (any_changed) { > ovs_list_push_back(&updates, &update->node); > } else { > 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]) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
