I merged this to master. Should this be backported as well?
On 9/9/21 6:31 AM, Mark Gray wrote:
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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
