From patchwork Wed Aug 18 21:35:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1518242 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=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (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 4Gqh5b66wHz9sW8 for ; Thu, 19 Aug 2021 07:35:31 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 4C87F80EB3; Wed, 18 Aug 2021 21:35:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6yPsFqCOvqPi; Wed, 18 Aug 2021 21:35:25 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 9D70F80D53; Wed, 18 Aug 2021 21:35:24 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7F6D4C001A; Wed, 18 Aug 2021 21:35:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 58464C000E for ; Wed, 18 Aug 2021 21:35:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 3F2F440191 for ; Wed, 18 Aug 2021 21:35:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IYwd7tmCXPoJ for ; Wed, 18 Aug 2021 21:35:19 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by smtp2.osuosl.org (Postfix) with ESMTPS id AA20D40171 for ; Wed, 18 Aug 2021 21:35:18 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id EB6DB60005; Wed, 18 Aug 2021 21:35:14 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Wed, 18 Aug 2021 17:35:11 -0400 Message-Id: <20210818213511.3076974-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn] Support additional 'vif-id' options in OVS inteface for claiming an lport. 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" From: Numan Siddique In order for the ovn-controller to claim a logical port, it maps the OVS interface external_ids:iface-id to the port binding's logical_port column. This patch adds support for another option - 'vif-id'. CMS needs to set the same key-value in Logical_Switch_Port.options column. ovn-controller will claim the OVS interface only if external_ids:iface-id matches with the Port_Binding.logical_port and external_ids:vif-id matches with the Port_Binding.options:vif-id. This is not mandatory. If Port_binding.options:vif-id is not set, then OVS Interface.external_ids:vif-id if set is ignored. This support is added so that CMS can uniquely identify the OVS interface to the corresponding logical port. The referenced bugzilla and this ovn-k8s commit [1] has additional details. [1] - https://github.com/ovn-org/ovn-kubernetes/commit/22bed6a10c6 Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1995333 Signed-off-by: Numan Siddique Acked-by: Mark D. Gray --- controller/binding.c | 47 ++++++++++++++- ovn-nb.xml | 8 +++ ovn-sb.xml | 8 +++ tests/ovn.at | 141 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 201 insertions(+), 3 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 52eb47b79..743809a21 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -600,6 +600,9 @@ static struct binding_lport *binding_lport_check_and_cleanup( struct binding_lport *, struct shash *b_lports); static char *get_lport_type_str(enum en_lport_type lport_type); +static bool is_ovs_iface_matches_lport_vif_id( + const struct ovsrec_interface *, + const struct sbrec_port_binding *); void related_lports_init(struct related_lports *rp) @@ -1165,9 +1168,30 @@ consider_vif_lport(const struct sbrec_port_binding *pb, struct binding_lport *b_lport = NULL; if (lbinding) { - struct shash *binding_lports = - &b_ctx_out->lbinding_data->lports; - b_lport = local_binding_add_lport(binding_lports, lbinding, pb, LP_VIF); + bool add_b_lport = true; + + /* Make sure that the pb's vif-id if set matches with the + * lbinding's vif-id. */ + if (lbinding->iface && + !is_ovs_iface_matches_lport_vif_id(lbinding->iface, pb)) { + /* We can't associate the b_lport for this local_binding + * because the vif-id doesn't match. */ + add_b_lport = false; + + b_lport = local_binding_get_primary_lport(lbinding); + if (b_lport) { + binding_lport_delete(&b_ctx_out->lbinding_data->lports, + b_lport); + b_lport = NULL; + } + } + + if (add_b_lport) { + struct shash *binding_lports = + &b_ctx_out->lbinding_data->lports; + b_lport = local_binding_add_lport(binding_lports, lbinding, pb, + LP_VIF); + } } return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, @@ -2850,3 +2874,20 @@ cleanup: return b_lport; } + + +static bool +is_ovs_iface_matches_lport_vif_id(const struct ovsrec_interface *iface, + const struct sbrec_port_binding *pb) +{ + const char *pb_vif_id = smap_get(&pb->options, "vif-id"); + const char *vif_id = smap_get(&iface->external_ids, "vif-id"); + + if (pb_vif_id) { + if (!vif_id || strcmp(pb_vif_id, vif_id)) { + return false; + } + } + + return true; +} diff --git a/ovn-nb.xml b/ovn-nb.xml index 8a942b54c..4848dbfe0 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1002,6 +1002,14 @@ one chassis. + + If set, this port will be bound by ovn-controller + only if this same key and value is configured in the + + column in the Open_vSwitch database's + table. + + If set, indicates the maximum rate for data sent from this interface, in bit/s. The traffic will be shaped according to this limit. diff --git a/ovn-sb.xml b/ovn-sb.xml index 687555c47..2163162b8 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -3149,6 +3149,14 @@ tcp.flags = RST; one chassis. + + If set, this port will be bound by ovn-controller + only if this same key and value is configured in the + + column in the Open_vSwitch database's + table. + + If set, indicates the maximum rate for data sent from this interface, in bit/s. The traffic will be shaped according to this limit. diff --git a/tests/ovn.at b/tests/ovn.at index f2882d1ad..17ac2b78a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -27900,3 +27900,144 @@ AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=24 | grep 10.0.1.2], [0], [ig OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) + + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller port binding with vif-id]) +ovn_start + +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl -- add-port br-int hv1-vm1 + +ovn-nbctl ls-add sw0 +ovn-nbctl lsp-add sw0 sw0-port1 +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3" + +ovn-nbctl lsp-set-options sw0-port1 vif-id=foo + +ovn-nbctl lsp-add sw0 sw0-port2 +ovn-nbctl --wait=hv lsp-set-addresses sw0-port2 "50:54:00:00:00:02 10.0.0.4" + +hv1_uuid=$(fetch_column Chassis _uuid name=hv1) +hv2_uuid=$(fetch_column Chassis _uuid name=hv2) + +check as hv1 ovs-vsctl add-port br-int vif11 \ + -- set interface vif11 external_ids:iface-id=sw0-port1 + +# sw0-port1 should not be claimed. +ovn-nbctl --wait=hv sync +check_column "" Port_Binding chassis logical_port=sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[0]] +---------------------------------------- +]) + +# Set vif-id on vif11. hv1 ovn-controller should claim it now. +check as hv1 ovs-vsctl set interface vif11 external_ids:vif-id=foo + +wait_for_ports_up sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[1]] +primary lport : [[sw0-port1]] +---------------------------------------- +]) + +# Clear the vif-id from vif11 and hv1 ovn-controller should release it. +check as hv1 ovs-vsctl remove interface vif11 external_ids vif-id +ovn-nbctl --wait=hv sync +check_column "" Port_Binding chassis logical_port=sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[0]] +---------------------------------------- +]) + +# Clear the sw0-port1 vif-id options and sw0-port1 should be claimed. +check ovn-nbctl clear logical_switch_port sw0-port1 options + +wait_for_ports_up sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[1]] +primary lport : [[sw0-port1]] +---------------------------------------- +]) + +# Set the options:vif-id to sw0-port1 with different value. +check ovn-nbctl lsp-set-options sw0-port1 vif-id=bar + +ovn-nbctl --wait=hv sync +check_column "" Port_Binding chassis logical_port=sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[0]] +---------------------------------------- +]) + +# Set vif-id on vif11. hv1 ovn-controller should claim it now. +check as hv1 ovs-vsctl set interface vif11 external_ids:vif-id=bar +wait_for_ports_up sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[1]] +primary lport : [[sw0-port1]] +---------------------------------------- +]) + +# Set a different vif-id on vif11. +check as hv1 ovs-vsctl set interface vif11 external_ids:vif-id=bar2 + +ovn-nbctl --wait=hv sync +check_column "" Port_Binding chassis logical_port=sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[0]] +---------------------------------------- +]) + +# Set the type of sw0-port1 to localport. vif-id is ignored for localports. +# So sw0-port1 should be internally claimed without setting sw0-port1 to up. +check ovn-nbctl lsp-set-type sw0-port1 localport + +ovn-nbctl --wait=hv sync +check_column "" Port_Binding chassis logical_port=sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[1]] +localport lport : [[sw0-port1]] +---------------------------------------- +]) + +check as hv1 ovs-vsctl add-port br-int vif12 \ + -- set interface vif12 external_ids:iface-id=sw0-port2 + +wait_for_ports_up sw0-port2 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-local-bindings], [0], [dnl +Local bindings: +name: [[sw0-port1]], OVS interface name : [[vif11]], num binding lports : [[1]] +localport lport : [[sw0-port1]] +---------------------------------------- +name: [[sw0-port2]], OVS interface name : [[vif12]], num binding lports : [[1]] +primary lport : [[sw0-port2]] +---------------------------------------- +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +])