Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1829059
Signed-off-by: Mark Michelson <mmich...@redhat.com>
---
northd/ovn-northd.c | 17 ++++++------
tests/ovn.at | 9 +++++++
utilities/ovn-nbctl.c | 60 +++++++++++++++++++++++++++++++------------
3 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 83e6134b0..5cd60dcd1 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -91,6 +91,7 @@ static bool controller_event_en;
* defined in Service_Monitor Southbound table. Since these packets
* all locally handled, having just one mac is good enough. */
static char svc_monitor_mac[ETH_ADDR_STRLEN + 1];
+static struct eth_addr svc_monitor_mac_ea;
/* Default probe interval for NB and SB DB connections. */
#define DEFAULT_PROBE_INTERVAL_MSEC 5000
@@ -3314,8 +3315,10 @@ ovn_lb_create(struct northd_context *ctx, struct hmap
*lbs,
ovs_assert(mon_info);
sbrec_service_monitor_set_options(
mon_info->sbrec_mon, &lb_health_check->options);
+ struct eth_addr ea;
if (!mon_info->sbrec_mon->src_mac ||
- strcmp(mon_info->sbrec_mon->src_mac, svc_monitor_mac)) {
+ !eth_addr_from_string(mon_info->sbrec_mon->src_mac, &ea) ||
+ !eth_addr_equals(ea, svc_monitor_mac_ea)) {
sbrec_service_monitor_set_src_mac(mon_info->sbrec_mon,
svc_monitor_mac);
}
@@ -11088,12 +11091,9 @@ ovnnb_db_run(struct northd_context *ctx,
const char *monitor_mac = smap_get(&nb->options, "svc_monitor_mac");
if (monitor_mac) {
- struct eth_addr addr;
-
- memset(&addr, 0, sizeof addr);
- if (eth_addr_from_string(monitor_mac, &addr)) {
+ if (eth_addr_from_string(monitor_mac, &svc_monitor_mac_ea)) {
snprintf(svc_monitor_mac, sizeof svc_monitor_mac,
- ETH_ADDR_FMT, ETH_ADDR_ARGS(addr));
+ ETH_ADDR_FMT, ETH_ADDR_ARGS(svc_monitor_mac_ea));
} else {
monitor_mac = NULL;
}
@@ -11114,10 +11114,9 @@ ovnnb_db_run(struct northd_context *ctx,
}
if (!monitor_mac) {
- struct eth_addr addr;
- eth_addr_random(&addr);
+ eth_addr_random(&svc_monitor_mac_ea);
snprintf(svc_monitor_mac, sizeof svc_monitor_mac,
- ETH_ADDR_FMT, ETH_ADDR_ARGS(addr));
+ ETH_ADDR_FMT, ETH_ADDR_ARGS(svc_monitor_mac_ea));
smap_replace(&options, "svc_monitor_mac", svc_monitor_mac);
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 52d994972..1676d8c04 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19179,3 +19179,12 @@ OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected])
OVN_CLEANUP([hv1])
AT_CLEANUP
+
+AT_SETUP([ovn -- Case-insensitive MAC])
+ovn_start
+
+ovn-nbctl lr-add r1
+ovn-nbctl lrp-add r1 rp1 CC:DD:EE:EE:DD:CC AEF0::1/64 BEF0::1/64
+
+AT_CHECK([ovn-nbctl --may-exist lrp-add r1 rp1 cc:dd:ee:ee:dd:cc aef0::1/64
bef0:0000:0000:0000::1/64])
+AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 02fc10c9e..95bf7f5fd 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4614,6 +4614,23 @@ nbctl_lrp_get_gateway_chassis(struct ctl_context *ctx)
free(gcs);
}
+static struct sset *
+lrp_network_sset(const char **networks, int n_networks)
+{
+ struct sset *network_set = xzalloc(sizeof *network_set);
+ sset_init(network_set);
+ for (int i = 0; i < n_networks; i++) {
+ char *norm = normalize_prefix_str(networks[i]);
+ if (!norm) {
+ sset_destroy(network_set);
+ free(network_set);
+ return NULL;
+ }
+ sset_add_and_free(network_set, norm);
+ }
+ return network_set;
+}
+
static void
nbctl_lrp_add(struct ctl_context *ctx)
{
@@ -4647,6 +4664,12 @@ nbctl_lrp_add(struct ctl_context *ctx)
char **settings = (char **) &ctx->argv[n_networks + 4];
int n_settings = ctx->argc - 4 - n_networks;
+ struct eth_addr ea;
+ if (!eth_addr_from_string(mac, &ea)) {
+ ctl_error(ctx, "%s: invalid mac address %s", lrp_name, mac);
+ return;
+ }
+
const struct nbrec_logical_router_port *lrp;
error = lrp_by_name_or_uuid(ctx, lrp_name, false, &lrp);
if (error) {
@@ -4673,23 +4696,34 @@ nbctl_lrp_add(struct ctl_context *ctx)
return;
}
- if (strcmp(mac, lrp->mac)) {
+ struct eth_addr lrp_ea;
+ eth_addr_from_string(lrp->mac, &lrp_ea);
+ if (!eth_addr_equals(ea, lrp_ea)) {
ctl_error(ctx, "%s: port already exists with mac %s", lrp_name,
lrp->mac);
return;
}
- struct sset new_networks = SSET_INITIALIZER(&new_networks);
- for (int i = 0; i < n_networks; i++) {
- sset_add(&new_networks, networks[i]);
+ struct sset *new_networks = lrp_network_sset(networks, n_networks);
+ if (!new_networks) {
+ ctl_error(ctx, "%s: Invalid networks configured", lrp_name);
+ return;
+ }
+ struct sset *orig_networks = lrp_network_sset((const char
**)lrp->networks,
+ lrp->n_networks);
+ if (!orig_networks) {
+ ctl_error(ctx, "%s: Existing port has invalid networks configured",
+ lrp_name);
+ sset_destroy(new_networks);
+ free(new_networks);
+ return;
}
- struct sset orig_networks = SSET_INITIALIZER(&orig_networks);
- sset_add_array(&orig_networks, lrp->networks, lrp->n_networks);
-
- bool same_networks = sset_equals(&orig_networks, &new_networks);
- sset_destroy(&orig_networks);
- sset_destroy(&new_networks);
+ bool same_networks = sset_equals(orig_networks, new_networks);
+ sset_destroy(orig_networks);
+ free(orig_networks);
+ sset_destroy(new_networks);
+ free(new_networks);
if (!same_networks) {
ctl_error(ctx, "%s: port already exists with different network",
lrp_name);
@@ -4715,12 +4749,6 @@ nbctl_lrp_add(struct ctl_context *ctx)
return;
}
- struct eth_addr ea;
- if (!eth_addr_from_string(mac, &ea)) {
- ctl_error(ctx, "%s: invalid mac address %s", lrp_name, mac);
- return;
- }
-
for (int i = 0; i < n_networks; i++) {
ovs_be32 ipv4;
unsigned int plen;