[ovs-dev] [PATCH v3 ovn 1/6] Avoid case-sensitive MAC address comparisons.

[Mark Michelson]

Hex digits in MAC addresses can be represented as either capital or
lowercase, and they're equal. Using strcmp to compare MAC addresses has
the potential to compare MAC addresses as different when they represent
the same address.

This commit fixes the issue by converting the string representation to
the binary representation and comparing that instead.

Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1829059
Signed-off-by: Mark Michelson <mmichels at redhat.com>
---
 northd/ovn-northd.c   | 17 ++++++++---------
 tests/ovn.at          |  9 +++++++++
 utilities/ovn-nbctl.c | 16 +++++++++-------
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index eb78f317e..cf3cffd67 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
@@ -3321,8 +3322,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);
                 }
@@ -11061,12 +11064,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;
         }
@@ -11087,10 +11087,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 15b40ca1e..9181b8425 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19720,3 +19720,12 @@ AT_CHECK([test 2287 = `ovs-ofctl dump-group-stats br-int | grep -o bucket | wc -
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+AT_SETUP([ovn -- case-insensitive lrp-add])
+ovn_start
+
+ovn-nbctl lr-add r1
+ovn-nbctl lrp-add r1 rp1 CC:DD:EE:EE:DD:CC
+
+AT_CHECK([ovn-nbctl --may-exist lrp-add r1 rp1 cc:dd:ee:ee:dd:cc])
+AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 6ccc7025d..638e6f7c0 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4678,6 +4678,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) {
@@ -4704,7 +4710,9 @@ 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;
@@ -4746,12 +4754,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;
-- 
2.25.4