[ovs-dev] [PATCH v4 ovn 1/6] Avoid case-sensitive MAC address comparisons.
Mark Michelson
mmichels at redhat.com
Fri Jun 26 14:21:28 UTC 2020
On 6/26/20 4:38 AM, Numan Siddique wrote:
>
>
> On Thu, Jun 25, 2020 at 12:51 AM Mark Michelson <mmichels at redhat.com
> <mailto:mmichels at 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 <mmichels at redhat.com
> <mailto:mmichels at redhat.com>>
>
>
> Acked-by: Numan Siddique <numans at ovn.org <mailto:numans at 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
> dev at openvswitch.org <mailto:dev at openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list