From patchwork Wed May 13 19:39:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Michelson X-Patchwork-Id: 1289684 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KERKHpJD; dkim-atps=neutral Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49MlPC10hVz9sRK for ; Thu, 14 May 2020 05:39:43 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9208189456; Wed, 13 May 2020 19:39:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U0Ij394Ggnaf; Wed, 13 May 2020 19:39:40 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 7ED1688287; Wed, 13 May 2020 19:39:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 645B4C0178; Wed, 13 May 2020 19:39:40 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id E2F15C016F for ; Wed, 13 May 2020 19:39:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id CE85587645 for ; Wed, 13 May 2020 19:39:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oJTgpqgn8Iuo for ; Wed, 13 May 2020 19:39:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by fraxinus.osuosl.org (Postfix) with ESMTPS id A8D20874FB for ; Wed, 13 May 2020 19:39:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589398776; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=z15rE2l0EHhnu9t+4HgkagjugmXh4uzdKOpmFQ8UssM=; b=KERKHpJDf+5bP0GcmevEEGgNvDTTOIHN3V8Qr9T/EuWI5UX8wbpslm/txgYdG+SBWWJsf5 7vKdGVCV+kf3Kc2h1tb3lu9Qd2Yvmin97fRxQW0HcEOms3aLuGNC1R9i1Wrrgqb/9pQ2rY e2HzFJvZv0EEMA5+jeuCk1VZr6QzybU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-284-f2MDMlLDNrCd1Ll9WKXv4w-1; Wed, 13 May 2020 15:39:33 -0400 X-MC-Unique: f2MDMlLDNrCd1Ll9WKXv4w-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8807A8014D7 for ; Wed, 13 May 2020 19:39:32 +0000 (UTC) Received: from monae.redhat.com (ovpn-114-12.rdu2.redhat.com [10.10.114.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA23E5C3E7 for ; Wed, 13 May 2020 19:39:31 +0000 (UTC) From: Mark Michelson To: dev@openvswitch.org Date: Wed, 13 May 2020 15:39:30 -0400 Message-Id: <20200513193930.3361135-1-mmichels@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2] Avoid case sensitive comparisons for MAC and IPv6 addresses X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" MAC addresses and IPv6 addresses use hex digits which are appropriate to express with either uppercase or lowercase letters. strcmp() detects two MAC or IPv6 addresses as different if theier capitalization differs. This fixes the issue by converting MAC strings to struct eth_addr and comparing those instead. This specifically is done when MAC addresses are provided via user-input. For cases where the MAC strings are not user-generated (e.g. pinctrl put_mac_binding) the code has not been touched. For IPv6 addresses, we use a normalized string format to compare them instead of using the raw user input or what is stored in the database. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1829059 Signed-off-by: Mark Michelson --- 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;