From patchwork Thu Dec 17 09:23:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1417550 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=fraxinus.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=Q5xYHSRF; dkim-atps=neutral Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CxRPr18VKz9sVp for ; Thu, 17 Dec 2020 20:23:43 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0A9CD87561; Thu, 17 Dec 2020 09:23:42 +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 PoE0n0rw-G6c; Thu, 17 Dec 2020 09:23:38 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id CDEB88748F; Thu, 17 Dec 2020 09:23:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B584AC088E; Thu, 17 Dec 2020 09:23:37 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 58A08C013B for ; Thu, 17 Dec 2020 09:23:36 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 54F3E86851 for ; Thu, 17 Dec 2020 09:23:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hNXyRE2lQTMN for ; Thu, 17 Dec 2020 09:23:35 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by whitealder.osuosl.org (Postfix) with ESMTPS id E2DC586773 for ; Thu, 17 Dec 2020 09:23:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608197013; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type; bh=h8f3xrxfL4SRc6nZUBIRYkCko+U4lULPgFGb4QP2Uwo=; b=Q5xYHSRFO8c9M/n3ssN9AbUyR1BPjMyziCr2LTba43Ze3l2b9Wh9WgxFOsyOnlv10Clo3f YWsr/titFXE4h3RLw5nAtORvQKlMJFdU0tallOtCJypEDHD9BjaxvbnVih+5g56Tqk/PUh pESRxXFTp3SPnKHKQoVfrfm97KP1Xhc= 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-269-JIJVD4voPXePEUnrmr0NZg-1; Thu, 17 Dec 2020 04:23:31 -0500 X-MC-Unique: JIJVD4voPXePEUnrmr0NZg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C5F66180A089; Thu, 17 Dec 2020 09:23:30 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-239.ams2.redhat.com [10.36.114.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9F9C819CAC; Thu, 17 Dec 2020 09:23:27 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 17 Dec 2020 10:23:20 +0100 Message-Id: <1608197000-637-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] ovn-controller: Always run the I-P OVS Interface change handler. 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" The incremental processing engine implementation is such that if an input node gets updated but the output node doesn't have a change handler for it then the output node is immediately recomputed. That is, no other input change handlers are executed. In case of the en_physical_flow_changes node, if a ct-zone changes we were also skipping the OVS interface change handler. That is incorrect as there is an implicit requirement that flows for OVS interfaces that got deleted should be cleared before physical_run() is called. Instead, we now add an explicit change handler for ct-zone changes. This change handler never processes ct-zone updates incrementally but ensures that the OVS interface change handler is also called. Moreover, OVS interface changes (including deletes) are now processed before physical_run() is called in the flow_output physical_flow_changes_handler. This is a requirement because otherwise physical_run() will fail to add flows for new OVS interfaces that correspond to unchanged Port_Bindings. Reported-by: Daniel Alvarez Reported-at: https://bugzilla.redhat.com/1908391 Fixes: a3005f0dc777 ("ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.") Signed-off-by: Dumitru Ceara Reported-by: Krzysztof Klimonda Acked-by: Numan Siddique Tested-by: Daniel Alvarez Acked-by: Daniel Alvarez --- controller/ovn-controller.c | 30 ++++++++++++++----- tests/ovn.at | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b5f0c13..5708b7b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1714,6 +1714,16 @@ en_physical_flow_changes_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); } +/* ct_zone changes are not handled incrementally but a handler is required + * to avoid skipping the ovs_iface incremental change handler. + */ +static bool +physical_flow_changes_ct_zones_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return false; +} + /* There are OVS interface changes. Indicate to the flow_output engine * to handle these OVS interface changes for physical flow computations. */ static bool @@ -2256,6 +2266,13 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) struct ed_type_pfc_data *pfc_data = engine_get_input_data("physical_flow_changes", node); + /* If there are OVS interface changes. Try to handle them incrementally. */ + if (pfc_data->ovs_ifaces_changed) { + if (!physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table)) { + return false; + } + } + if (pfc_data->recompute_physical_flows) { /* This indicates that we need to recompute the physical flows. */ physical_clear_unassoc_flows_with_db(&fo->flow_table); @@ -2265,12 +2282,6 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) return true; } - if (pfc_data->ovs_ifaces_changed) { - /* There are OVS interface changes. Try to handle them - * incrementally. */ - return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); - } - return true; } @@ -2549,11 +2560,14 @@ main(int argc, char *argv[]) /* Engine node physical_flow_changes indicates whether * we can recompute only physical flows or we can * incrementally process the physical flows. + * + * Note: The order of inputs is important, all OVS interface changes must + * be handled before any ct_zone changes. */ - engine_add_input(&en_physical_flow_changes, &en_ct_zones, - NULL); engine_add_input(&en_physical_flow_changes, &en_ovs_interface, physical_flow_changes_ovs_iface_handler); + engine_add_input(&en_physical_flow_changes, &en_ct_zones, + physical_flow_changes_ct_zones_handler); engine_add_input(&en_flow_output, &en_addr_sets, flow_output_addr_sets_handler); diff --git a/tests/ovn.at b/tests/ovn.at index 80bc099..66088a7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22059,6 +22059,78 @@ OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) = "xru OVN_CLEANUP([hv1]) AT_CLEANUP +AT_SETUP([ovn -- Multiple OVS port changes Incremental Processing]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.10 + +check ovn-nbctl ls-add sw +check ovn-nbctl lsp-add sw lsp1 +check ovn-nbctl lsp-add sw lsp2 + +as hv1 +ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=lsp1 \ + ofport-request=1 +ovs-vsctl \ + -- add-port br-int vif2 \ + -- set Interface vif2 external_ids:iface-id=lsp2 \ + ofport-request=2 + +# Wait for ports to be bound. +wait_row_count Chassis 1 name=hv1 +ch=$(fetch_column Chassis _uuid name=hv1) +wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch +wait_row_count Port_Binding 1 logical_port=lsp2 chassis=$ch + +AS_BOX([check output flows for initial interfaces]) +as hv1 ovs-ofctl dump-flows br-int table=65 > offlows_table65.txt +AT_CAPTURE_FILE([offlows_table65.txt]) +AT_CHECK_UNQUOTED([grep -c "output:1" offlows_table65.txt], [0], [dnl +1 +]) +AT_CHECK_UNQUOTED([grep -c "output:2" offlows_table65.txt], [0], [dnl +1 +]) + +AS_BOX([delete and add OVS interfaces and force batch of updates]) +as hv1 ovn-appctl -t ovn-controller debug/pause + +as hv1 +ovs-vsctl \ + -- del-port vif1 \ + -- del-port vif2 + +as hv1 +ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=lsp1 \ + ofport-request=3 \ + -- add-port br-int vif2 \ + -- set Interface vif2 external_ids:iface-id=lsp2 \ + ofport-request=4 + +as hv1 ovn-appctl -t ovn-controller debug/resume +check ovn-nbctl --wait=hv sync + +AS_BOX([check output flows for new interfaces]) +as hv1 ovs-ofctl dump-flows br-int table=65 > offlows_table65_2.txt +AT_CAPTURE_FILE([offlows_table65_2.txt]) +AT_CHECK_UNQUOTED([grep -c "output:3" offlows_table65_2.txt], [0], [dnl +1 +]) +AT_CHECK_UNQUOTED([grep -c "output:4" offlows_table65_2.txt], [0], [dnl +1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + # Test dropping traffic destined to router owned IPs. AT_SETUP([ovn -- gateway router drop traffic for own IPs]) ovn_start