From patchwork Mon Aug 13 17:48:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 957080 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="VLFDLJEQ"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41q3HG6HqVz9s78 for ; Tue, 14 Aug 2018 03:53:14 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 870A3D6D; Mon, 13 Aug 2018 17:48:41 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 49996DC2 for ; Mon, 13 Aug 2018 17:48:38 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 3FDA67C4 for ; Mon, 13 Aug 2018 17:48:37 +0000 (UTC) Received: by mail-pf1-f169.google.com with SMTP id l9-v6so7989862pff.9 for ; Mon, 13 Aug 2018 10:48:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=4henZXUdlLbk78ZPhl72aO8ju02tHedm4e28IMlp7uk=; b=VLFDLJEQYQLA8pjOIeZw5AH03tT6Zq7ITShQom6f73386wtNEn9Irn28cLiZ6UyEj1 T9QEFKzLr7XtBQv1vDPgXJCIS51aefF+k3QtsSMaqWexKbOwDrlIglN7eNc/Lr2IPJ9m m2C7OY6WUlal+4ytXSMlWLLWWY8ADuYkE2TknAevfl9qTwcAwlOI+7NlxVEBkmhnT6gZ d3/JBgNisvBDqmQ+ESH1HrBhtrYRC/Z2pxKzVj3GGemk2SOFmXug7C6Q/g1O/4zkH/Pt v4jjPCfSSCP112spQ0iZupPF6p4QORVwy6xkw4chST5+QU3g8VTDRqAbyH90ee7t4pjC +mkA== 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:in-reply-to :references; bh=4henZXUdlLbk78ZPhl72aO8ju02tHedm4e28IMlp7uk=; b=ZgJD9atVW6MLhxLJShAseoOAXoOxYaLoGYqodSemEXqh+csuxpgqvxo5GKVvYLGAHj JBTmvg9AVgyAvPgfqFwa5bNIiontC6Pu97e/bmOuYVanmD9z8Hiw+omt0a8MIXUp8IHc LZktVUry8KvWDn0k/Rvy5LSSV5/W2ZZXfpXrSxuy0Zuk4D60DjxbG2an0HoN9CtvrF/5 kRHJiqdfJZ3swNdsl5ElHDPkCAJkY/m4wJFWPbgsQVrsFYSctpsupaRQPGjNqj5IKgSf aiEPbqz8WATBxKEnobqRF5Ph/WLCFsGJLGLprI5qbUbaBNJyGiUPGbHU5GdfzNMX9PrX IjQg== X-Gm-Message-State: AOUpUlECAYFSMy5GL/jfVxihWeT9MOrS1Hxrn0pL7i/G5QP05A8cayRn lK7mKvYYhG2CTsoFm8yTDdM4N+NU X-Google-Smtp-Source: AA+uWPzbaFXMTaO0W3mQ6SV3zUiKlIS9OQmLNag01FmslAv/8qXQ2UnY8jnE6ZMU+qaJ7ZFANf3zFw== X-Received: by 2002:a63:ee56:: with SMTP id n22-v6mr17191424pgk.402.1534182516564; Mon, 13 Aug 2018 10:48:36 -0700 (PDT) Received: from localhost.localdomain.localdomain ([216.113.160.70]) by smtp.gmail.com with ESMTPSA id d22-v6sm38068664pfk.69.2018.08.13.10.48.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Aug 2018 10:48:36 -0700 (PDT) From: Han Zhou X-Google-Original-From: Han Zhou To: dev@openvswitch.org Date: Mon, 13 Aug 2018 10:48:09 -0700 Message-Id: <1534182499-75914-11-git-send-email-hzhou8@ebay.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1534182499-75914-1-git-send-email-hzhou8@ebay.com> References: <1534182499-75914-1-git-send-email-hzhou8@ebay.com> X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v5 10/20] ovn-controller: port-binding incremental processing for physical flows X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This patch implements change handler for port-binding in flow_output for physical flows computing, so that physical flow computing will be incremental. Signed-off-by: Han Zhou --- ovn/controller/ovn-controller.c | 95 ++++++++++++++++++++++++++++++++++- ovn/controller/physical.c | 106 ++++++++++++++++++++++++++-------------- ovn/controller/physical.h | 10 ++++ 3 files changed, 173 insertions(+), 38 deletions(-) diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index ba589fe..9fd2cba 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -1087,6 +1087,99 @@ flow_output_sb_logical_flow_handler(struct engine_node *node) return handled; } +static bool +flow_output_sb_port_binding_handler(struct engine_node *node) +{ + struct ed_type_runtime_data *data = + (struct ed_type_runtime_data *)engine_get_input( + "runtime_data", node)->data; + struct hmap *local_datapaths = &data->local_datapaths; + struct sset *active_tunnels = &data->active_tunnels; + struct simap *ct_zones = &data->ct_zones; + + struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = + (struct ed_type_mff_ovn_geneve *)engine_get_input( + "mff_ovn_geneve", node)->data; + enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; + + struct ovsrec_open_vswitch_table *ovs_table = + (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( + engine_get_input("OVS_open_vswitch", node)); + struct ovsrec_bridge_table *bridge_table = + (struct ovsrec_bridge_table *)EN_OVSDB_GET( + engine_get_input("OVS_bridge", node)); + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); + const char *chassis_id = get_chassis_id(ovs_table); + + struct ovsdb_idl_index *sbrec_chassis_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_chassis", node), + "name"); + const struct sbrec_chassis *chassis = NULL; + if (chassis_id) { + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); + } + ovs_assert(br_int && chassis); + + struct ed_type_flow_output *fo = + (struct ed_type_flow_output *)node->data; + struct ovn_desired_flow_table *flow_table = &fo->flow_table; + + struct ovsdb_idl_index *sbrec_port_binding_by_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_port_binding", node), + "name"); + + struct sbrec_port_binding_table *port_binding_table = + (struct sbrec_port_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_port_binding", node)); + + /* XXX: now we handles port-binding changes for physical flow processing + * only, but port-binding change can have impact to logical flow + * processing, too, in below circumstances: + * + * - When a port-binding for a lport is inserted/deleted but the lflow + * using that lport doesn't change. + * + * This is likely to happen only when the lport name is used by ACL + * match condition, which is specified by user. Even in that case, when + * port is actually bound on the chassis it will trigger recompute on + * that chassis since ovs interface is updated. So the only situation + * this would have real impact is when user defines an ACL that includes + * lport that is not the ingress/egress lport, e.g.: + * + * to-lport 1000 'outport=="A" && inport=="B"' allow-related + * + * If "B" is created and bound after the ACL is created, the ACL may not + * take effect on the chassis where "A" is bound, until a recompute is + * triggered there later. + * + * - When is_chassis_resident is used in lflow. In this case the port + * binding is patch type, since this condition is used only for lrouter + * ports. In current "runtime_data" handling, port-binding changes of + * patch ports always trigger recomputing. So it is fine even if we do + * not handle it here. + * + * - When a mac-binding doesn't change but the port-binding related to + * that mac-binding is deleted. In this case the neighbor flow generated + * for the mac-binding should be deleted. This would not cause any real + * issue for now, since mac-binding change triggers recomputing. + * + * To address the above issues, we will need to maintain a mapping between + * lport names and the lflows that uses them, and reprocess the related + * lflows when a port-binding corresponding to a lport name changes. + */ + + physical_handle_port_binding_changes( + sbrec_chassis_by_name, sbrec_port_binding_by_name, + port_binding_table, mff_ovn_geneve, + chassis, ct_zones, local_datapaths, + active_tunnels, flow_table); + + node->changed = true; + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -1204,7 +1297,7 @@ main(int argc, char *argv[]) engine_add_input(&en_flow_output, &en_sb_encap, NULL); engine_add_input(&en_flow_output, &en_sb_multicast_group, NULL); engine_add_input(&en_flow_output, &en_sb_datapath_binding, NULL); - engine_add_input(&en_flow_output, &en_sb_port_binding, NULL); + engine_add_input(&en_flow_output, &en_sb_port_binding, flow_output_sb_port_binding_handler); engine_add_input(&en_flow_output, &en_sb_mac_binding, NULL); engine_add_input(&en_flow_output, &en_sb_logical_flow, flow_output_sb_logical_flow_handler); engine_add_input(&en_flow_output, &en_sb_dhcp_options, NULL); diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index c9ea2a6..1cb9f7b 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -360,7 +360,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, ofpact_finish_CLONE(ofpacts_p, &clone); ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); return; } @@ -429,7 +429,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, } ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); goto out; } @@ -572,7 +572,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, /* Resubmit to first logical ingress pipeline table. */ put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p); ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, - tag ? 150 : 100, 0, &match, ofpacts_p, hc_uuid); + tag ? 150 : 100, 0, &match, ofpacts_p, + &binding->header_.uuid); if (!tag && (!strcmp(binding->type, "localnet") || !strcmp(binding->type, "l2gateway"))) { @@ -582,7 +583,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, * action. */ ofpbuf_pull(ofpacts_p, ofpacts_orig_size); match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI)); - ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p, hc_uuid); + ofctrl_add_flow(flow_table, 0, 100, 0, &match, ofpacts_p, + &binding->header_.uuid); } /* Table 65, Priority 100. @@ -610,7 +612,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, ofpact_put_STRIP_VLAN(ofpacts_p); } ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); } else if (!tun && !is_ha_remote) { /* Remote port connected by localnet port */ /* Table 33, priority 100. @@ -633,7 +635,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, /* Resubmit to table 33. */ put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p); ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); } else { /* Remote port connected by tunnel */ @@ -724,7 +726,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name, ofpact_finish_BUNDLE(ofpacts_p, &bundle); } ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &binding->header_.uuid); } out: if (gateway_chassis) { @@ -738,8 +740,6 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, const struct hmap *local_datapaths, const struct sbrec_chassis *chassis, const struct sbrec_multicast_group *mc, - struct ofpbuf *ofpacts_p, - struct ofpbuf *remote_ofpacts_p, struct ovn_desired_flow_table *flow_table) { uint32_t dp_key = mc->datapath->tunnel_key; @@ -759,15 +759,17 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, * - For remote ports, add the chassis to 'remote_chassis'. * * - For local ports (other than logical patch ports), add actions - * to 'ofpacts_p' to set the output port and resubmit. + * to 'ofpacts' to set the output port and resubmit. * - * - For logical patch ports, add actions to 'remote_ofpacts_p' + * - For logical patch ports, add actions to 'remote_ofpacts' * instead. (If we put them in 'ofpacts', then the output * would happen on every hypervisor in the multicast group, * effectively duplicating the packet.) */ - ofpbuf_clear(ofpacts_p); - ofpbuf_clear(remote_ofpacts_p); + struct ofpbuf ofpacts; + ofpbuf_init(&ofpacts, 0); + struct ofpbuf remote_ofpacts; + ofpbuf_init(&remote_ofpacts, 0); for (size_t i = 0; i < mc->n_ports; i++) { struct sbrec_port_binding *port = mc->ports[i]; @@ -781,20 +783,20 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, int zone_id = simap_get(ct_zones, port->logical_port); if (zone_id) { - put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, ofpacts_p); + put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts); } if (!strcmp(port->type, "patch")) { put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, - remote_ofpacts_p); - put_resubmit(OFTABLE_CHECK_LOOPBACK, remote_ofpacts_p); + &remote_ofpacts); + put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts); } else if (simap_contains(&localvif_to_ofport, (port->parent_port && *port->parent_port) ? port->parent_port : port->logical_port) || (!strcmp(port->type, "l3gateway") && port->chassis == chassis)) { - put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p); - put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p); + put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); + put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); } else if (port->chassis && !get_localnet_port(local_datapaths, mc->datapath->tunnel_key)) { /* Add remote chassis only when localnet port not exist, @@ -809,14 +811,14 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, * * Handle output to the local logical ports in the multicast group, if * any. */ - bool local_ports = ofpacts_p->size > 0; + bool local_ports = ofpacts.size > 0; if (local_ports) { /* Following delivery to local logical ports, restore the multicast * group as the logical output port. */ - put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p); + put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0, - &match, ofpacts_p, hc_uuid); + &match, &ofpacts, &mc->header_.uuid); } /* Table 32, priority 100. @@ -824,12 +826,12 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, * * Handle output to the remote chassis in the multicast group, if * any. */ - if (!sset_is_empty(&remote_chassis) || remote_ofpacts_p->size > 0) { - if (remote_ofpacts_p->size > 0) { + if (!sset_is_empty(&remote_chassis) || remote_ofpacts.size > 0) { + if (remote_ofpacts.size > 0) { /* Following delivery to logical patch ports, restore the * multicast group as the logical output port. */ put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, - remote_ofpacts_p); + &remote_ofpacts); } const char *chassis_name; @@ -843,20 +845,22 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, if (!prev || tun->type != prev->type) { put_encapsulation(mff_ovn_geneve, tun, mc->datapath, - mc->tunnel_key, remote_ofpacts_p); + mc->tunnel_key, &remote_ofpacts); prev = tun; } - ofpact_put_OUTPUT(remote_ofpacts_p)->port = tun->ofport; + ofpact_put_OUTPUT(&remote_ofpacts)->port = tun->ofport; } - if (remote_ofpacts_p->size) { + if (remote_ofpacts.size) { if (local_ports) { - put_resubmit(OFTABLE_LOCAL_OUTPUT, remote_ofpacts_p); + put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts); } ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100, 0, - &match, remote_ofpacts_p, hc_uuid); + &match, &remote_ofpacts, &mc->header_.uuid); } } + ofpbuf_uninit(&ofpacts); + ofpbuf_uninit(&remote_ofpacts); sset_destroy(&remote_chassis); } @@ -871,6 +875,38 @@ update_ofports(struct simap *old, struct simap *new) return changed; } +void physical_handle_port_binding_changes( + struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_port_binding_by_name, + const struct sbrec_port_binding_table *pb_table, + enum mf_field_id mff_ovn_geneve, + const struct sbrec_chassis *chassis, + const struct simap *ct_zones, + struct hmap *local_datapaths, + struct sset *active_tunnels, + struct ovn_desired_flow_table *flow_table) +{ + const struct sbrec_port_binding *binding; + struct ofpbuf ofpacts; + ofpbuf_init(&ofpacts, 0); + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) { + if (sbrec_port_binding_is_deleted(binding)) { + ofctrl_remove_flows(flow_table, &binding->header_.uuid); + } else { + if (!sbrec_port_binding_is_new(binding)) { + ofctrl_remove_flows(flow_table, &binding->header_.uuid); + } + consider_port_binding(sbrec_chassis_by_name, + sbrec_port_binding_by_name, + mff_ovn_geneve, ct_zones, + active_tunnels, local_datapaths, + binding, chassis, + flow_table, &ofpacts); + } + } + +} + void physical_run(struct ovsdb_idl_index *sbrec_chassis_by_name, struct ovsdb_idl_index *sbrec_port_binding_by_name, @@ -1011,22 +1047,18 @@ physical_run(struct ovsdb_idl_index *sbrec_chassis_by_name, consider_port_binding(sbrec_chassis_by_name, sbrec_port_binding_by_name, mff_ovn_geneve, ct_zones, - active_tunnels, - local_datapaths, binding, chassis, + active_tunnels, local_datapaths, + binding, chassis, flow_table, &ofpacts); } /* Handle output to multicast groups, in tables 32 and 33. */ const struct sbrec_multicast_group *mc; - struct ofpbuf remote_ofpacts; - ofpbuf_init(&remote_ofpacts, 0); SBREC_MULTICAST_GROUP_TABLE_FOR_EACH (mc, multicast_group_table) { - consider_mc_group(mff_ovn_geneve, ct_zones, local_datapaths, chassis, - mc, &ofpacts, &remote_ofpacts, flow_table); + consider_mc_group(mff_ovn_geneve, ct_zones, local_datapaths, + chassis, mc, flow_table); } - ofpbuf_uninit(&remote_ofpacts); - /* Table 0, priority 100. * ====================== * diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h index 1dd2b9d..a9c8445 100644 --- a/ovn/controller/physical.h +++ b/ovn/controller/physical.h @@ -55,5 +55,15 @@ void physical_run(struct ovsdb_idl_index *sbrec_chassis_by_name, const struct sset *local_lports, const struct sset *active_tunnels, struct ovn_desired_flow_table *); +void physical_handle_port_binding_changes( + struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_port_binding_by_name, + const struct sbrec_port_binding_table *, + enum mf_field_id mff_ovn_geneve, + const struct sbrec_chassis *, + const struct simap *ct_zones, + struct hmap *local_datapaths, + struct sset *active_tunnels, + struct ovn_desired_flow_table *); #endif /* ovn/physical.h */