From patchwork Thu Jul 15 02:07:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ihar Hrachyshka X-Patchwork-Id: 1505506 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.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) 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=J3Ft3yym; dkim-atps=neutral Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GQHnY57Swz9sWk for ; Thu, 15 Jul 2021 12:07:27 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id D4D2F421DC; Thu, 15 Jul 2021 02:07:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1c-4o2V2IkPn; Thu, 15 Jul 2021 02:07:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 424FD40494; Thu, 15 Jul 2021 02:07:22 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D9644C0010; Thu, 15 Jul 2021 02:07:21 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7E4E3C000E for ; Thu, 15 Jul 2021 02:07:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 5F3C240518 for ; Thu, 15 Jul 2021 02:07:20 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ROk7KDa1AoVr for ; Thu, 15 Jul 2021 02:07:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 0A33440494 for ; Thu, 15 Jul 2021 02:07:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1626314836; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=yaTst7W9etZdvs5GgAYgTznQwGNefObjeeTATSw0lMY=; b=J3Ft3yymRIWKglZh9AdKf/y55uGhkD7HlNEv3yE9RbyLmhv8a8r8oT0wiOMM08E+x9pFVZ 7aYSOgQtp2OeiZUPxGLU/UfEa3X5p2nKfqjfps9/ShwtJhK3rfq/oW7hBS6w4/3kpgr/yD c5eGnjP68/Z40CIDO9J89/pMLjJyzRM= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-517-_GLTg2--PKiq66VimYQ1nQ-1; Wed, 14 Jul 2021 22:07:13 -0400 X-MC-Unique: _GLTg2--PKiq66VimYQ1nQ-1 Received: by mail-qk1-f199.google.com with SMTP id a6-20020a37b1060000b02903b488f9d348so2601344qkf.20 for ; Wed, 14 Jul 2021 19:07:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=yaTst7W9etZdvs5GgAYgTznQwGNefObjeeTATSw0lMY=; b=d770nD+Yx8NbdgGaNp6bEMnuyRpy9V1qNjGMCJkagSgYLfoATF3S/AbGqkBwl4OO5V tRib5Ah2Cyty7QqFL7Zydzb5KhMa4nCOX5yxjobH+8Q0uSHtfEGS5IXZ3zAfrEYXiVKc 4+1p+eHu/HIpJZS55q7qrVDQNxgl6y4YhiIBliFkE8kuiUnyNNtpAI9biKKnXvnU371x bouHiX3qpNvdFtqCWTldtcgfuqLJDVrY6yaMu/yUEm7n3n4/Ct6dBO3HLvrYbQC9fIYq naeKckWIGnmA7WJmNo09DRUR8bmZkN36jJwoZLwd9despRfuVCXPVQ0ug/DFVpOHckHp Cp7w== X-Gm-Message-State: AOAM5300jpkUpm0LvtGZXKMRe7eJf9iAkFDhTuCKCJysDkRvCM+pRJ6F czYD8hnuHWUDVGKJJKJGfCANNSdVQoqR2yl4CZHy5B1YnNnC9xsz6exZGcIwo1RPn5TbShmMrY3 oRK0b9VaaLqVSL0e+LwSnw1CV2rE2I07OEa3Dy+EBPbxCd5bOIDxf3eDWBCCPFe0z X-Received: by 2002:ac8:1389:: with SMTP id h9mr1276910qtj.173.1626314832420; Wed, 14 Jul 2021 19:07:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxEqD9Bbjhyk19MeG/LzHITWhM+yUxe+VAUq0N8hr7J+ZaYqitw2uYVVLIgXmH0aRCnSboqiw== X-Received: by 2002:ac8:1389:: with SMTP id h9mr1276868qtj.173.1626314832034; Wed, 14 Jul 2021 19:07:12 -0700 (PDT) Received: from localhost.localdomain.com (cpe-172-73-180-250.carolina.res.rr.com. [172.73.180.250]) by smtp.googlemail.com with ESMTPSA id a20sm1489336qtp.19.2021.07.14.19.07.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Jul 2021 19:07:11 -0700 (PDT) From: Ihar Hrachyshka To: dev@openvswitch.org Date: Wed, 14 Jul 2021 22:07:04 -0400 Message-Id: <20210715020704.2622538-1-ihrachys@redhat.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ihrachys@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn branch-21.06] Don't suppress localport traffic directed to external port 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" Recently, we stopped leaking localport traffic through localnet ports into fabric to avoid unnecessary flipping between chassis hosting the same localport. Despite the type name, in some scenarios localports are supposed to talk outside the hosting chassis. Specifically, in OpenStack [1] metadata service for SR-IOV ports is implemented as a localport hosted on another chassis that is exposed to the chassis owning the SR-IOV port through an "external" port. In this case, "leaking" localport traffic into fabric is desirable. This patch inserts a higher priority flow per external port on the same datapath that avoids dropping localport traffic. This backport returns false from binding_handle_port_binding_changes on external port delete to enforce physical flow recalculation. This fixes the test case. Fixes: 96959e56d634 ("physical: do not forward traffic from localport to a localnet one") [1] https://docs.openstack.org/neutron/latest/admin/ovn/sriov.html Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1974062 Signed-off-by: Ihar Hrachyshka Signed-off-by: Numan Siddique (cherry picked from commit 1148580290d0ace803f20aeaa0241dd51c100630) --- controller/binding.c | 39 +++++++++++++++-- controller/ovn-controller.c | 2 + controller/ovn-controller.h | 2 + controller/physical.c | 46 ++++++++++++++++++++ tests/ovn.at | 85 +++++++++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 4 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 594babc98..1c648fc17 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -108,6 +108,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, hmap_insert(local_datapaths, &ld->hmap_node, dp_key); ld->datapath = datapath; ld->localnet_port = NULL; + shash_init(&ld->external_ports); ld->has_local_l3gateway = has_local_l3gateway; if (tracked_datapaths) { @@ -474,6 +475,18 @@ is_network_plugged(const struct sbrec_port_binding *binding_rec, return network ? !!shash_find_data(bridge_mappings, network) : false; } +static void +update_ld_external_ports(const struct sbrec_port_binding *binding_rec, + struct hmap *local_datapaths) +{ + struct local_datapath *ld = get_local_datapath( + local_datapaths, binding_rec->datapath->tunnel_key); + if (ld) { + shash_replace(&ld->external_ports, binding_rec->logical_port, + binding_rec); + } +} + static void update_ld_localnet_port(const struct sbrec_port_binding *binding_rec, struct shash *bridge_mappings, @@ -1631,8 +1644,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL; struct ovs_list localnet_lports = OVS_LIST_INITIALIZER(&localnet_lports); + struct ovs_list external_lports = OVS_LIST_INITIALIZER(&external_lports); - struct localnet_lport { + struct lport { struct ovs_list list_node; const struct sbrec_port_binding *pb; }; @@ -1680,11 +1694,14 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) case LP_EXTERNAL: consider_external_lport(pb, b_ctx_in, b_ctx_out); + struct lport *ext_lport = xmalloc(sizeof *ext_lport); + ext_lport->pb = pb; + ovs_list_push_back(&external_lports, &ext_lport->list_node); break; case LP_LOCALNET: { consider_localnet_lport(pb, b_ctx_in, b_ctx_out, &qos_map); - struct localnet_lport *lnet_lport = xmalloc(sizeof *lnet_lport); + struct lport *lnet_lport = xmalloc(sizeof *lnet_lport); lnet_lport->pb = pb; ovs_list_push_back(&localnet_lports, &lnet_lport->list_node); break; @@ -1711,7 +1728,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) /* Run through each localnet lport list to see if it is a localnet port * on local datapaths discovered from above loop, and update the * corresponding local datapath accordingly. */ - struct localnet_lport *lnet_lport; + struct lport *lnet_lport; LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) { update_ld_localnet_port(lnet_lport->pb, &bridge_mappings, b_ctx_out->egress_ifaces, @@ -1719,6 +1736,15 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) free(lnet_lport); } + /* Run through external lport list to see if these are external ports + * on local datapaths discovered from above loop, and update the + * corresponding local datapath accordingly. */ + struct lport *ext_lport; + LIST_FOR_EACH_POP (ext_lport, list_node, &external_lports) { + update_ld_external_ports(ext_lport->pb, b_ctx_out->local_datapaths); + free(ext_lport); + } + shash_destroy(&bridge_mappings); if (!sset_is_empty(b_ctx_out->egress_ifaces) @@ -1921,6 +1947,8 @@ remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, pb->logical_port)) { ld->localnet_port = NULL; } + } else if (!strcmp(pb->type, "external")) { + shash_find_and_delete(&ld->external_ports, pb->logical_port); } if (!strcmp(pb->type, "l3gateway")) { @@ -2391,6 +2419,7 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, struct shash deleted_other_pbs = SHASH_INITIALIZER(&deleted_other_pbs); const struct sbrec_port_binding *pb; + bool external_port_deleted = false; bool handled = true; SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, @@ -2423,6 +2452,7 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, } else if (lport_type == LP_VIRTUAL) { shash_add(&deleted_virtual_pbs, pb->logical_port, pb); } else { + external_port_deleted |= (lport_type == LP_EXTERNAL); shash_add(&deleted_other_pbs, pb->logical_port, pb); } } @@ -2578,6 +2608,7 @@ delete_done: case LP_EXTERNAL: handled = consider_external_lport(pb, b_ctx_in, b_ctx_out); + update_ld_external_ports(pb, b_ctx_out->local_datapaths); break; case LP_LOCALNET: { @@ -2616,7 +2647,7 @@ delete_done: } destroy_qos_map(&qos_map); - return handled; + return handled && !external_port_deleted; } /* Static functions for local_lbindind and binding_lport. */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6499e361a..69da7e9d4 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1097,6 +1097,7 @@ en_runtime_data_cleanup(void *data) HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { free(cur_node->peer_ports); + shash_destroy(&cur_node->external_ports); hmap_remove(&rt_data->local_datapaths, &cur_node->hmap_node); free(cur_node); } @@ -1208,6 +1209,7 @@ en_runtime_data_run(struct engine_node *node, void *data) struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, local_datapaths) { free(cur_node->peer_ports); + shash_destroy(&cur_node->external_ports); hmap_remove(local_datapaths, &cur_node->hmap_node); free(cur_node); } diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index 5d9466880..2bf1fecbf 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -67,6 +67,8 @@ struct local_datapath { size_t n_peer_ports; size_t n_allocated_peer_ports; + + struct shash external_ports; }; struct local_datapath *get_local_datapath(const struct hmap *, diff --git a/controller/physical.c b/controller/physical.c index 018e09540..45ea6cf05 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1272,6 +1272,52 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 160, binding->header_.uuid.parts[0], &match, ofpacts_p, &binding->header_.uuid); + + /* localport traffic directed to external is *not* local */ + struct shash_node *node; + SHASH_FOR_EACH (node, &ld->external_ports) { + const struct sbrec_port_binding *pb = node->data; + + /* skip ports that are not claimed by this chassis */ + if (!pb->chassis) { + continue; + } + if (strcmp(pb->chassis->name, chassis->name)) { + continue; + } + + ofpbuf_clear(ofpacts_p); + for (int i = 0; i < MFF_N_LOG_REGS; i++) { + put_load(0, MFF_REG0 + i, 0, 32, ofpacts_p); + } + put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, ofpacts_p); + + /* allow traffic directed to external MAC address */ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + for (int i = 0; i < pb->n_mac; i++) { + char *err_str; + struct eth_addr peer_mac; + if ((err_str = str_to_mac(pb->mac[i], &peer_mac))) { + VLOG_WARN_RL( + &rl, "Parsing MAC failed for external port: %s, " + "with error: %s", pb->logical_port, err_str); + free(err_str); + continue; + } + + match_init_catchall(&match); + match_set_metadata(&match, htonll(dp_key)); + match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, + port_key); + match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0, + MLF_LOCALPORT, MLF_LOCALPORT); + match_set_dl_dst(&match, peer_mac); + + ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 170, + binding->header_.uuid.parts[0], &match, + ofpacts_p, &binding->header_.uuid); + } + } } } else if (!tun && !is_ha_remote) { diff --git a/tests/ovn.at b/tests/ovn.at index a2ea59721..515bcf7c7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -12136,6 +12136,91 @@ OVN_CLEANUP([hv1]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([localport doesn't suppress ARP directed to external port]) + +ovn_start +net_add n1 + +check ovs-vsctl add-br br-phys +check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys +ovn_attach n1 br-phys 192.168.0.1 + +check ovn-nbctl ls-add ls + +# create topology to allow to talk from localport through localnet to external port +check ovn-nbctl lsp-add ls lp +check ovn-nbctl lsp-set-addresses lp "00:00:00:00:00:01 10.0.0.1" +check ovn-nbctl lsp-set-type lp localport +check ovs-vsctl add-port br-int lp -- set Interface lp external-ids:iface-id=lp + +check ovn-nbctl --wait=sb ha-chassis-group-add hagrp +check ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp main 10 +check ovn-nbctl lsp-add ls lext +check ovn-nbctl lsp-set-addresses lext "00:00:00:00:00:02 10.0.0.2" +check ovn-nbctl lsp-set-type lext external +hagrp_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp` +check ovn-nbctl set logical_switch_port lext ha_chassis_group=$hagrp_uuid + +check ovn-nbctl lsp-add ls ln +check ovn-nbctl lsp-set-addresses ln unknown +check ovn-nbctl lsp-set-type ln localnet +check ovn-nbctl lsp-set-options ln network_name=phys +check ovn-nbctl --wait=hv sync + +# also create second external port AFTER localnet to check that order is irrelevant +check ovn-nbctl lsp-add ls lext2 +check ovn-nbctl lsp-set-addresses lext2 "00:00:00:00:00:10 10.0.0.10" +check ovn-nbctl lsp-set-type lext2 external +check ovn-nbctl set logical_switch_port lext2 ha_chassis_group=$hagrp_uuid +check ovn-nbctl --wait=hv sync + +# create and immediately delete an external port to later check that flows for +# deleted ports are not left over in flow table +check ovn-nbctl lsp-add ls lext-deleted +check ovn-nbctl lsp-set-addresses lext-deleted "00:00:00:00:00:03 10.0.0.3" +check ovn-nbctl lsp-set-type lext-deleted external +check ovn-nbctl set logical_switch_port lext-deleted ha_chassis_group=$hagrp_uuid +check ovn-nbctl --wait=hv sync +check ovn-nbctl lsp-del lext-deleted +check ovn-nbctl --wait=hv sync + +send_garp() { + local inport=$1 eth_src=$2 eth_dst=$3 spa=$4 tpa=$5 + local request=${eth_dst}${eth_src}08060001080006040001${eth_src}${spa}${eth_dst}${tpa} + ovs-appctl netdev-dummy/receive $inport $request +} + +spa=$(ip_to_hex 10 0 0 1) +tpa=$(ip_to_hex 10 0 0 2) +send_garp lp 000000000001 000000000002 $spa $tpa + +spa=$(ip_to_hex 10 0 0 1) +tpa=$(ip_to_hex 10 0 0 10) +send_garp lp 000000000001 000000000010 $spa $tpa + +spa=$(ip_to_hex 10 0 0 1) +tpa=$(ip_to_hex 10 0 0 3) +send_garp lp 000000000001 000000000003 $spa $tpa + +dnl external traffic from localport should be sent to localnet +AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a000002 | wc -l],[0],[dnl +1 +],[ignore]) + +#dnl ...regardless of localnet / external ports creation order +AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a00000a | wc -l],[0],[dnl +1 +],[ignore]) + +dnl traffic from localport should not be sent to deleted external port +AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a000003 | wc -l],[0],[dnl +0 +],[ignore]) + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn -- 1 LR with HA distributed router gateway port]) ovn_start