On 6/26/20 4:38 AM, Numan Siddique wrote:


On Thu, Jun 25, 2020 at 12:51 AM Mark Michelson <mmich...@redhat.com <mailto:mmich...@redhat.com>> wrote:

    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 <mmich...@redhat.com
    <mailto:mmich...@redhat.com>>


Acked-by: Numan Siddique <num...@ovn.org <mailto:num...@ovn.org>> for all whole series.

I think you can add the below tag in the commit message of thi patch.

Fixes: ba0d6eae960d("ovn-northd: Add support for Load Balancer health check")

Thanks, Numan. I added that tag to patch 1 of the series. I pushed this to master and branch-20.06.


Thanks
Numan

    ---
      northd/ovn-northd.c   | 17 ++++++++---------
      tests/ovn.at <http://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 <http://ovn.at> b/tests/ovn.at <http://ovn.at>
    index 15b40ca1e..9181b8425 100644
    --- a/tests/ovn.at <http://ovn.at>
    +++ b/tests/ovn.at <http://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

    _______________________________________________
    dev mailing list
    d...@openvswitch.org <mailto:d...@openvswitch.org>
    https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to