On 08/09/2021 13:16, Ilya Maximets wrote: > 'smap_add' doesn't check for duplicates. This leads to having > more than one entry with a 'mac_prefix' key in the 'options' smap. > > 'ovsdb_datum_sort_unique' removes duplicates before sending the > transaction. It does that by sorting values and keeping the first one > per key. In normal case 'mac_prefix' transitions from nothing to > random and it works fine. However, northd also tries to change > all-zero prefix to random one and this fails, because all-zero prefix > goes first in a sorted map and a new random value gets dropped from > the transaction. This makes northd to update macs of all ports > on every iteration with a new random mac prefix. > > Fix that by replacing smap value and not relying on IDL for removing > duplicates.
Thanks, this looks good to me and I tested it and it worked. I couldn't find this behavior (ability to assign a random mac) documented anywhere (for example in ovn-nb.5). It might be worth documenting it. Acked-by: Mark D. Gray <[email protected]> > > Fixes: 39242c106eff ("northd: refactor and split some IPAM functions") > Signed-off-by: Ilya Maximets <[email protected]> > --- > northd/ovn-northd.c | 2 +- > tests/ovn.at | 13 +++++++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index ee761cef0..eef752542 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -14212,7 +14212,7 @@ ovnnb_db_run(struct northd_context *ctx, > struct smap options; > smap_clone(&options, &nb->options); > > - smap_add(&options, "mac_prefix", mac_addr_prefix); > + smap_replace(&options, "mac_prefix", mac_addr_prefix); > > if (!monitor_mac) { > eth_addr_random(&svc_monitor_mac_ea); > diff --git a/tests/ovn.at b/tests/ovn.at > index 5104a6895..30625ec37 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -8030,6 +8030,19 @@ mac_prefix=$(ovn-nbctl --wait=sb get NB_Global . > options:mac_prefix | tr -d \") > port_addr=$(ovn-nbctl get Logical-Switch-Port p91 dynamic_addresses | tr -d > \") > AT_CHECK([test "$port_addr" = "${mac_prefix}:00:00:09"], [0], []) > > +# set mac_prefix to all-zeroes and check it is allocated in a random manner > +ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="00:00:00:00:00:00" > +ovn-nbctl ls-add sw14 > +ovn-nbctl --wait=sb set Logical-Switch sw14 other_config:mac_only=true > +ovn-nbctl --wait=sb lsp-add sw14 p141 -- lsp-set-addresses p141 dynamic > + > +mac_prefix=$(ovn-nbctl --wait=sb get NB_Global . options:mac_prefix | tr -d > \") > +port_addr=$(ovn-nbctl get Logical-Switch-Port p141 dynamic_addresses | tr -d > \") > +AT_CHECK([test "$mac_prefix" != "00:00:00:00:00:00"], [0], []) > +AT_CHECK([test "$port_addr" = "${mac_prefix}:00:00:0a"], [0], []) > +ovn-nbctl --wait=sb lsp-del sw14 p141 > +ovn-nbctl --wait=sb ls-del sw14 > + > ovn-nbctl --wait=hv set NB_Global . options:mac_prefix="00:11:22" > ovn-nbctl ls-add sw10 > ovn-nbctl --wait=sb set Logical-Switch sw10 other_config:ipv6_prefix="ae01::" > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
