From patchwork Fri Nov 20 00:13:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Michelson X-Patchwork-Id: 1403386 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=XZToDVjM; 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 4CccTt4RVzz9sTL for ; Fri, 20 Nov 2020 11:13:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id C9BDC842DD; Fri, 20 Nov 2020 00:13:51 +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 kE_7RZyPQMgv; Fri, 20 Nov 2020 00:13:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 5F6CC80857; Fri, 20 Nov 2020 00:13:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4D85CC1825; Fri, 20 Nov 2020 00:13:50 +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 E54B8C0891 for ; Fri, 20 Nov 2020 00:13:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id CC14F8214C for ; Fri, 20 Nov 2020 00:13:49 +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 rmZvkYUx+AzW for ; Fri, 20 Nov 2020 00:13:48 +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 622BF82125 for ; Fri, 20 Nov 2020 00:13:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605831226; 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=8pRdysAIsALAC3rbr02BlMCRTtsie8NEcv0OJEizUOQ=; b=XZToDVjM01aovaLn9XPK1ZaV5XGoRQJjYmN5JVc+g6lRBb7RUk5o0akI6g8dZfGFqlLaeF bCmaF7Hq5XsJ+G9UnZRGJCUTUKHs3a3xMaWpDIh2b/j8ICAndKFY8YeKo/aa3xSvyXNP3L YzloVkQfOW7YII6FMghRf+BKDFq+u1A= 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-403-e56DYTiaNbSV-VvMlPDh3A-1; Thu, 19 Nov 2020 19:13:45 -0500 X-MC-Unique: e56DYTiaNbSV-VvMlPDh3A-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 373A3107ACFA for ; Fri, 20 Nov 2020 00:13:44 +0000 (UTC) Received: from monae.redhat.com (ovpn-113-146.rdu2.redhat.com [10.10.113.146]) by smtp.corp.redhat.com (Postfix) with ESMTP id BEEDA1346D for ; Fri, 20 Nov 2020 00:13:43 +0000 (UTC) From: Mark Michelson To: dev@openvswitch.org Date: Thu, 19 Nov 2020 19:13:43 -0500 Message-Id: <20201120001343.1485936-1-mmichels@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=mmichels@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] Clear port binding flows when datapath CT zone changes. 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" In commit f9cab11d5fabe2ae321a3b4bad5972b61df958c0, a LOG_TO_PHY flow was changed so that it was no longer associated with a particular port binding. The logic there was that the particular flow contains data pertaining to the port binding's peer's datapath, so it didn't make sense to associate the flow with the port binding. This change was necessary in order for flows to be recalculated properly if the requested SNAT CT zone on a gateway router was changed. Since the datapath was changed but no port bindings were changed, that particular flow needed to be cleared so it could be recalculated with the new CT zones put in place. Unfortunately, that change broke some other behavior. Specifically, if a router was changed from a distributed router to a gateway router, then its port bindings and its port bindings' peers would be updated so that they were no longer type "patch" but instead type "l3gateway". They would attempt to remove all associated physical flows and then install the newly relevant ones. Since the LOG_TO_PHY flow was no longer associated with a port binding, that flow would remain. The result was that traffic could be sent to the gateway router on chassis where the gateway router was not pinned. This commit seeks to fix both behaviors. Now if CT zones are changed on a particular datapath, then all port bindings on that datapath, as well as all of those port bindings' peers will have their physical flows removed. When physical flows are recomputed, all of the appropriate flows will be added. Signed-off-by: Mark Michelson --- controller/ovn-controller.c | 31 ++++++++++++++++++-- controller/physical.c | 58 +++++++++++++++++++++++++++++-------- controller/physical.h | 4 +++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 25de0c72f..eceb804ac 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -64,6 +64,7 @@ #include "timer.h" #include "stopwatch.h" #include "lib/inc-proc-eng.h" +#include "hmapx.h" VLOG_DEFINE_THIS_MODULE(main); @@ -549,7 +550,7 @@ add_pending_ct_zone_entry(struct shash *pending_ct_zones, static void update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, struct simap *ct_zones, unsigned long *ct_zone_bitmap, - struct shash *pending_ct_zones) + struct shash *pending_ct_zones, struct hmapx *updated_dps) { struct simap_node *ct_zone, *ct_zone_next; int scan_start = 1; @@ -563,6 +564,7 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, } /* Local patched datapath (gateway routers) need zones assigned. */ + struct shash all_lds = SHASH_INITIALIZER(&all_lds); const struct local_datapath *ld; HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { /* XXX Add method to limit zone assignment to logical router @@ -571,6 +573,8 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, char *snat = alloc_nat_zone_key(&ld->datapath->header_.uuid, "snat"); sset_add(&all_users, dnat); sset_add(&all_users, snat); + shash_add(&all_lds, dnat, ld); + shash_add(&all_lds, snat, ld); int req_snat_zone = datapath_snat_ct_zone(ld->datapath); if (req_snat_zone >= 0) { @@ -636,6 +640,11 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, bitmap_set1(ct_zone_bitmap, snat_req_node->data); simap_put(ct_zones, snat_req_node->name, snat_req_node->data); + struct shash_node *ld_node = shash_find(&all_lds, snat_req_node->name); + if (ld_node) { + struct local_datapath *dp = ld_node->data; + hmapx_add(updated_dps, (void *) dp->datapath); + } } /* xxx This is wasteful to assign a zone to each port--even if no @@ -664,10 +673,17 @@ update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths, bitmap_set1(ct_zone_bitmap, zone); simap_put(ct_zones, user, zone); + + struct shash_node *ld_node = shash_find(&all_lds, user); + if (ld_node) { + struct local_datapath *dp = ld_node->data; + hmapx_add(updated_dps, (void *) dp->datapath); + } } simap_destroy(&req_snat_zones); sset_destroy(&all_users); + shash_destroy(&all_lds); } static void @@ -1098,6 +1114,9 @@ struct ed_type_runtime_data { bool tracked; bool local_lports_changed; struct hmap tracked_dp_bindings; + + /* CT zone data. Contains datapaths that had updated CT zones */ + struct hmapx ct_updated_datapaths; }; /* struct ed_type_runtime_data has the below members for tracking the @@ -1189,6 +1208,8 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, /* Init the tracked data. */ hmap_init(&data->tracked_dp_bindings); + hmapx_init(&data->ct_updated_datapaths); + return data; } @@ -1348,6 +1369,7 @@ en_runtime_data_run(struct engine_node *node, void *data) sset_init(&rt_data->egress_ifaces); smap_init(&rt_data->local_iface_ids); local_bindings_init(&rt_data->local_bindings); + hmapx_clear(&rt_data->ct_updated_datapaths); } struct binding_ctx_in b_ctx_in; @@ -1486,9 +1508,11 @@ en_ct_zones_run(struct engine_node *node, void *data) struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); + hmapx_clear(&rt_data->ct_updated_datapaths); update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths, &ct_zones_data->current, ct_zones_data->bitmap, - &ct_zones_data->pending); + &ct_zones_data->pending, &rt_data->ct_updated_datapaths); + engine_set_node_state(node, EN_UPDATED); } @@ -1698,6 +1722,7 @@ static void init_physical_ctx(struct engine_node *node, p_ctx->ct_zones = ct_zones; p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; p_ctx->local_bindings = &rt_data->local_bindings; + p_ctx->ct_updated_datapaths = &rt_data->ct_updated_datapaths; } static void init_lflow_ctx(struct engine_node *node, @@ -2125,6 +2150,8 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) 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); + physical_clear_dp_flows(&p_ctx, &rt_data->ct_updated_datapaths, + &fo->flow_table); physical_run(&p_ctx, &fo->flow_table); return true; } diff --git a/controller/physical.c b/controller/physical.c index 00c4ca4fd..fa5d0d692 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -44,6 +44,7 @@ #include "sset.h" #include "util.h" #include "vswitch-idl.h" +#include "hmapx.h" VLOG_DEFINE_THIS_MODULE(physical); @@ -860,6 +861,28 @@ load_logical_ingress_metadata(const struct sbrec_port_binding *binding, put_load(port_key, MFF_LOG_INPORT, 0, 32, ofpacts_p); } +static const struct sbrec_port_binding * +get_binding_peer(struct ovsdb_idl_index *sbrec_port_binding_by_name, + const struct sbrec_port_binding *binding) +{ + const char *peer_name = smap_get(&binding->options, "peer"); + if (!peer_name) { + return NULL; + } + + const struct sbrec_port_binding *peer = lport_lookup_by_name( + sbrec_port_binding_by_name, peer_name); + if (!peer || strcmp(peer->type, binding->type)) { + return NULL; + } + const char *peer_peer_name = smap_get(&peer->options, "peer"); + if (!peer_peer_name || strcmp(peer_peer_name, binding->logical_port)) { + return NULL; + } + + return peer; +} + static void consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, enum mf_field_id mff_ovn_geneve, @@ -882,18 +905,10 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, if (!strcmp(binding->type, "patch") || (!strcmp(binding->type, "l3gateway") && binding->chassis == chassis)) { - const char *peer_name = smap_get(&binding->options, "peer"); - if (!peer_name) { - return; - } - const struct sbrec_port_binding *peer = lport_lookup_by_name( - sbrec_port_binding_by_name, peer_name); - if (!peer || strcmp(peer->type, binding->type)) { - return; - } - const char *peer_peer_name = smap_get(&peer->options, "peer"); - if (!peer_peer_name || strcmp(peer_peer_name, binding->logical_port)) { + const struct sbrec_port_binding *peer = get_binding_peer( + sbrec_port_binding_by_name, binding); + if (!peer) { return; } @@ -926,7 +941,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, binding->header_.uuid.parts[0], - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); return; } @@ -1874,3 +1889,22 @@ physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table *flow_table) ofctrl_remove_flows(flow_table, hc_uuid); } } + +void +physical_clear_dp_flows(struct physical_ctx *p_ctx, + struct hmapx *ct_updated_datapaths, + struct ovn_desired_flow_table *flow_table) +{ + const struct sbrec_port_binding *binding; + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, p_ctx->port_binding_table) { + if (!hmapx_find(ct_updated_datapaths, binding->datapath)) { + continue; + } + const struct sbrec_port_binding *peer = + get_binding_peer(p_ctx->sbrec_port_binding_by_name, binding); + ofctrl_remove_flows(flow_table, &binding->header_.uuid); + if (peer) { + ofctrl_remove_flows(flow_table, &peer->header_.uuid); + } + } +} diff --git a/controller/physical.h b/controller/physical.h index feab41df4..0a4c3bab8 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -56,12 +56,16 @@ struct physical_ctx { const struct simap *ct_zones; enum mf_field_id mff_ovn_geneve; struct shash *local_bindings; + struct hmapx *ct_updated_datapaths; }; void physical_register_ovs_idl(struct ovsdb_idl *); void physical_run(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table *); +void physical_clear_dp_flows(struct physical_ctx *p_ctx, + struct hmapx *ct_updated_datapaths, + struct ovn_desired_flow_table *flow_table); void physical_handle_port_binding_changes(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_handle_mc_group_changes(struct physical_ctx *,