Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1306576/?format=api
{ "id": 1306576, "url": "http://patchwork.ozlabs.org/api/patches/1306576/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openvswitch/patch/20200610061819.1973526-1-numans@ovn.org/", "project": { "id": 47, "url": "http://patchwork.ozlabs.org/api/projects/47/?format=api", "name": "Open vSwitch", "link_name": "openvswitch", "list_id": "ovs-dev.openvswitch.org", "list_email": "ovs-dev@openvswitch.org", "web_url": "http://openvswitch.org/", "scm_url": "git@github.com:openvswitch/ovs.git", "webscm_url": "https://github.com/openvswitch/ovs", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20200610061819.1973526-1-numans@ovn.org>", "list_archive_url": null, "date": "2020-06-10T06:18:19", "name": "[ovs-dev,ovn,v11,2/6] ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "676d59e06e19c06ea335a55e695dd670bc5abb6c", "submitter": { "id": 77669, "url": "http://patchwork.ozlabs.org/api/people/77669/?format=api", "name": "Numan Siddique", "email": "numans@ovn.org" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/openvswitch/patch/20200610061819.1973526-1-numans@ovn.org/mbox/", "series": [ { "id": 182423, "url": "http://patchwork.ozlabs.org/api/series/182423/?format=api", "web_url": "http://patchwork.ozlabs.org/project/openvswitch/list/?series=182423", "date": "2020-06-10T06:17:26", "name": "Incremental processing improvements.", "version": 11, "mbox": "http://patchwork.ozlabs.org/series/182423/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/1306576/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/1306576/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<ovs-dev-bounces@openvswitch.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "dev@openvswitch.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "ovs-dev@lists.linuxfoundation.org" ], "Authentication-Results": [ "ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.133; helo=hemlock.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=<UNKNOWN>)", "ozlabs.org;\n dmarc=none (p=none dis=none) header.from=ovn.org" ], "Received": [ "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 49hcHv1QDzz9sSF\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 10 Jun 2020 16:18:35 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 73FAC889CA;\n\tWed, 10 Jun 2020 06:18:33 +0000 (UTC)", "from hemlock.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id erUknp2-JIwB; Wed, 10 Jun 2020 06:18:31 +0000 (UTC)", "from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id C5911889D0;\n\tWed, 10 Jun 2020 06:18:31 +0000 (UTC)", "from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id A227FC016F;\n\tWed, 10 Jun 2020 06:18:31 +0000 (UTC)", "from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 854F5C016F\n for <dev@openvswitch.org>; Wed, 10 Jun 2020 06:18:30 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n by fraxinus.osuosl.org (Postfix) with ESMTP id 7564386FC3\n for <dev@openvswitch.org>; Wed, 10 Jun 2020 06:18:30 +0000 (UTC)", "from fraxinus.osuosl.org ([127.0.0.1])\n by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n with ESMTP id kcki3XJ_zzqj for <dev@openvswitch.org>;\n Wed, 10 Jun 2020 06:18:29 +0000 (UTC)", "from relay10.mail.gandi.net (relay10.mail.gandi.net\n [217.70.178.230])\n by fraxinus.osuosl.org (Postfix) with ESMTPS id 975F586F8E\n for <dev@openvswitch.org>; Wed, 10 Jun 2020 06:18:28 +0000 (UTC)", "from nusiddiq.home.org.com (unknown [115.99.171.40])\n (Authenticated sender: numans@ovn.org)\n by relay10.mail.gandi.net (Postfix) with ESMTPSA id 7326E240002;\n Wed, 10 Jun 2020 06:18:24 +0000 (UTC)" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "From": "numans@ovn.org", "To": "dev@openvswitch.org", "Date": "Wed, 10 Jun 2020 11:48:19 +0530", "Message-Id": "<20200610061819.1973526-1-numans@ovn.org>", "X-Mailer": "git-send-email 2.26.2", "In-Reply-To": "<20200610061726.1937231-1-numans@ovn.org>", "References": "<20200610061726.1937231-1-numans@ovn.org>", "MIME-Version": "1.0", "Subject": "[ovs-dev] [PATCH ovn v11 2/6] ovn-controller: I-P for ct zone and\n\tOVS interface changes in flow output stage.", "X-BeenThere": "ovs-dev@openvswitch.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "<ovs-dev.openvswitch.org>", "List-Unsubscribe": "<https://mail.openvswitch.org/mailman/options/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>", "List-Archive": "<http://mail.openvswitch.org/pipermail/ovs-dev/>", "List-Post": "<mailto:ovs-dev@openvswitch.org>", "List-Help": "<mailto:ovs-dev-request@openvswitch.org?subject=help>", "List-Subscribe": "<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "ovs-dev-bounces@openvswitch.org", "Sender": "\"dev\" <ovs-dev-bounces@openvswitch.org>" }, "content": "From: Numan Siddique <numans@ovn.org>\n\nThis patch handles ct zone changes and OVS interface changes incrementally\nin the flow output stage.\n\nAny changes to ct zone can be handled by running physical_run() instead of running\nflow_output_run(). And any changes to OVS interfaces can be either handled\nincrementally (for OVS interfaces representing VIFs) or just running\nphysical_run() (for tunnel and other types of interfaces).\n\nTo better handle this, a new engine node 'physical_flow_changes' is added which\nhandles changes to ct zone and OVS interfaces.\n\nSigned-off-by: Numan Siddique <numans@ovn.org>\n---\n controller/binding.c | 23 +-----\n controller/binding.h | 24 +++++-\n controller/ovn-controller.c | 145 +++++++++++++++++++++++++++++++++++-\n controller/physical.c | 51 +++++++++++++\n controller/physical.h | 5 +-\n 5 files changed, 223 insertions(+), 25 deletions(-)", "diff": "diff --git a/controller/binding.c b/controller/binding.c\nindex e79220ed5..61cdc8dbc 100644\n--- a/controller/binding.c\n+++ b/controller/binding.c\n@@ -502,7 +502,7 @@ remove_local_lport_ids(const struct sbrec_port_binding *pb,\n * 'struct local_binding' is used. A shash of these local bindings is\n * maintained with the 'external_ids:iface-id' as the key to the shash.\n *\n- * struct local_binding has 3 main fields:\n+ * struct local_binding (defined in binding.h) has 3 main fields:\n * - type\n * - OVS interface row object\n * - Port_Binding row object\n@@ -553,21 +553,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *pb,\n * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent\n * is bound to this chassis.\n */\n-enum local_binding_type {\n- BT_VIF,\n- BT_CONTAINER,\n- BT_VIRTUAL\n-};\n-\n-struct local_binding {\n- char *name;\n- enum local_binding_type type;\n- const struct ovsrec_interface *iface;\n- const struct sbrec_port_binding *pb;\n-\n- /* shash of 'struct local_binding' representing children. */\n- struct shash children;\n-};\n \n static struct local_binding *\n local_binding_create(const char *name, const struct ovsrec_interface *iface,\n@@ -589,12 +574,6 @@ local_binding_add(struct shash *local_bindings, struct local_binding *lbinding)\n shash_add(local_bindings, lbinding->name, lbinding);\n }\n \n-static struct local_binding *\n-local_binding_find(struct shash *local_bindings, const char *name)\n-{\n- return shash_find_data(local_bindings, name);\n-}\n-\n static void\n local_binding_destroy(struct local_binding *lbinding)\n {\ndiff --git a/controller/binding.h b/controller/binding.h\nindex f10c92bf9..e3d1f07de 100644\n--- a/controller/binding.h\n+++ b/controller/binding.h\n@@ -18,6 +18,7 @@\n #define OVN_BINDING_H 1\n \n #include <stdbool.h>\n+#include \"openvswitch/shash.h\"\n \n struct hmap;\n struct ovsdb_idl;\n@@ -32,7 +33,6 @@ struct sbrec_chassis;\n struct sbrec_port_binding_table;\n struct sset;\n struct sbrec_port_binding;\n-struct shash;\n \n struct binding_ctx_in {\n struct ovsdb_idl_txn *ovnsb_idl_txn;\n@@ -64,6 +64,28 @@ struct binding_ctx_out {\n struct smap *local_iface_ids;\n };\n \n+enum local_binding_type {\n+ BT_VIF,\n+ BT_CONTAINER,\n+ BT_VIRTUAL\n+};\n+\n+struct local_binding {\n+ char *name;\n+ enum local_binding_type type;\n+ const struct ovsrec_interface *iface;\n+ const struct sbrec_port_binding *pb;\n+\n+ /* shash of 'struct local_binding' representing children. */\n+ struct shash children;\n+};\n+\n+static inline struct local_binding *\n+local_binding_find(struct shash *local_bindings, const char *name)\n+{\n+ return shash_find_data(local_bindings, name);\n+}\n+\n void binding_register_ovs_idl(struct ovsdb_idl *);\n void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);\n bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,\ndiff --git a/controller/ovn-controller.c b/controller/ovn-controller.c\nindex bb82b15dc..a6bee1f76 100644\n--- a/controller/ovn-controller.c\n+++ b/controller/ovn-controller.c\n@@ -1361,8 +1361,13 @@ static void init_physical_ctx(struct engine_node *node,\n \n ovs_assert(br_int && chassis);\n \n+ struct ovsrec_interface_table *iface_table =\n+ (struct ovsrec_interface_table *)EN_OVSDB_GET(\n+ engine_get_input(\"OVS_interface\", node));\n+\n struct ed_type_ct_zones *ct_zones_data =\n engine_get_input_data(\"ct_zones\", node);\n+\n struct simap *ct_zones = &ct_zones_data->current;\n \n p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name;\n@@ -1370,12 +1375,14 @@ static void init_physical_ctx(struct engine_node *node,\n p_ctx->mc_group_table = multicast_group_table;\n p_ctx->br_int = br_int;\n p_ctx->chassis_table = chassis_table;\n+ p_ctx->iface_table = iface_table;\n p_ctx->chassis = chassis;\n p_ctx->active_tunnels = &rt_data->active_tunnels;\n p_ctx->local_datapaths = &rt_data->local_datapaths;\n p_ctx->local_lports = &rt_data->local_lports;\n p_ctx->ct_zones = ct_zones;\n p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;\n+ p_ctx->local_bindings = &rt_data->local_bindings;\n }\n \n static void init_lflow_ctx(struct engine_node *node,\n@@ -1783,6 +1790,125 @@ flow_output_port_groups_handler(struct engine_node *node, void *data)\n return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP);\n }\n \n+/* Engine node en_physical_flow_changes indicates whether\n+ * there is a need to\n+ * - recompute only physical flows or\n+ * - we can incrementally process the physical flows.\n+ *\n+ * en_physical_flow_changes is an input to flow_output engine node.\n+ * If the engine node 'en_physical_flow_changes' gets updated during\n+ * engine run, it means the handler for this -\n+ * flow_output_physical_flow_changes_handler() will either\n+ * - recompute the physical flows by calling 'physical_run() or\n+ * - incrementlly process some of the changes for physical flow\n+ * calculation. Right now we handle OVS interfaces changes\n+ * for physical flow computation.\n+ *\n+ * When ever a port binding happens, the follow up\n+ * activity is the zone id allocation for that port binding.\n+ * With this intermediate engine node, we avoid full recomputation.\n+ * Instead we do physical flow computation (either full recomputation\n+ * by calling physical_run() or handling the changes incrementally.\n+ *\n+ * Hence this is an intermediate engine node to indicate the\n+ * flow_output engine to recomputes/compute the physical flows.\n+ *\n+ * TODO 1. Ideally this engine node should recompute/compute the physical\n+ * flows instead of relegating it to the flow_output node.\n+ * But this requires splitting the flow_output node to\n+ * logical_flow_output and physical_flow_output.\n+ *\n+ * TODO 2. We can further optimise the en_ct_zone changes to\n+ * compute the phsyical flows for changed zone ids.\n+ *\n+ * TODO 3: physical.c has a global simap -localvif_to_ofport which stores the\n+ * local OVS interfaces and the ofport numbers. Ideally this should be\n+ * part of the engine data.\n+ */\n+struct ed_type_pfc_data {\n+ /* Both these variables are tracked and set in each engine run. */\n+ bool recompute_physical_flows;\n+ bool ovs_ifaces_changed;\n+};\n+\n+static void\n+en_physical_flow_changes_clear_tracked_data(void *data_)\n+{\n+ struct ed_type_pfc_data *data = data_;\n+ data->recompute_physical_flows = false;\n+ data->ovs_ifaces_changed = false;\n+}\n+\n+static void *\n+en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED,\n+ struct engine_arg *arg OVS_UNUSED)\n+{\n+ struct ed_type_pfc_data *data = xzalloc(sizeof *data);\n+ return data;\n+}\n+\n+static void\n+en_physical_flow_changes_cleanup(void *data OVS_UNUSED)\n+{\n+}\n+\n+/* Indicate to the flow_output engine that we need to recompute physical\n+ * flows. */\n+static void\n+en_physical_flow_changes_run(struct engine_node *node, void *data)\n+{\n+ struct ed_type_pfc_data *pfc_tdata = data;\n+ pfc_tdata->recompute_physical_flows = true;\n+ engine_set_node_state(node, EN_UPDATED);\n+}\n+\n+/* There are OVS interface changes. Indicate to the flow_output engine\n+ * to handle these OVS interface changes for physical flow computations. */\n+static bool\n+physical_flow_changes_ovs_iface_handler(struct engine_node *node, void *data)\n+{\n+ struct ed_type_pfc_data *pfc_tdata = data;\n+ pfc_tdata->ovs_ifaces_changed = true;\n+ engine_set_node_state(node, EN_UPDATED);\n+ return true;\n+}\n+\n+static bool\n+flow_output_physical_flow_changes_handler(struct engine_node *node, void *data)\n+{\n+ struct ed_type_runtime_data *rt_data =\n+ engine_get_input_data(\"runtime_data\", node);\n+\n+ struct ed_type_flow_output *fo = data;\n+ struct physical_ctx p_ctx;\n+ init_physical_ctx(node, rt_data, &p_ctx);\n+\n+ engine_set_node_state(node, EN_UPDATED);\n+ struct ed_type_pfc_data *pfc_data =\n+ engine_get_input_data(\"physical_flow_changes\", node);\n+\n+ if (pfc_data->recompute_physical_flows) {\n+ /* This indicates that we need to recompute the physical flows. */\n+ physical_run(&p_ctx, &fo->flow_table);\n+ return true;\n+ }\n+\n+ if (pfc_data->ovs_ifaces_changed) {\n+ /* There are OVS interface changes. Try to handle them\n+ * incrementally. */\n+ return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table);\n+ }\n+\n+ return true;\n+}\n+\n+static bool\n+flow_output_noop_handler(struct engine_node *node OVS_UNUSED,\n+ void *data OVS_UNUSED)\n+{\n+ return true;\n+}\n+\n struct ovn_controller_exit_args {\n bool *exiting;\n bool *restart;\n@@ -1907,6 +2033,8 @@ main(int argc, char *argv[])\n ENGINE_NODE(runtime_data, \"runtime_data\");\n ENGINE_NODE(mff_ovn_geneve, \"mff_ovn_geneve\");\n ENGINE_NODE(ofctrl_is_connected, \"ofctrl_is_connected\");\n+ ENGINE_NODE_WITH_CLEAR_TRACK_DATA(physical_flow_changes,\n+ \"physical_flow_changes\");\n ENGINE_NODE(flow_output, \"flow_output\");\n ENGINE_NODE(addr_sets, \"addr_sets\");\n ENGINE_NODE(port_groups, \"port_groups\");\n@@ -1926,13 +2054,28 @@ main(int argc, char *argv[])\n engine_add_input(&en_port_groups, &en_sb_port_group,\n port_groups_sb_port_group_handler);\n \n+ /* Engine node physical_flow_changes indicates whether\n+ * we can recompute only physical flows or we can\n+ * incrementally process the physical flows.\n+ */\n+ engine_add_input(&en_physical_flow_changes, &en_ct_zones,\n+ NULL);\n+ engine_add_input(&en_physical_flow_changes, &en_ovs_interface,\n+ physical_flow_changes_ovs_iface_handler);\n+\n engine_add_input(&en_flow_output, &en_addr_sets,\n flow_output_addr_sets_handler);\n engine_add_input(&en_flow_output, &en_port_groups,\n flow_output_port_groups_handler);\n engine_add_input(&en_flow_output, &en_runtime_data, NULL);\n- engine_add_input(&en_flow_output, &en_ct_zones, NULL);\n engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL);\n+ engine_add_input(&en_flow_output, &en_physical_flow_changes,\n+ flow_output_physical_flow_changes_handler);\n+\n+ /* We need this input nodes for only data. Hence the noop handler. */\n+ engine_add_input(&en_flow_output, &en_ct_zones, flow_output_noop_handler);\n+ engine_add_input(&en_flow_output, &en_ovs_interface,\n+ flow_output_noop_handler);\n \n engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL);\n engine_add_input(&en_flow_output, &en_ovs_bridge, NULL);\ndiff --git a/controller/physical.c b/controller/physical.c\nindex f06313b9d..240ec158e 100644\n--- a/controller/physical.c\n+++ b/controller/physical.c\n@@ -1780,6 +1780,57 @@ physical_run(struct physical_ctx *p_ctx,\n simap_destroy(&new_tunnel_to_ofport);\n }\n \n+bool\n+physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx,\n+ struct ovn_desired_flow_table *flow_table)\n+{\n+ const struct ovsrec_interface *iface_rec;\n+ OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) {\n+ if (!strcmp(iface_rec->type, \"geneve\") ||\n+ !strcmp(iface_rec->type, \"patch\")) {\n+ return false;\n+ }\n+ }\n+\n+ struct ofpbuf ofpacts;\n+ ofpbuf_init(&ofpacts, 0);\n+\n+ OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) {\n+ const char *iface_id = smap_get(&iface_rec->external_ids, \"iface-id\");\n+ if (!iface_id) {\n+ continue;\n+ }\n+\n+ const struct local_binding *lb =\n+ local_binding_find(p_ctx->local_bindings, iface_id);\n+\n+ if (!lb || !lb->pb) {\n+ continue;\n+ }\n+\n+ int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;\n+ if (ovsrec_interface_is_deleted(iface_rec)) {\n+ ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid);\n+ simap_find_and_delete(&localvif_to_ofport, iface_id);\n+ } else {\n+ if (!ovsrec_interface_is_new(iface_rec)) {\n+ ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid);\n+ }\n+\n+ simap_put(&localvif_to_ofport, iface_id, ofport);\n+ consider_port_binding(p_ctx->sbrec_port_binding_by_name,\n+ p_ctx->mff_ovn_geneve, p_ctx->ct_zones,\n+ p_ctx->active_tunnels,\n+ p_ctx->local_datapaths,\n+ lb->pb, p_ctx->chassis,\n+ flow_table, &ofpacts);\n+ }\n+ }\n+\n+ ofpbuf_uninit(&ofpacts);\n+ return true;\n+}\n+\n bool\n get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport)\n {\ndiff --git a/controller/physical.h b/controller/physical.h\nindex dadf84ea1..9ca34436a 100644\n--- a/controller/physical.h\n+++ b/controller/physical.h\n@@ -49,11 +49,13 @@ struct physical_ctx {\n const struct ovsrec_bridge *br_int;\n const struct sbrec_chassis_table *chassis_table;\n const struct sbrec_chassis *chassis;\n+ const struct ovsrec_interface_table *iface_table;\n const struct sset *active_tunnels;\n struct hmap *local_datapaths;\n struct sset *local_lports;\n const struct simap *ct_zones;\n enum mf_field_id mff_ovn_geneve;\n+ struct shash *local_bindings;\n };\n \n void physical_register_ovs_idl(struct ovsdb_idl *);\n@@ -63,7 +65,8 @@ void physical_handle_port_binding_changes(struct physical_ctx *,\n struct ovn_desired_flow_table *);\n void physical_handle_mc_group_changes(struct physical_ctx *,\n struct ovn_desired_flow_table *);\n-\n+bool physical_handle_ovs_iface_changes(struct physical_ctx *,\n+ struct ovn_desired_flow_table *);\n bool get_tunnel_ofport(const char *chassis_name, char *encap_ip,\n ofp_port_t *ofport);\n #endif /* controller/physical.h */\n", "prefixes": [ "ovs-dev", "ovn", "v11", "2/6" ] }