From patchwork Wed Sep 9 07:19:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1441555 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7D56ckHz9sRf for ; Thu, 18 Feb 2021 19:29:25 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id DE9D5605E3 for ; Thu, 18 Feb 2021 08:29:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Bzvqcg92mamk for ; Thu, 18 Feb 2021 08:29:22 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id 35B3B605E8; Thu, 18 Feb 2021 08:29:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id F0E6960591; Thu, 18 Feb 2021 08:29:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CB4DEC000E; Thu, 18 Feb 2021 08:29:15 +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 27D04C000D for ; Thu, 18 Feb 2021 08:29:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 16A7E80646 for ; Thu, 18 Feb 2021 08:29:15 +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 O-J3a806KHGh for ; Thu, 18 Feb 2021 08:29:14 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id AC0D18647A for ; Thu, 18 Feb 2021 08:28:55 +0000 (UTC) X-Mailbox-Line: From 22037c2c5c25aef29e4fd0e3d3fb88bc371c0d55 Mon Sep 17 00:00:00 2001 Message-Id: <22037c2c5c25aef29e4fd0e3d3fb88bc371c0d55.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Numan Siddique To: dev@openvswitch.org Date: Wed, 9 Sep 2020 12:49:39 +0530 Subject: [ovs-dev] [PATCH ovn branch-20.06 01/15] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb 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" when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without passing --detach and --monoitor and the process is exec'd. For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded to new schema. CMS has to handle the db upgrade separately. This patch handles the db upgrade by starting ovsdb-server in background and then waits on ovsdb-server to complete. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392 Acked-by: Mark Michelson Signed-off-by: Numan Siddique (cherry picked from commit 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c) Signed-off-by: Frode Nordahl --- utilities/ovn-ctl | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl index 8afe68a0a..ad8e6bb04 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -288,7 +288,21 @@ $cluster_remote_port set "$@" --sync-from=`cat $active_conf_file` fi - "$@" "$file" + local run_ovsdb_in_bg="no" + local process_id= + if test X$detach = Xno && test $mode = cluster && test -z "$cluster_remote_addr" ; then + # When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands) + # we want to run ovsdb-server in background rather than running it in + # foreground so that the OVN dbs are upgraded for the cluster mode. + # Otherwise, CMS has to take the responsibility of upgrading the dbs. + # Note: We run only the ovsdb-server in backgroud which created the + # cluster (i.e cluster_remote_addr is not set.). + run_ovsdb_in_bg="yes" + "$@" $file & + process_id=$! + else + start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file" + fi # Initialize the database if it's NOT joining a cluster. if test -z "$cluster_remote_addr"; then @@ -298,6 +312,10 @@ $cluster_remote_port if test $mode = cluster; then upgrade_cluster "$schema" "unix:$sock" fi + + if test $run_ovsdb_in_bg = yes; then + wait $process_id + fi } start_nb_ovsdb() { From patchwork Thu Aug 20 01:37:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441557 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7FV70V6z9sRf for ; Thu, 18 Feb 2021 19:30:38 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 3F55D605EE for ; Thu, 18 Feb 2021 08:30:37 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ET5gXPgIhoxk for ; Thu, 18 Feb 2021 08:30:32 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id DB49C605D8; Thu, 18 Feb 2021 08:30:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id D936260593; Thu, 18 Feb 2021 08:30:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BF77EC000E; Thu, 18 Feb 2021 08:30:15 +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 2452FC000D for ; Thu, 18 Feb 2021 08:30:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 19410866CA for ; Thu, 18 Feb 2021 08:30:14 +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 f4VZXBFKmfbJ for ; Thu, 18 Feb 2021 08:30:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id 7EAB7866B9 for ; Thu, 18 Feb 2021 08:29:34 +0000 (UTC) X-Mailbox-Line: From 0491154a19b2938edad7eff1fb4b0a5a51f39d2c Mon Sep 17 00:00:00 2001 Message-Id: <0491154a19b2938edad7eff1fb4b0a5a51f39d2c.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Wed, 19 Aug 2020 18:37:51 -0700 Subject: [ovs-dev] [PATCH ovn branch-20.06 02/15] ofctrl.c: Maintain references between installed flows and desired flows. 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" Currently there is no link maintained between installed flows and desired flows. This patch maintains the mapping between them, which will be useful for a future patch that incrementally processes the flow installation without having to do the full comparison between them. This patch also refactors the struct ovn_flow with two different wrapper types: desired_flow and installed_flow, and the related static functions, to make the code easier to read and avoid misuses of the struct. Acked-by: Mark Michelson Signed-off-by: Han Zhou (cherry picked from commit 354d3853d40cbce89a434632f67daed7fc992d8b) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 428 ++++++++++++++++++++++++++++++-------------- 1 file changed, 294 insertions(+), 134 deletions(-) sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) { @@ -706,7 +804,7 @@ sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) static void link_flow_to_sb(struct ovn_desired_flow_table *flow_table, - struct ovn_flow *f, const struct uuid *sb_uuid) + struct desired_flow *f, const struct uuid *sb_uuid) { struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); sfr->flow = f; @@ -744,26 +842,26 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, const struct uuid *sb_uuid, bool log_duplicate_flow) { - struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, - actions); + struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, + match, actions); - if (ovn_flow_lookup(&flow_table->match_flow_table, f, sb_uuid)) { + if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) { if (log_duplicate_flow) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); if (!VLOG_DROP_DBG(&rl)) { - char *s = ovn_flow_to_string(f); + char *s = ovn_flow_to_string(&f->flow); VLOG_DBG("dropping duplicate flow: %s", s); free(s); } } - ovn_flow_destroy(f); + desired_flow_destroy(f); return; } hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, - f->match_hmap_node.hash); + f->flow.hash); link_flow_to_sb(flow_table, f, sb_uuid); - ovn_flow_log(f, "ofctrl_add_flow"); + ovn_flow_log(&f->flow, "ofctrl_add_flow"); } void @@ -786,11 +884,11 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, const struct ofpbuf *actions, const struct uuid *sb_uuid) { - struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, - actions); + struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, + match, actions); - struct ovn_flow *existing; - existing = ovn_flow_lookup(&desired_flows->match_flow_table, f, NULL); + struct desired_flow *existing; + existing = desired_flow_lookup(desired_flows, &f->flow, NULL); if (existing) { /* There's already a flow with this particular match. Append the * action to that flow rather than adding a new flow @@ -798,26 +896,27 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, uint64_t compound_stub[64 / 8]; struct ofpbuf compound; ofpbuf_use_stub(&compound, compound_stub, sizeof(compound_stub)); - ofpbuf_put(&compound, existing->ofpacts, existing->ofpacts_len); - ofpbuf_put(&compound, f->ofpacts, f->ofpacts_len); + ofpbuf_put(&compound, existing->flow.ofpacts, + existing->flow.ofpacts_len); + ofpbuf_put(&compound, f->flow.ofpacts, f->flow.ofpacts_len); - free(existing->ofpacts); - existing->ofpacts = xmemdup(compound.data, compound.size); - existing->ofpacts_len = compound.size; + free(existing->flow.ofpacts); + existing->flow.ofpacts = xmemdup(compound.data, compound.size); + existing->flow.ofpacts_len = compound.size; ofpbuf_uninit(&compound); - ovn_flow_destroy(f); + desired_flow_destroy(f); f = existing; } else { hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node, - f->match_hmap_node.hash); + f->flow.hash); } link_flow_to_sb(desired_flows, f, sb_uuid); if (existing) { - ovn_flow_log(f, "ofctrl_add_or_append_flow (append)"); + ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (append)"); } else { - ovn_flow_log(f, "ofctrl_add_or_append_flow (add)"); + ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (add)"); } } @@ -833,16 +932,19 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { ovs_list_remove(&sfr->sb_list); ovs_list_remove(&sfr->flow_list); - struct ovn_flow *f = sfr->flow; + struct desired_flow *f = sfr->flow; free(sfr); if (ovs_list_is_empty(&f->references)) { if (log_msg) { - ovn_flow_log(f, log_msg); + ovn_flow_log(&f->flow, log_msg); } hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - ovn_flow_destroy(f); + if (f->installed_flow) { + unlink_installed_to_desired(f->installed_flow, f); + } + desired_flow_destroy(f); } } hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); @@ -903,8 +1005,8 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, /* Traverse all flows for the given sb_uuid. */ struct sb_flow_ref *sfr, *next; LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { - struct ovn_flow *f = sfr->flow; - ovn_flow_log(f, "flood remove"); + struct desired_flow *f = sfr->flow; + ovn_flow_log(&f->flow, "flood remove"); ovs_list_remove(&sfr->sb_list); ovs_list_remove(&sfr->flow_list); @@ -917,7 +1019,10 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, * be empty in most cases. */ hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - ovn_flow_destroy(f); + if (f->installed_flow) { + unlink_installed_to_desired(f->installed_flow, f); + } + desired_flow_destroy(f); } else { ovs_list_insert(&to_be_removed, &f->list_node); } @@ -930,7 +1035,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, /* Detach the items in f->references from the sfr.flow_list lists, * so that recursive calls will not mess up the sfr.sb_list list. */ - struct ovn_flow *f, *f_next; + struct desired_flow *f, *f_next; LIST_FOR_EACH (f, list_node, &to_be_removed) { ovs_assert(!ovs_list_is_empty(&f->references)); LIST_FOR_EACH (sfr, sb_list, &f->references) { @@ -951,7 +1056,10 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, ovs_list_remove(&f->list_node); hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - ovn_flow_destroy(f); + if (f->installed_flow) { + unlink_installed_to_desired(f->installed_flow, f); + } + desired_flow_destroy(f); } } @@ -973,22 +1081,32 @@ ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, } } -/* ovn_flow. */ +/* flow operations. */ -static struct ovn_flow * -ovn_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, - const struct match *match, const struct ofpbuf *actions) +static void +ovn_flow_init(struct ovn_flow *f, uint8_t table_id, uint16_t priority, + uint64_t cookie, const struct match *match, + const struct ofpbuf *actions) { - struct ovn_flow *f = xmalloc(sizeof *f); - ovs_list_init(&f->references); - ovs_list_init(&f->list_node); f->table_id = table_id; f->priority = priority; minimatch_init(&f->match, match); f->ofpacts = xmemdup(actions->data, actions->size); f->ofpacts_len = actions->size; - f->match_hmap_node.hash = ovn_flow_match_hash(f); + f->hash = ovn_flow_match_hash(f); f->cookie = cookie; +} + +static struct desired_flow * +desired_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, + const struct match *match, const struct ofpbuf *actions) +{ + struct desired_flow *f = xmalloc(sizeof *f); + ovs_list_init(&f->references); + ovs_list_init(&f->list_node); + ovs_list_init(&f->installed_ref_list_node); + f->installed_flow = NULL; + ovn_flow_init(&f->flow, table_id, priority, cookie, match, actions); return f; } @@ -1001,48 +1119,47 @@ ovn_flow_match_hash(const struct ovn_flow *f) minimatch_hash(&f->match, 0)); } -/* Duplicate an ovn_flow structure. */ -static struct ovn_flow * -ovn_flow_dup(struct ovn_flow *src) +/* Duplicate a desired flow to an installed flow. */ +static struct installed_flow * +installed_flow_dup(struct desired_flow *src) { - struct ovn_flow *dst = xmalloc(sizeof *dst); - ovs_list_init(&dst->references); - dst->table_id = src->table_id; - dst->priority = src->priority; - minimatch_clone(&dst->match, &src->match); - dst->ofpacts = xmemdup(src->ofpacts, src->ofpacts_len); - dst->ofpacts_len = src->ofpacts_len; - dst->match_hmap_node.hash = src->match_hmap_node.hash; - dst->cookie = src->cookie; + struct installed_flow *dst = xmalloc(sizeof *dst); + ovs_list_init(&dst->desired_refs); + dst->desired_flow = NULL; + dst->flow.table_id = src->flow.table_id; + dst->flow.priority = src->flow.priority; + minimatch_clone(&dst->flow.match, &src->flow.match); + dst->flow.ofpacts = xmemdup(src->flow.ofpacts, src->flow.ofpacts_len); + dst->flow.ofpacts_len = src->flow.ofpacts_len; + dst->flow.hash = src->flow.hash; + dst->flow.cookie = src->flow.cookie; return dst; } -/* Finds and returns an ovn_flow in 'flow_table' whose key is identical to +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to * 'target''s key, or NULL if there is none. * * If sb_uuid is not NULL, the function will also check if the found flow is - * referenced by the sb_uuid. - * - * NOTE: sb_uuid can only be used for ovn_desired_flow_table lookup. */ -static struct ovn_flow * -ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, - const struct uuid *sb_uuid) + * referenced by the sb_uuid. */ +static struct desired_flow * +desired_flow_lookup(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target, + const struct uuid *sb_uuid) { - struct ovn_flow *f; - - HMAP_FOR_EACH_WITH_HASH (f, match_hmap_node, target->match_hmap_node.hash, - flow_table) { + struct desired_flow *d; + HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, + &flow_table->match_flow_table) { + struct ovn_flow *f = &d->flow; if (f->table_id == target->table_id && f->priority == target->priority && minimatch_equal(&f->match, &target->match)) { if (!sb_uuid) { - return f; + return d; } - ovs_assert(flow_table != &installed_flows); struct sb_flow_ref *sfr; - LIST_FOR_EACH (sfr, sb_list, &f->references) { + LIST_FOR_EACH (sfr, sb_list, &d->references) { if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { - return f; + return d; } } } @@ -1050,6 +1167,24 @@ ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, return NULL; } +/* Finds and returns an installed_flow in installed_flows whose key is + * identical to 'target''s key, or NULL if there is none. */ +static struct installed_flow * +installed_flow_lookup(const struct ovn_flow *target) +{ + struct installed_flow *i; + HMAP_FOR_EACH_WITH_HASH (i, match_hmap_node, target->hash, + &installed_flows) { + struct ovn_flow *f = &i->flow; + if (f->table_id == target->table_id + && f->priority == target->priority + && minimatch_equal(&f->match, &target->match)) { + return i; + } + } + return NULL; +} + static char * ovn_flow_to_string(const struct ovn_flow *f) { @@ -1076,17 +1211,35 @@ ovn_flow_log(const struct ovn_flow *f, const char *action) } static void -ovn_flow_destroy(struct ovn_flow *f) +ovn_flow_uninit(struct ovn_flow *f) +{ + minimatch_destroy(&f->match); + free(f->ofpacts); +} + +static void +desired_flow_destroy(struct desired_flow *f) { if (f) { ovs_assert(ovs_list_is_empty(&f->references)); - minimatch_destroy(&f->match); - free(f->ofpacts); + ovs_assert(!f->installed_flow); + ovn_flow_uninit(&f->flow); + free(f); + } +} + +static void +installed_flow_destroy(struct installed_flow *f) +{ + if (f) { + ovs_assert(ovs_list_is_empty(&f->desired_refs)); + ovs_assert(!f->desired_flow); + ovn_flow_uninit(&f->flow); free(f); } } -/* Flow tables of struct ovn_flow. */ +/* Desired flow table operations. */ void ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) { @@ -1112,13 +1265,16 @@ ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *flow_table) hmap_destroy(&flow_table->uuid_flow_table); } + +/* Installed flow table operations. */ static void ovn_installed_flow_table_clear(void) { - struct ovn_flow *f, *next; + struct installed_flow *f, *next; HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_flows) { hmap_remove(&installed_flows, &f->match_hmap_node); - ovn_flow_destroy(f); + unlink_all_refs_for_installed_flow(f); + installed_flow_destroy(f); } } @@ -1442,81 +1598,85 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, /* Iterate through all of the installed flows. If any of them are no * longer desired, delete them; if any of them should have different * actions, update them. */ - struct ovn_flow *i, *next; + struct installed_flow *i, *next; HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { - struct ovn_flow *d = ovn_flow_lookup(&flow_table->match_flow_table, - i, NULL); + unlink_all_refs_for_installed_flow(i); + struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow, + NULL); if (!d) { /* Installed flow is no longer desirable. Delete it from the * switch and from installed_flows. */ struct ofputil_flow_mod fm = { - .match = i->match, - .priority = i->priority, - .table_id = i->table_id, + .match = i->flow.match, + .priority = i->flow.priority, + .table_id = i->flow.table_id, .command = OFPFC_DELETE_STRICT, }; add_flow_mod(&fm, &msgs); - ovn_flow_log(i, "removing installed"); + ovn_flow_log(&i->flow, "removing installed"); hmap_remove(&installed_flows, &i->match_hmap_node); - ovn_flow_destroy(i); + installed_flow_destroy(i); } else { - if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, - d->ofpacts, d->ofpacts_len) || - i->cookie != d->cookie) { + if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, + d->flow.ofpacts, d->flow.ofpacts_len) || + i->flow.cookie != d->flow.cookie) { /* Update actions in installed flow. */ struct ofputil_flow_mod fm = { - .match = i->match, - .priority = i->priority, - .table_id = i->table_id, - .ofpacts = d->ofpacts, - .ofpacts_len = d->ofpacts_len, + .match = i->flow.match, + .priority = i->flow.priority, + .table_id = i->flow.table_id, + .ofpacts = d->flow.ofpacts, + .ofpacts_len = d->flow.ofpacts_len, .command = OFPFC_MODIFY_STRICT, }; /* Update cookie if it is changed. */ - if (i->cookie != d->cookie) { + if (i->flow.cookie != d->flow.cookie) { fm.modify_cookie = true; - fm.new_cookie = htonll(d->cookie); + fm.new_cookie = htonll(d->flow.cookie); /* Use OFPFC_ADD so that cookie can be updated. */ fm.command = OFPFC_ADD, - i->cookie = d->cookie; + i->flow.cookie = d->flow.cookie; } add_flow_mod(&fm, &msgs); - ovn_flow_log(i, "updating installed"); + ovn_flow_log(&i->flow, "updating installed"); /* Replace 'i''s actions by 'd''s. */ - free(i->ofpacts); - i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len); - i->ofpacts_len = d->ofpacts_len; + free(i->flow.ofpacts); + i->flow.ofpacts = xmemdup(d->flow.ofpacts, + d->flow.ofpacts_len); + i->flow.ofpacts_len = d->flow.ofpacts_len; } + link_installed_to_desired(i, d); } } /* Iterate through the desired flows and add those that aren't found * in the installed flow table. */ - struct ovn_flow *d; + struct desired_flow *d; HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { - i = ovn_flow_lookup(&installed_flows, d, NULL); + i = installed_flow_lookup(&d->flow); if (!i) { /* Send flow_mod to add flow. */ struct ofputil_flow_mod fm = { - .match = d->match, - .priority = d->priority, - .table_id = d->table_id, - .ofpacts = d->ofpacts, - .ofpacts_len = d->ofpacts_len, - .new_cookie = htonll(d->cookie), + .match = d->flow.match, + .priority = d->flow.priority, + .table_id = d->flow.table_id, + .ofpacts = d->flow.ofpacts, + .ofpacts_len = d->flow.ofpacts_len, + .new_cookie = htonll(d->flow.cookie), .command = OFPFC_ADD, }; add_flow_mod(&fm, &msgs); - ovn_flow_log(d, "adding installed"); + ovn_flow_log(&d->flow, "adding installed"); /* Copy 'd' from 'flow_table' to installed_flows. */ - struct ovn_flow *new_node = ovn_flow_dup(d); - hmap_insert(&installed_flows, &new_node->match_hmap_node, - new_node->match_hmap_node.hash); + i = installed_flow_dup(d); + hmap_insert(&installed_flows, &i->match_hmap_node, + i->flow.hash); } + link_installed_to_desired(i, d); } /* Iterate through the installed groups from previous runs. If they diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 11832b8d7..a28c6b759 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -51,7 +51,27 @@ VLOG_DEFINE_THIS_MODULE(ofctrl); -/* An OpenFlow flow. +/* An OpenFlow flow. */ +struct ovn_flow { + /* Key. */ + uint8_t table_id; + uint16_t priority; + struct minimatch match; + + /* Hash. */ + uint32_t hash; + + /* Data. */ + struct ofpact *ofpacts; + size_t ofpacts_len; + uint64_t cookie; +}; + +/* A desired flow, in struct ovn_desired_flow_table, calculated by the + * incremental processing engine. + * - They are added/removed incrementally when I-P engine is able to process + * the changes incrementally, or + * - Completely cleared and recomputed by I-P engine when recompute happens. * * Links are maintained between desired flows and SB data. The relationship * is M to N. The struct sb_flow_ref is used to link a pair of desired flow @@ -82,52 +102,92 @@ VLOG_DEFINE_THIS_MODULE(ofctrl); * The links are updated whenever there is a change in desired flows, which is * usually triggered by a SB data change in I-P engine. */ -struct ovn_flow { +struct desired_flow { + struct ovn_flow flow; struct hmap_node match_hmap_node; /* For match based hashing. */ struct ovs_list list_node; /* For handling lists of flows. */ - struct ovs_list references; /* A list of struct sb_flow_ref nodes, which - references this flow. (There are cases - that multiple SB entities share the same - desired OpenFlow flow, e.g. when - conjunction is used.) */ - /* Key. */ - uint8_t table_id; - uint16_t priority; - struct minimatch match; + /* A list of struct sb_flow_ref nodes, which references this flow. (There + * are cases that multiple SB entities share the same desired OpenFlow + * flow, e.g. when conjunction is used.) */ + struct ovs_list references; - /* Data. */ - struct ofpact *ofpacts; - size_t ofpacts_len; - uint64_t cookie; + /* The corresponding flow in installed table. */ + struct installed_flow *installed_flow; + + /* Node in installed_flow.desired_refs list. */ + struct ovs_list installed_ref_list_node; }; struct sb_to_flow { struct hmap_node hmap_node; /* Node in ovn_desired_flow_table.uuid_flow_table. */ struct uuid sb_uuid; - struct ovs_list flows; /* A list of struct sb_flow_ref nodes that are - referenced by the sb_uuid. */ + struct ovs_list flows; /* A list of struct sb_flow_ref nodes that + are referenced by the sb_uuid. */ }; struct sb_flow_ref { - struct ovs_list sb_list; /* List node in ovn_flow.references. */ - struct ovs_list flow_list; /* List node in sb_to_flow.ovn_flows. */ - struct ovn_flow *flow; + struct ovs_list sb_list; /* List node in desired_flow.references. */ + struct ovs_list flow_list; /* List node in sb_to_flow.desired_flows. */ + struct desired_flow *flow; struct uuid sb_uuid; }; -static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t priority, - uint64_t cookie, - const struct match *match, - const struct ofpbuf *actions); +/* A installed flow, in static variable installed_flows. + * + * Installed flows are updated in ofctrl_put for maintaining the flow + * installation to OVS. They are updated according to desired flows: either by + * processing the tracked desired flow changes, or by comparing desired flows + * with currently installed flows when tracked desired flows changes are not + * available. + * + * In addition, when ofctrl state machine enters S_CLEAR, the installed flows + * will be cleared. (This happens in initialization phase and also when + * ovs-vswitchd is disconnected/reconnected). + * + * Links are maintained between installed flows and desired flows. The + * relationship is 1 to N. A link is added when a flow addition is processed. + * A link is removed when a flow deletion is processed, the desired flow + * table is cleared, or the installed flow table is cleared. + */ +struct installed_flow { + struct ovn_flow flow; + struct hmap_node match_hmap_node; /* For match based hashing. */ + + /* A list of desired ovn_flow nodes (linked by + * desired_flow.installed_ref_list_node), which reference this installed + * flow. (There are cases that multiple desired flows reference the same + * installed flow, e.g. when there are conflict/duplicated ACLs that + * generates same match conditions). */ + struct ovs_list desired_refs; + + /* The corresponding flow in desired table. It must be one of the flows in + * desired_refs list. If there are more than one flows in references list, + * this is the one that is actually installed. */ + struct desired_flow *desired_flow; +}; + +static struct desired_flow *desired_flow_alloc( + uint8_t table_id, + uint16_t priority, + uint64_t cookie, + const struct match *match, + const struct ofpbuf *actions); +static struct desired_flow *desired_flow_lookup( + struct ovn_desired_flow_table *, + const struct ovn_flow *target, + const struct uuid *sb_uuid); +static void desired_flow_destroy(struct desired_flow *); + +static struct installed_flow *installed_flow_lookup( + const struct ovn_flow *target); +static void installed_flow_destroy(struct installed_flow *); +static struct installed_flow *installed_flow_dup(struct desired_flow *); + static uint32_t ovn_flow_match_hash(const struct ovn_flow *); -static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table, - const struct ovn_flow *target, - const struct uuid *sb_uuid); static char *ovn_flow_to_string(const struct ovn_flow *); static void ovn_flow_log(const struct ovn_flow *, const char *action); -static void ovn_flow_destroy(struct ovn_flow *); /* OpenFlow connection to the switch. */ static struct rconn *swconn; @@ -216,7 +276,6 @@ static struct ofpbuf *encode_meter_mod(const struct ofputil_meter_mod *); static void ovn_installed_flow_table_clear(void); static void ovn_installed_flow_table_destroy(void); -static struct ovn_flow *ovn_flow_dup(struct ovn_flow *source); static void ofctrl_recv(const struct ofp_header *, enum ofptype); @@ -691,6 +750,45 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } +static void +link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) +{ + if (i->desired_flow == d) { + return; + } + + if (ovs_list_is_empty(&i->desired_refs)) { + ovs_assert(!i->desired_flow); + i->desired_flow = d; + } + ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); + d->installed_flow = i; +} + +static void +unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) +{ + ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); + ovs_assert(d && d->installed_flow == i); + ovs_list_remove(&d->installed_ref_list_node); + d->installed_flow = NULL; + if (i->desired_flow == d) { + i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : + CONTAINER_OF(ovs_list_front(&i->desired_refs), + struct desired_flow, + installed_ref_list_node); + } +} + +static void +unlink_all_refs_for_installed_flow(struct installed_flow *i) +{ + struct desired_flow *d, *next; + LIST_FOR_EACH_SAFE (d, next, installed_ref_list_node, &i->desired_refs) { + unlink_installed_to_desired(i, d); + } +} + static struct sb_to_flow * From patchwork Fri Aug 21 04:38:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441558 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.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7G53cMSz9sRf for ; Thu, 18 Feb 2021 19:31:09 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D000286CF4; Thu, 18 Feb 2021 08:31:07 +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 NcZ+6i6B1U9O; Thu, 18 Feb 2021 08:31:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 2350E86974; Thu, 18 Feb 2021 08:31:05 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 09535C000E; Thu, 18 Feb 2021 08:31:05 +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 24409C000D for ; Thu, 18 Feb 2021 08:31:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 0D12A866B9 for ; Thu, 18 Feb 2021 08:31:03 +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 ChuLVBm1cOo7 for ; Thu, 18 Feb 2021 08:31:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id F25E186AEB for ; Thu, 18 Feb 2021 08:30:40 +0000 (UTC) X-Mailbox-Line: From 5dc588d5fd1c0a9cc62d286dc6c2e01b1f0d1f3a Mon Sep 17 00:00:00 2001 Message-Id: <5dc588d5fd1c0a9cc62d286dc6c2e01b1f0d1f3a.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Thu, 20 Aug 2020 21:38:30 -0700 Subject: [ovs-dev] [PATCH ovn branch-20.06 03/15] ofctrl.c: Refactor - move openflow msg construction to functions. 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" These functions will be reused in multiple places in a future patch. Acked-by: Mark Michelson Signed-off-by: Han Zhou (cherry picked from commit 23063cf4178c05f5d6b3e4ec6d323ccc88df6101) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 103 ++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index a28c6b759..98e291243 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1481,6 +1481,63 @@ add_meter(struct ovn_extend_table_info *m_desired, free(mm.meter.bands); } +static void +installed_flow_add(struct ovn_flow *d, struct ovs_list *msgs) +{ + /* Send flow_mod to add flow. */ + struct ofputil_flow_mod fm = { + .match = d->match, + .priority = d->priority, + .table_id = d->table_id, + .ofpacts = d->ofpacts, + .ofpacts_len = d->ofpacts_len, + .new_cookie = htonll(d->cookie), + .command = OFPFC_ADD, + }; + add_flow_mod(&fm, msgs); +} + +static void +installed_flow_mod(struct ovn_flow *i, struct ovn_flow *d, + struct ovs_list *msgs) +{ + /* Update actions in installed flow. */ + struct ofputil_flow_mod fm = { + .match = i->match, + .priority = i->priority, + .table_id = i->table_id, + .ofpacts = d->ofpacts, + .ofpacts_len = d->ofpacts_len, + .command = OFPFC_MODIFY_STRICT, + }; + /* Update cookie if it is changed. */ + if (i->cookie != d->cookie) { + fm.modify_cookie = true; + fm.new_cookie = htonll(d->cookie); + /* Use OFPFC_ADD so that cookie can be updated. */ + fm.command = OFPFC_ADD; + } + add_flow_mod(&fm, msgs); + + /* Replace 'i''s actions and cookie by 'd''s. */ + free(i->ofpacts); + i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len); + i->ofpacts_len = d->ofpacts_len; + i->cookie = d->cookie; +} + +static void +installed_flow_del(struct ovn_flow *i, struct ovs_list *msgs) +{ + struct ofputil_flow_mod fm = { + .match = i->match, + .priority = i->priority, + .table_id = i->table_id, + .command = OFPFC_DELETE_STRICT, + }; + add_flow_mod(&fm, msgs); +} + /* The flow table can be updated if the connection to the switch is up and * in the correct state and not backlogged with existing flow_mods. (Our * criteria for being backlogged appear very conservative, but the socket @@ -1606,46 +1663,16 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, if (!d) { /* Installed flow is no longer desirable. Delete it from the * switch and from installed_flows. */ - struct ofputil_flow_mod fm = { - .match = i->flow.match, - .priority = i->flow.priority, - .table_id = i->flow.table_id, - .command = OFPFC_DELETE_STRICT, - }; - add_flow_mod(&fm, &msgs); + installed_flow_del(&i->flow, &msgs); ovn_flow_log(&i->flow, "removing installed"); - hmap_remove(&installed_flows, &i->match_hmap_node); installed_flow_destroy(i); } else { if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, d->flow.ofpacts, d->flow.ofpacts_len) || i->flow.cookie != d->flow.cookie) { - /* Update actions in installed flow. */ - struct ofputil_flow_mod fm = { - .match = i->flow.match, - .priority = i->flow.priority, - .table_id = i->flow.table_id, - .ofpacts = d->flow.ofpacts, - .ofpacts_len = d->flow.ofpacts_len, - .command = OFPFC_MODIFY_STRICT, - }; - /* Update cookie if it is changed. */ - if (i->flow.cookie != d->flow.cookie) { - fm.modify_cookie = true; - fm.new_cookie = htonll(d->flow.cookie); - /* Use OFPFC_ADD so that cookie can be updated. */ - fm.command = OFPFC_ADD, - i->flow.cookie = d->flow.cookie; - } - add_flow_mod(&fm, &msgs); ovn_flow_log(&i->flow, "updating installed"); - - /* Replace 'i''s actions by 'd''s. */ - free(i->flow.ofpacts); - i->flow.ofpacts = xmemdup(d->flow.ofpacts, - d->flow.ofpacts_len); - i->flow.ofpacts_len = d->flow.ofpacts_len; + installed_flow_mod(&i->flow, &d->flow, &msgs); } link_installed_to_desired(i, d); @@ -1658,17 +1685,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { i = installed_flow_lookup(&d->flow); if (!i) { - /* Send flow_mod to add flow. */ - struct ofputil_flow_mod fm = { - .match = d->flow.match, - .priority = d->flow.priority, - .table_id = d->flow.table_id, - .ofpacts = d->flow.ofpacts, - .ofpacts_len = d->flow.ofpacts_len, - .new_cookie = htonll(d->flow.cookie), - .command = OFPFC_ADD, - }; - add_flow_mod(&fm, &msgs); + installed_flow_add(&d->flow, &msgs); ovn_flow_log(&d->flow, "adding installed"); /* Copy 'd' from 'flow_table' to installed_flows. */ From patchwork Mon Aug 17 22:49:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441559 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=) 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 4Dh7H60SWmz9sRf for ; Thu, 18 Feb 2021 19:32:02 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 82E448613E; Thu, 18 Feb 2021 08:32:00 +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 uHeS1JPNYrQV; Thu, 18 Feb 2021 08:31:59 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0413185F90; Thu, 18 Feb 2021 08:31:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D9720C000E; Thu, 18 Feb 2021 08:31:58 +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 6BB7EC000D for ; Thu, 18 Feb 2021 08:31:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 5A71C86974 for ; Thu, 18 Feb 2021 08:31:57 +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 aHZW-zWOb7xo for ; Thu, 18 Feb 2021 08:31:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id 8BC1E866CA for ; Thu, 18 Feb 2021 08:31:29 +0000 (UTC) X-Mailbox-Line: From 0ad2c2b617d791fa17f2188cfa74c372dd536db2 Mon Sep 17 00:00:00 2001 Message-Id: <0ad2c2b617d791fa17f2188cfa74c372dd536db2.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Mon, 17 Aug 2020 15:49:18 -0700 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn branch-20.06 04/15] ofctrl: Incremental processing for flow installation by tracking. 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" With incremental processing for flow computation, the bottle neck of ovn-controller in large scale environment is in the flow installation (ofctrl_put()), which does full comparison between the two big flow tables: the installed flows and desired flows. This patch implements tracking desired flow changes when flows are incrementally computed by I-P engine, and then incrementally processing the flow installation using the tracked information in ofctrl_put(). It falls back to the full comparison whenever tracking information is unavailable, e.g. when I-P engine triggers full recompute. In ovn-scale-test with 3000 HVs and 30k lports, the end-to-end latency between the moment a lflow is updated in SB DB and the moment when all the 3K HVs complete OVS flow updating has reduced around 60% (from 1s to 400ms). Below is the perf result for a ovn-controller processing a new port-binding: Beore: + 96.76% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff + 90.21% 0.00% ovn-controller ovn-controller [.] main + 39.93% 1.19% ovn-controller ovn-controller [.] ofctrl_put + 31.27% 12.47% ovn-controller ovn-controller [.] ovn_flow_lookup + 22.12% 3.12% ovn-controller ovn-controller [.] encaps_run + 18.69% 2.77% ovn-controller ovn-controller [.] minimatch_equal + 17.63% 4.11% ovn-controller ovn-controller [.] patch_run + 15.91% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined) + 14.03% 12.08% ovn-controller ovn-controller [.] minimask_equal + 12.41% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined) + 11.40% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined) After: + 94.59% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff + 83.56% 0.09% ovn-controller ovn-controller [.] main + 35.37% 3.13% ovn-controller ovn-controller [.] encaps_run + 27.54% 7.53% ovn-controller ovn-controller [.] patch_run + 24.86% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined) + 20.01% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined) + 18.51% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined) + 14.08% 0.17% ovn-controller ovn-controller [.] physical_run + 11.37% 11.28% ovn-controller ovn-controller [.] next_real_row + 10.50% 2.59% ovn-controller ovn-controller [.] consider_port_binding .. + 2.14% 0.32% ovn-controller ovn-controller [.] ofctrl_put ▒ Before the optimization, ofctrl_put took 40% of CPU, and now it disappears from the hot spots. Acked-by: Mark Michelson Signed-off-by: Han Zhou (cherry picked from commit 6f0b1e02d9ab3a94048c4818f2d382938cad4b71) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 289 +++++++++++++++++++++++++++++++++++--------- controller/ofctrl.h | 6 +- 2 files changed, 240 insertions(+), 55 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 98e291243..b0670bf44 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -101,6 +101,39 @@ struct ovn_flow { * * The links are updated whenever there is a change in desired flows, which is * usually triggered by a SB data change in I-P engine. + * + * ** Tracking ** + * + * A desired flow can be tracked - listed in ovn_desired_flow_table's + * tracked_flows. + * + * Tracked flows is initially empty, and stays empty after the first run of I-P + * engine when installed flows are initially populated. After that, flow + * changes are tracked when I-P engine incrementally computes flow changes. + * Tracked flows are then processed and removed completely in ofctrl_put. + * ("processed" means OpenFlow change messages are composed and sent/queued to + * OVS, which ensures flows in OVS is always in sync (eventually) with the + * installed flows table). + * + * In case of full recompute of I-P engine, tracked flows are not + * added/removed, and ofctrl_put will not rely on tracked flows. (It is I-P + * engine's responsibility to ensure the tracked flows are cleared before + * recompute). + * + * Tracked flows can be preserved across multiple I-P engine runs - if in some + * iterations ofctrl_put() is skipped. Tracked flows are cleared only when it + * is consumed or when flow recompute happens. + * + * The "change_tracked" member of desired flow table maintains the status of + * whether flow changes are tracked or not. It is always set to true when + * ofctrl_put is completed, and transition to false whenever + * ovn_desired_flow_table_clear is called. + * + * NOTE: A tracked flow is just a reference to a desired flow, instead of a new + * copy. When a desired flow is removed and tracked, it is removed from the + * match_flow_table and uuid_flow_table indexes, and added to the tracked_flows + * list, marking is_deleted = true, but not immediately destroyed. It is + * destroyed when the tracking is processed for installed flow updates. */ struct desired_flow { struct ovn_flow flow; @@ -117,6 +150,11 @@ struct desired_flow { /* Node in installed_flow.desired_refs list. */ struct ovs_list installed_ref_list_node; + + /* For tracking. */ + struct ovs_list track_list_node; /* node in ovn_desired_flow_table's + * tracked_flows list. */ + bool is_deleted; /* If the tracked flow is deleted. */ }; struct sb_to_flow { @@ -789,6 +827,62 @@ unlink_all_refs_for_installed_flow(struct installed_flow *i) } } +static void +track_flow_add_or_modify(struct ovn_desired_flow_table *flow_table, + struct desired_flow *f) +{ + if (!flow_table->change_tracked) { + return; + } + + /* If same node (flow adding/modifying) was tracked, remove it from + * tracking first. */ + if (!ovs_list_is_empty(&f->track_list_node)) { + ovs_list_remove(&f->track_list_node); + } + f->is_deleted = false; + ovs_list_push_back(&flow_table->tracked_flows, &f->track_list_node); + +} + +static void +track_flow_del(struct ovn_desired_flow_table *flow_table, + struct desired_flow *f) +{ + if (!flow_table->change_tracked) { + return; + } + /* If same node (flow adding/modifying) was tracked, remove it from + * tracking first. */ + if (!ovs_list_is_empty(&f->track_list_node)) { + ovs_list_remove(&f->track_list_node); + if (!f->installed_flow) { + /* If it is not installed yet, simply destroy it. */ + desired_flow_destroy(f); + return; + } + } + f->is_deleted = true; + ovs_list_push_back(&flow_table->tracked_flows, &f->track_list_node); +} + +/* When a desired flow is being removed, depending on "change_tracked", this + * function either unlinks a desired flow from installed flow and destroy it, + * or do nothing but track it. */ +static void +track_or_destroy_for_flow_del(struct ovn_desired_flow_table *flow_table, + struct desired_flow *f) +{ + if (flow_table->change_tracked) { + track_flow_del(flow_table, f); + } else { + if (f->installed_flow) { + unlink_installed_to_desired(f->installed_flow, f); + } + desired_flow_destroy(f); + } +} + static struct sb_to_flow * sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) { @@ -861,6 +955,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, f->flow.hash); link_flow_to_sb(flow_table, f, sb_uuid); + track_flow_add_or_modify(flow_table, f); ovn_flow_log(&f->flow, "ofctrl_add_flow"); } @@ -912,6 +1007,7 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, f->flow.hash); } link_flow_to_sb(desired_flows, f, sb_uuid); + track_flow_add_or_modify(desired_flows, f); if (existing) { ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (append)"); @@ -941,10 +1037,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, } hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - if (f->installed_flow) { - unlink_installed_to_desired(f->installed_flow, f); - } - desired_flow_destroy(f); + track_or_destroy_for_flow_del(flow_table, f); } } hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); @@ -1019,10 +1112,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, * be empty in most cases. */ hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - if (f->installed_flow) { - unlink_installed_to_desired(f->installed_flow, f); - } - desired_flow_destroy(f); + track_or_destroy_for_flow_del(flow_table, f); } else { ovs_list_insert(&to_be_removed, &f->list_node); } @@ -1056,10 +1146,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, ovs_list_remove(&f->list_node); hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); - if (f->installed_flow) { - unlink_installed_to_desired(f->installed_flow, f); - } - desired_flow_destroy(f); + track_or_destroy_for_flow_del(flow_table, f); } } @@ -1105,7 +1192,9 @@ desired_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, ovs_list_init(&f->references); ovs_list_init(&f->list_node); ovs_list_init(&f->installed_ref_list_node); + ovs_list_init(&f->track_list_node); f->installed_flow = NULL; + f->is_deleted = false; ovn_flow_init(&f->flow, table_id, priority, cookie, match, actions); return f; @@ -1245,11 +1334,27 @@ ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) { hmap_init(&flow_table->match_flow_table); hmap_init(&flow_table->uuid_flow_table); + ovs_list_init(&flow_table->tracked_flows); + flow_table->change_tracked = false; } void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *flow_table) { + flow_table->change_tracked = false; + + struct desired_flow *f, *f_next; + LIST_FOR_EACH_SAFE (f, f_next, track_list_node, + &flow_table->tracked_flows) { + ovs_list_remove(&f->track_list_node); + if (f->is_deleted) { + if (f->installed_flow) { + unlink_installed_to_desired(f->installed_flow, f); + } + desired_flow_destroy(f); + } + } + struct sb_to_flow *stf, *next; HMAP_FOR_EACH_SAFE (stf, next, hmap_node, &flow_table->uuid_flow_table) { @@ -1538,6 +1643,117 @@ installed_flow_del(struct ovn_flow *i, struct ovs_list *msgs) add_flow_mod(&fm, msgs); } +static void +update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, + struct ovs_list *msgs) +{ + ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); + /* Iterate through all of the installed flows. If any of them are no + * longer desired, delete them; if any of them should have different + * actions, update them. */ + struct installed_flow *i, *next; + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { + unlink_all_refs_for_installed_flow(i); + struct desired_flow *d = + desired_flow_lookup(flow_table, &i->flow, NULL); + if (!d) { + /* Installed flow is no longer desirable. Delete it from the + * switch and from installed_flows. */ + installed_flow_del(&i->flow, msgs); + ovn_flow_log(&i->flow, "removing installed"); + + hmap_remove(&installed_flows, &i->match_hmap_node); + installed_flow_destroy(i); + } else { + if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, + d->flow.ofpacts, d->flow.ofpacts_len) || + i->flow.cookie != d->flow.cookie) { + installed_flow_mod(&i->flow, &d->flow, msgs); + ovn_flow_log(&i->flow, "updating installed"); + } + link_installed_to_desired(i, d); + + } + } + + /* Iterate through the desired flows and add those that aren't found + * in the installed flow table. */ + struct desired_flow *d; + HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { + i = installed_flow_lookup(&d->flow); + if (!i) { + ovn_flow_log(&d->flow, "adding installed"); + installed_flow_add(&d->flow, msgs); + + /* Copy 'd' from 'flow_table' to installed_flows. */ + i = installed_flow_dup(d); + hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash); + } + link_installed_to_desired(i, d); + } +} + +static void +update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, + struct ovs_list *msgs) +{ + struct desired_flow *f, *f_next; + LIST_FOR_EACH_SAFE (f, f_next, track_list_node, + &flow_table->tracked_flows) { + ovs_list_remove(&f->track_list_node); + if (f->is_deleted) { + /* The desired flow was deleted */ + if (f->installed_flow) { + struct installed_flow *i = f->installed_flow; + unlink_installed_to_desired(i, f); + + if (!i->desired_flow) { + installed_flow_del(&i->flow, msgs); + ovn_flow_log(&i->flow, "removing installed (tracked)"); + + hmap_remove(&installed_flows, &i->match_hmap_node); + installed_flow_destroy(i); + } else { + /* There are other desired flow(s) referencing this + * installed flow, so update the OVS flow for the new + * active flow (at least the cookie will be different, + * even if the actions are the same). */ + struct desired_flow *d = i->desired_flow; + ovn_flow_log(&i->flow, "updating installed (tracked)"); + installed_flow_mod(&i->flow, &d->flow, msgs); + } + } + desired_flow_destroy(f); + } else { + /* The desired flow was added or modified. */ + struct installed_flow *i = installed_flow_lookup(&f->flow); + if (!i) { + /* Adding a new flow. */ + installed_flow_add(&f->flow, msgs); + ovn_flow_log(&f->flow, "adding installed (tracked)"); + + /* Copy 'f' from 'flow_table' to installed_flows. */ + struct installed_flow *new_node = installed_flow_dup(f); + hmap_insert(&installed_flows, &new_node->match_hmap_node, + new_node->flow.hash); + link_installed_to_desired(new_node, f); + } else if (i->desired_flow == f) { + /* The installed flow is installed for f, but f has change + * tracked, so it must have been modified. */ + ovn_flow_log(&i->flow, "updating installed (tracked)"); + installed_flow_mod(&i->flow, &f->flow, msgs); + } else { + /* Adding a new flow that conflicts with an existing installed + * flow, so just add it to the link. */ + link_installed_to_desired(i, f); + } + /* The track_list_node emptyness is used to check if the node is + * already added to track list, so initialize it again here. */ + ovs_list_init(&f->track_list_node); + } + } +} + /* The flow table can be updated if the connection to the switch is up and * in the correct state and not backlogged with existing flow_mods. (Our * criteria for being backlogged appear very conservative, but the socket @@ -1652,48 +1868,10 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, } } - /* Iterate through all of the installed flows. If any of them are no - * longer desired, delete them; if any of them should have different - * actions, update them. */ - struct installed_flow *i, *next; - HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { - unlink_all_refs_for_installed_flow(i); - struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow, - NULL); - if (!d) { - /* Installed flow is no longer desirable. Delete it from the - * switch and from installed_flows. */ - installed_flow_del(&i->flow, &msgs); - ovn_flow_log(&i->flow, "removing installed"); - hmap_remove(&installed_flows, &i->match_hmap_node); - installed_flow_destroy(i); - } else { - if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, - d->flow.ofpacts, d->flow.ofpacts_len) || - i->flow.cookie != d->flow.cookie) { - ovn_flow_log(&i->flow, "updating installed"); - installed_flow_mod(&i->flow, &d->flow, &msgs); - } - link_installed_to_desired(i, d); - - } - } - - /* Iterate through the desired flows and add those that aren't found - * in the installed flow table. */ - struct desired_flow *d; - HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { - i = installed_flow_lookup(&d->flow); - if (!i) { - installed_flow_add(&d->flow, &msgs); - ovn_flow_log(&d->flow, "adding installed"); - - /* Copy 'd' from 'flow_table' to installed_flows. */ - i = installed_flow_dup(d); - hmap_insert(&installed_flows, &i->match_hmap_node, - i->flow.hash); - } - link_installed_to_desired(i, d); + if (flow_table->change_tracked) { + update_installed_flows_by_track(flow_table, &msgs); + } else { + update_installed_flows_by_compare(flow_table, &msgs); } /* Iterate through the installed groups from previous runs. If they @@ -1807,6 +1985,9 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, /* We were completely up-to-date before and still are. */ cur_cfg = nb_cfg; } + + flow_table->change_tracked = true; + ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); } /* Looks up the logical port with the name 'port_name' in 'br_int_'. If diff --git a/controller/ofctrl.h b/controller/ofctrl.h index 8789ce490..64b0ea5dd 100644 --- a/controller/ofctrl.h +++ b/controller/ofctrl.h @@ -38,6 +38,11 @@ struct ovn_desired_flow_table { /* SB uuid index for the cross reference nodes that link to the nodes in * match_flow_table.*/ struct hmap uuid_flow_table; + + /* Is flow changes tracked. */ + bool change_tracked; + /* Tracked flow changes. */ + struct ovs_list tracked_flows; }; /* Interface for OVN main loop. */ @@ -99,7 +104,6 @@ void ofctrl_flood_remove_flows(struct ovn_desired_flow_table *, struct hmap *flood_remove_nodes); void ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, const struct uuid *sb_uuid); - void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); void ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *); From patchwork Fri Aug 21 00:04:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441560 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7Hz0Gt6z9sVF for ; Thu, 18 Feb 2021 19:32:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 5787E60601 for ; Thu, 18 Feb 2021 08:32:45 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aChEsNEYOfqF for ; Thu, 18 Feb 2021 08:32:43 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id B013160603; Thu, 18 Feb 2021 08:32:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 0153C605DB; Thu, 18 Feb 2021 08:32:34 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C25F4C000E; Thu, 18 Feb 2021 08:32:34 +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 378ACC000D for ; Thu, 18 Feb 2021 08:32:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 25B96866B9 for ; Thu, 18 Feb 2021 08:32:34 +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 WwI--g-3jhs7 for ; Thu, 18 Feb 2021 08:32:33 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id C85C4866CA for ; Thu, 18 Feb 2021 08:32:01 +0000 (UTC) X-Mailbox-Line: From a181bfadcee39e7dde6bb0c9fa403e854e338ad7 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Thu, 20 Aug 2020 17:04:04 -0700 Subject: [ovs-dev] [PATCH ovn branch-20.06 05/15] ofctrl.c: Merge opposite changes of tracked flows before installing. 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" This patch optimizes the previous patch that incrementally processes flow installation by merging the "add-after-delete" flow changes as much as possible to avoid unnecessary OpenFlow updates. Acked-by: Mark Michelson Signed-off-by: Han Zhou (cherry picked from commit f4e508dd7a6cfbfc2e3250a8c11a8d0fdc1dfdd0) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 74 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index b0670bf44..2d0e15978 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1693,10 +1693,84 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, } } +/* Finds and returns a desired_flow in 'deleted_flows' that is exactly the + * same as 'target', including cookie and actions. + */ +static struct desired_flow * +deleted_flow_lookup(struct hmap *deleted_flows, struct ovn_flow *target) +{ + struct desired_flow *d; + HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, + deleted_flows) { + struct ovn_flow *f = &d->flow; + if (f->table_id == target->table_id + && f->priority == target->priority + && minimatch_equal(&f->match, &target->match) + && f->cookie == target->cookie + && ofpacts_equal(f->ofpacts, f->ofpacts_len, target->ofpacts, + target->ofpacts_len)) { + return d; + } + } + return NULL; +} + +/* This function scans the tracked flow changes in the order and merges "add" + * or "update" after "deleted" operations for exactly same flow (priority, + * table, match, action and cookie), to avoid unnecessary OF messages being + * sent to OVS. */ +static void +merge_tracked_flows(struct ovn_desired_flow_table *flow_table) +{ + struct hmap deleted_flows = HMAP_INITIALIZER(&deleted_flows); + struct desired_flow *f, *next; + LIST_FOR_EACH_SAFE (f, next, track_list_node, + &flow_table->tracked_flows) { + if (f->is_deleted) { + /* reuse f->match_hmap_node field since it is already removed from + * the desired flow table's match index. */ + hmap_insert(&deleted_flows, &f->match_hmap_node, + f->flow.hash); + } else { + struct desired_flow *del_f = deleted_flow_lookup(&deleted_flows, + &f->flow); + if (!del_f) { + continue; + } + + /* del_f must have been installed, otherwise it should have been + * removed during track_flow_add_or_modify. */ + ovs_assert(del_f->installed_flow); + if (!f->installed_flow) { + /* f is not installed yet. */ + struct installed_flow *i = del_f->installed_flow; + unlink_installed_to_desired(i, del_f); + link_installed_to_desired(i, f); + } else { + /* f has been installed before, and now was updated to exact + * the same flow as del_f. */ + ovs_assert(f->installed_flow == del_f->installed_flow); + unlink_installed_to_desired(del_f->installed_flow, del_f); + } + hmap_remove(&deleted_flows, &del_f->match_hmap_node); + ovs_list_remove(&del_f->track_list_node); + desired_flow_destroy(del_f); + + ovs_list_remove(&f->track_list_node); + ovs_list_init(&f->track_list_node); + } + } + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &deleted_flows) { + hmap_remove(&deleted_flows, &f->match_hmap_node); + } + hmap_destroy(&deleted_flows); +} + static void update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, struct ovs_list *msgs) { + merge_tracked_flows(flow_table); struct desired_flow *f, *f_next; LIST_FOR_EACH_SAFE (f, f_next, track_list_node, &flow_table->tracked_flows) { From patchwork Fri Oct 2 05:49:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441561 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7Jt2WGkz9sVF for ; Thu, 18 Feb 2021 19:33:34 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 81AF9605D9 for ; Thu, 18 Feb 2021 08:33:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EhI0ELum6mNo for ; Thu, 18 Feb 2021 08:33:29 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id D20C260623; Thu, 18 Feb 2021 08:33:29 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 8A10860630; Thu, 18 Feb 2021 08:33:05 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 452E2C0010; Thu, 18 Feb 2021 08:33:05 +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 1AF52C000E for ; Thu, 18 Feb 2021 08:33:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 0A27D8692C for ; Thu, 18 Feb 2021 08:33:04 +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 CpxfpynYWLhb for ; Thu, 18 Feb 2021 08:33:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id 8AAF1866B9 for ; Thu, 18 Feb 2021 08:32:40 +0000 (UTC) X-Mailbox-Line: From 07f59241dbce11409841eb3af060d07d98b7e34b Mon Sep 17 00:00:00 2001 Message-Id: <07f59241dbce11409841eb3af060d07d98b7e34b.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Thu, 1 Oct 2020 22:49:04 -0700 Subject: [ovs-dev] [PATCH ovn branch-20.06 06/15] ofctrl.c: Fix duplicated flow handling in I-P while merging opposite 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" In a scenario in I-P when a desired flow is removed and an exactly same flow is added back in the same iteration, the merge_tracked_flows() function will merge the operation without firing any OpenFlow action. However, if there are multiple desired flows (e.g. F1 and F2) matching the same installed flow, and if the one being re-added (say, F1) is the one currently installed in OVS, the current implementation will update the currently installed flow to F2 in the data structure while the actual OVS flow is not updated (still F1). So far there is still no problem, but the member desired_flow of the installed flow is pointing to the wrong desired flow. Now there is only one place where the desired_flow member is consulted, in update_installed_flows_by_track() for handling a tracked *modified* flow. In reality flow modification happens only when conjunction flows need to be combined, which would never happen together with the above scenario of merge_tracked_flows(). So this wrong desired_flow pointer doesn't cause any real problem yet. However, there is a place that can already utilize this active desired_flow information, which is when handling a tracked flow deletion in update_installed_flows_by_track(): we can check if the flow being deleted is the currently active one installed in OVS. If not, we don't need to send and OpenFlow modify to OVS. This optimization is added in this patch, apart from fixing the problem of merge_tracked_flows(). Without the fix, this optimization would cause the test case added in this patch fail. Fixes: f4e508dd7 ("ofctrl.c: Merge opposite changes of tracked flows before installing.") Acked-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit 9d2e8d32fb9865513b70408a665184a67564390d) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 28 +++++++++++- tests/ovn.at | 108 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 2d0e15978..3225e6c67 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -788,6 +788,24 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } +/* Returns true if a desired flow is active (the one currently installed + * among the list of desired flows that are linked to the same installed + * flow). */ +static inline bool +desired_flow_is_active(struct desired_flow *d) +{ + return (d->installed_flow && d->installed_flow->desired_flow == d); +} + +/* Set a desired flow as the active one among the list of desired flows + * that are linked to the same installed flow. */ +static inline void +desired_flow_set_active(struct desired_flow *d) +{ + ovs_assert(d->installed_flow); + d->installed_flow->desired_flow = d; +} + static void link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { @@ -1741,6 +1759,8 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) /* del_f must have been installed, otherwise it should have been * removed during track_flow_add_or_modify. */ ovs_assert(del_f->installed_flow); + + bool del_f_was_active = desired_flow_is_active(del_f); if (!f->installed_flow) { /* f is not installed yet. */ struct installed_flow *i = del_f->installed_flow; @@ -1752,6 +1772,9 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) ovs_assert(f->installed_flow == del_f->installed_flow); unlink_installed_to_desired(del_f->installed_flow, del_f); } + if (del_f_was_active) { + desired_flow_set_active(f); + } hmap_remove(&deleted_flows, &del_f->match_hmap_node); ovs_list_remove(&del_f->track_list_node); desired_flow_destroy(del_f); @@ -1779,6 +1802,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, /* The desired flow was deleted */ if (f->installed_flow) { struct installed_flow *i = f->installed_flow; + bool was_active = desired_flow_is_active(f); unlink_installed_to_desired(i, f); if (!i->desired_flow) { @@ -1787,7 +1811,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, hmap_remove(&installed_flows, &i->match_hmap_node); installed_flow_destroy(i); - } else { + } else if (was_active) { /* There are other desired flow(s) referencing this * installed flow, so update the OVS flow for the new * active flow (at least the cookie will be different, @@ -1811,7 +1835,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, hmap_insert(&installed_flows, &new_node->match_hmap_node, new_node->flow.hash); link_installed_to_desired(new_node, f); - } else if (i->desired_flow == f) { + } else if (desired_flow_is_active(f)) { /* The installed flow is installed for f, but f has change * tracked, so it must have been modified. */ ovn_flow_log(&i->flow, "updating installed (tracked)"); diff --git a/tests/ovn.at b/tests/ovn.at index 210bec8d3..745fcb8d0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -21279,3 +21279,111 @@ AT_CHECK([test "$encap_rec_mvtep" == "$encap_rec_mvtep1"], [0], []) OVN_CLEANUP([hv1]) AT_CLEANUP + +# This test cases tests a scenario of ACL confliction with address set update. +# It is to cover a corner case when flows are re-processed in the I-P +# iteration, combined with the scenario of conflicting ACLs. +AT_SETUP([ovn -- conflict ACLs with address set]) +ovn_start + +ovn-nbctl ls-add ls1 + +ovn-nbctl lsp-add ls1 lsp1 \ +-- lsp-set-addresses lsp1 "f0:00:00:00:00:01 10.0.0.1" + +ovn-nbctl lsp-add ls1 lsp2 \ +-- lsp-set-addresses lsp2 "f0:00:00:00:00:02 10.0.0.2" + +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-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=lsp1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +ovs-vsctl -- add-port br-int hv1-vif2 -- \ + set interface hv1-vif2 external-ids:iface-id=lsp2 \ + options:tx_pcap=hv1/vif2-tx.pcap \ + options:rxq_pcap=hv1/vif2-rx.pcap \ + ofport-request=2 + +# Default drop +ovn-nbctl acl-add ls1 to-lport 1000 \ +'outport == "lsp1" && ip4' drop + +# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT... +# +# This shell function causes an ip packet to be received on INPORT. +# The packet's content has Ethernet destination DST and source SRC +# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits). +# The OUTPORTs (zero or more) list the VIFs on which the packet should +# be received. INPORT and the OUTPORTs are specified as logical switch +# port numbers, e.g. 11 for vif11. +test_ip() { + # This packet has bad checksums but logical L3 routing doesn't check. + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 + local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ +${dst_ip}0035111100080000 + shift; shift; shift; shift; shift + as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $packet + for outport; do + echo $packet >> $outport.expected + done +} + +ip_to_hex() { + printf "%02x%02x%02x%02x" "$@" +} + +reset_pcap_file() { + local iface=$1 + local pcap_file=$2 + ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ +options:rxq_pcap=dummy-rx.pcap + rm -f ${pcap_file}*.pcap + ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ +options:rxq_pcap=${pcap_file}-rx.pcap +} + +# Create an address set +ovn-nbctl create Address_Set name=as1 \ +addresses=\"10.0.0.2\",\"10.0.0.3\" + +# Create overlapping ACLs resulting in conflict desired OVS flows +# Add ACL1 uses the address set +ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ +'outport == "lsp1" && ip4 && ip4.src == $as1' allow + +# Add ACL2 which uses a single IP, which shouldn't take effect because +# when it is added incrementally there is already a conflict one installed. +ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ +'outport == "lsp1" && ip4 && ip4.src == 10.0.0.2' drop + + +sip=`ip_to_hex 10 0 0 2` +dip=`ip_to_hex 10 0 0 1` +test_ip 2 f00000000002 f00000000001 $sip $dip 1 +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) + +# Update the address set which causes flow reprocessing but the OVS flow +# for allowing 10.0.0.2 should keep unchanged +ovn-nbctl --wait=hv set Address_Set as1 addresses=\"10.0.0.2\" + +test_ip 2 f00000000002 f00000000001 $sip $dip 1 +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) + +# Delete the ACL1 that has "allow" action +ovn-nbctl acl-del ls1 to-lport 1001 \ +'outport == "lsp1" && ip4 && ip4.src == $as1' + +# ACL2 should take effect and packet should be dropped +test_ip 2 f00000000002 f00000000001 $sip $dip +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP From patchwork Fri Oct 2 19:47:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1441562 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7KT46pJz9sRf for ; Thu, 18 Feb 2021 19:34:05 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id D7AE56062E for ; Thu, 18 Feb 2021 08:34:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id iFF0iIEix3-N for ; Thu, 18 Feb 2021 08:34:01 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id BACBA605F5; Thu, 18 Feb 2021 08:34:01 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 2F2C8605FF; Thu, 18 Feb 2021 08:33:35 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D8F48C0010; Thu, 18 Feb 2021 08:33:34 +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 9C45CC000E for ; Thu, 18 Feb 2021 08:33:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9675A86B78 for ; Thu, 18 Feb 2021 08:33:33 +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 DcmlK9VPZ5de for ; Thu, 18 Feb 2021 08:33:33 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id 3C74F86A10 for ; Thu, 18 Feb 2021 08:33:10 +0000 (UTC) X-Mailbox-Line: From 71c7d2dcde55ad14532d87345a7a6d7c1814b614 Mon Sep 17 00:00:00 2001 Message-Id: <71c7d2dcde55ad14532d87345a7a6d7c1814b614.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Han Zhou To: dev@openvswitch.org Date: Fri, 2 Oct 2020 12:47:52 -0700 Subject: [ovs-dev] [PATCH ovn branch-20.06 07/15] ofctrl.c: Avoid repeatedly linking an installed flow and a desired flow. 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" In update_installed_flows_by_compare() there are two loops. The first loop iterates the installed flows and find its peer in desired flows to: 1. uninstall flows that are not needed anymore 2. update flows if needed At the same time, it links the desired flow found for the installed flow which also set the desired flow as the current active installed flow. The second loop iterates the desired flows and find its peer in installed flows to install missing flows. At the same time it will detect if there are conflict desired flows matching same installed flow then just link them. However, currently in the second loop, it blindly link the desired flows to the installed flows, without checking if it is already linked in the first loop. Lucky enough, this won't cause any real problem so far, because when there are conflict flows, the one found in the first loop will be set as active in the installed_flow, and in the function link_installed_to_desired() checks if it is already the active desired flow it just does nothing but return. However, the check in the link_installed_to_desired() is confusing because a desired_flow may be linked to the installed_flow already but not the active flow, and the check is insufficient. It should be rather an assertion and let the caller ensure that a pair of desired_flow and installed_flow is never linked twice. For the above reason, this patch does the following changes: 1. Removes the check in link_installed_to_desired() and convert it to an assert. 2. Before calling link_installed_to_desired() in the above mentioned loop, check if the desired flow is already installed. Acked-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit 7cab7bd1268ba67429954da4f73de91090acf779) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 3225e6c67..384659d0c 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -806,13 +806,18 @@ desired_flow_set_active(struct desired_flow *d) d->installed_flow->desired_flow = d; } +/* Adds the desired flow to the list of desired flows that have same match + * conditions as the installed flow. + * + * If the newly added desired flow is the first one in the list, it is also set + * as the active one. + * + * It is caller's responsibility to make sure the link between the pair didn't + * exist before. */ static void link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { - if (i->desired_flow == d) { - return; - } - + ovs_assert(i->desired_flow != d); if (ovs_list_is_empty(&i->desired_refs)) { ovs_assert(!i->desired_flow); i->desired_flow = d; @@ -1706,8 +1711,12 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, /* Copy 'd' from 'flow_table' to installed_flows. */ i = installed_flow_dup(d); hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash); + link_installed_to_desired(i, d); + } else if (!d->installed_flow) { + /* This is a desired_flow that conflicts with one installed + * previously but not linked yet. */ + link_installed_to_desired(i, d); } - link_installed_to_desired(i, d); } } From patchwork Sun Oct 11 12:05:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441563 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7Km1zx0z9sRf for ; Thu, 18 Feb 2021 19:34:19 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 4BBD38729E; Thu, 18 Feb 2021 08:34:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OpDOlujZTTd3; Thu, 18 Feb 2021 08:34:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 72C0786DF1; Thu, 18 Feb 2021 08:34:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 55FC9C000E; Thu, 18 Feb 2021 08:34:15 +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 41C09C000E for ; Thu, 18 Feb 2021 08:34:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 31EDC86C25 for ; Thu, 18 Feb 2021 08:34:14 +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 59vc6Lfq1sUZ for ; Thu, 18 Feb 2021 08:34:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id D23CA864C5 for ; Thu, 18 Feb 2021 08:33:49 +0000 (UTC) X-Mailbox-Line: From 2c2836d8cf30a6930e27ab623d430761b889e13a Mon Sep 17 00:00:00 2001 Message-Id: <2c2836d8cf30a6930e27ab623d430761b889e13a.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Sun, 11 Oct 2020 14:05:31 +0200 Subject: [ovs-dev] [PATCH ovn branch-20.06 08/15] ofctrl.c: Only merge actions for conjunctive flows. 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" In ofctrl_add_or_append_flow() when merging flow actions make sure we only do that for conjunctive flows. All other actions can not be merged with action "conjunction". CC: Mark Michelson Fixes: e659bab31a91 ("Combine conjunctions with identical matches into one flow.") Signed-off-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit dadae4f800ccb1f2759378f0bd804dd002e31605) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 124 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 99 insertions(+), 25 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 384659d0c..8404479bc 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -206,6 +206,9 @@ struct installed_flow { struct desired_flow *desired_flow; }; +typedef bool +(*desired_flow_match_cb)(const struct desired_flow *candidate, + const void *arg); static struct desired_flow *desired_flow_alloc( uint8_t table_id, uint16_t priority, @@ -213,9 +216,15 @@ static struct desired_flow *desired_flow_alloc( const struct match *match, const struct ofpbuf *actions); static struct desired_flow *desired_flow_lookup( + struct ovn_desired_flow_table *, + const struct ovn_flow *target); +static struct desired_flow *desired_flow_lookup_check_uuid( struct ovn_desired_flow_table *, const struct ovn_flow *target, - const struct uuid *sb_uuid); + const struct uuid *); +static struct desired_flow *desired_flow_lookup_conjunctive( + struct ovn_desired_flow_table *, + const struct ovn_flow *target); static void desired_flow_destroy(struct desired_flow *); static struct installed_flow *installed_flow_lookup( @@ -806,6 +815,19 @@ desired_flow_set_active(struct desired_flow *d) d->installed_flow->desired_flow = d; } +static bool +flow_action_has_conj(const struct ovn_flow *f) +{ + const struct ofpact *a = NULL; + + OFPACT_FOR_EACH (a, f->ofpacts, f->ofpacts_len) { + if (a->type == OFPACT_CONJUNCTION) { + return true; + } + } + return false; +} + /* Adds the desired flow to the list of desired flows that have same match * conditions as the installed flow. * @@ -962,7 +984,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, match, actions); - if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) { + if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { if (log_duplicate_flow) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); if (!VLOG_DROP_DBG(&rl)) { @@ -1002,14 +1024,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, const struct ofpbuf *actions, const struct uuid *sb_uuid) { - struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, - match, actions); - struct desired_flow *existing; - existing = desired_flow_lookup(desired_flows, &f->flow, NULL); + struct desired_flow *f; + + f = desired_flow_alloc(table_id, priority, cookie, match, actions); + existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow); if (existing) { - /* There's already a flow with this particular match. Append the - * action to that flow rather than adding a new flow + /* There's already a flow with this particular match and action + * 'conjunction'. Append the action to that flow rather than + * adding a new flow. */ uint64_t compound_stub[64 / 8]; struct ofpbuf compound; @@ -1248,15 +1271,11 @@ installed_flow_dup(struct desired_flow *src) return dst; } -/* Finds and returns a desired_flow in 'flow_table' whose key is identical to - * 'target''s key, or NULL if there is none. - * - * If sb_uuid is not NULL, the function will also check if the found flow is - * referenced by the sb_uuid. */ static struct desired_flow * -desired_flow_lookup(struct ovn_desired_flow_table *flow_table, - const struct ovn_flow *target, - const struct uuid *sb_uuid) +desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target, + desired_flow_match_cb match_cb, + const void *arg) { struct desired_flow *d; HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, @@ -1265,20 +1284,76 @@ desired_flow_lookup(struct ovn_desired_flow_table *flow_table, if (f->table_id == target->table_id && f->priority == target->priority && minimatch_equal(&f->match, &target->match)) { - if (!sb_uuid) { + + if (!match_cb || match_cb(d, arg)) { return d; } - struct sb_flow_ref *sfr; - LIST_FOR_EACH (sfr, sb_list, &d->references) { - if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { - return d; - } - } } } return NULL; } +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + */ +static struct desired_flow * +desired_flow_lookup(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target) +{ + return desired_flow_lookup__(flow_table, target, NULL, NULL); +} + +static bool +flow_lookup_match_uuid_cb(const struct desired_flow *candidate, + const void *arg) +{ + const struct uuid *sb_uuid = arg; + struct sb_flow_ref *sfr; + + LIST_FOR_EACH (sfr, sb_list, &candidate->references) { + if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { + return true; + } + } + return false; +} + +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + * + * The function will also check if the found flow is referenced by the + * 'sb_uuid'. + */ +static struct desired_flow * +desired_flow_lookup_check_uuid(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target, + const struct uuid *sb_uuid) +{ + return desired_flow_lookup__(flow_table, target, flow_lookup_match_uuid_cb, + sb_uuid); +} + +static bool +flow_lookup_match_conj_cb(const struct desired_flow *candidate, + const void *arg OVS_UNUSED) +{ + return flow_action_has_conj(&candidate->flow); +} + +/* Finds and returns a desired_flow in 'flow_table' whose key is identical to + * 'target''s key, or NULL if there is none. + * + * The function will only return a matching flow if it contains action + * 'conjunction'. + */ +static struct desired_flow * +desired_flow_lookup_conjunctive(struct ovn_desired_flow_table *flow_table, + const struct ovn_flow *target) +{ + return desired_flow_lookup__(flow_table, target, flow_lookup_match_conj_cb, + NULL); +} + /* Finds and returns an installed_flow in installed_flows whose key is * identical to 'target''s key, or NULL if there is none. */ static struct installed_flow * @@ -1677,8 +1752,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, struct installed_flow *i, *next; HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { unlink_all_refs_for_installed_flow(i); - struct desired_flow *d = - desired_flow_lookup(flow_table, &i->flow, NULL); + struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow); if (!d) { /* Installed flow is no longer desirable. Delete it from the * switch and from installed_flows. */ From patchwork Sun Oct 11 12:05:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441564 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7LQ1Mzfz9sRf for ; Thu, 18 Feb 2021 19:34:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id B79D18725F; Thu, 18 Feb 2021 08:34:52 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rCbUghAOKAQh; Thu, 18 Feb 2021 08:34:52 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 36E9B86E5B; Thu, 18 Feb 2021 08:34:52 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 28A4CC000E; Thu, 18 Feb 2021 08:34:52 +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 DCAC2C000D for ; Thu, 18 Feb 2021 08:34:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D106786D3F for ; Thu, 18 Feb 2021 08:34:50 +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 FMWnBz2UyORq for ; Thu, 18 Feb 2021 08:34:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id C0FC586DDC for ; Thu, 18 Feb 2021 08:34:20 +0000 (UTC) X-Mailbox-Line: From 6426f3b116cd53f3a43f143c7103ea21a4361195 Mon Sep 17 00:00:00 2001 Message-Id: <6426f3b116cd53f3a43f143c7103ea21a4361195.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Sun, 11 Oct 2020 14:05:45 +0200 Subject: [ovs-dev] [PATCH ovn branch-20.06 09/15] ofctrl.c: Do not change flow ordering when merging opposite 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Instead of removing the old desired flow from the list and inserting the new one at the end we now directly replace the old flow with the new one while maintaining the same ordering. For now order of the flows is not relevant but further commits require maintaining the order of desired flows. Signed-off-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit e49ce9a33f38f29c44e3c30afcc189b5f6a9ef8e) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 8404479bc..0262a5cc3 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -848,6 +848,26 @@ link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) d->installed_flow = i; } +/* Replaces 'old_desired' with 'new_desired' in the list of desired flows + * that have same match conditions as the installed flow. + * + * If 'old_desired' was the active flow, 'new_desired' becomes the active one. + */ +static void +replace_installed_to_desired(struct installed_flow *i, + struct desired_flow *old_desired, + struct desired_flow *new_desired) +{ + ovs_assert(old_desired->installed_flow == i); + ovs_list_replace(&new_desired->installed_ref_list_node, + &old_desired->installed_ref_list_node); + old_desired->installed_flow = NULL; + new_desired->installed_flow = i; + if (i->desired_flow == old_desired) { + i->desired_flow = new_desired; + } +} + static void unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { @@ -1843,21 +1863,15 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) * removed during track_flow_add_or_modify. */ ovs_assert(del_f->installed_flow); - bool del_f_was_active = desired_flow_is_active(del_f); if (!f->installed_flow) { /* f is not installed yet. */ - struct installed_flow *i = del_f->installed_flow; - unlink_installed_to_desired(i, del_f); - link_installed_to_desired(i, f); + replace_installed_to_desired(del_f->installed_flow, del_f, f); } else { /* f has been installed before, and now was updated to exact * the same flow as del_f. */ ovs_assert(f->installed_flow == del_f->installed_flow); unlink_installed_to_desired(del_f->installed_flow, del_f); } - if (del_f_was_active) { - desired_flow_set_active(f); - } hmap_remove(&deleted_flows, &del_f->match_hmap_node); ovs_list_remove(&del_f->track_list_node); desired_flow_destroy(del_f); From patchwork Sun Oct 11 12:05:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441566 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.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 4Dh7MR51YVz9sVR for ; Thu, 18 Feb 2021 19:35:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 2D04160613 for ; Thu, 18 Feb 2021 08:35:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id R8-tgyIM83Y7 for ; Thu, 18 Feb 2021 08:35:43 +0000 (UTC) Received: by smtp3.osuosl.org (Postfix, from userid 1001) id 1503E60618; Thu, 18 Feb 2021 08:35:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id B5EFC605EA; Thu, 18 Feb 2021 08:35:25 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 81357C000E; Thu, 18 Feb 2021 08:35:25 +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 4B82FC000D for ; Thu, 18 Feb 2021 08:35:24 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 375438666A for ; Thu, 18 Feb 2021 08:35:24 +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 TXSQ3sDHWg07 for ; Thu, 18 Feb 2021 08:35:23 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id E0E1686690 for ; Thu, 18 Feb 2021 08:34:57 +0000 (UTC) X-Mailbox-Line: From e0849c4c18b256060b2289a5226be451368dfa76 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Sun, 11 Oct 2020 14:05:59 +0200 Subject: [ovs-dev] [PATCH ovn branch-20.06 10/15] ofctrl.c: Simplify active desired flow selection. 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" Always consider the first "desired flow" in the list of flows that refer an "installed flow" as the active flow. This simplifies the logic of the flow update code and is used in a further commit to implement a partial ordering of desired flows within installed flows. Signed-off-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit 107bb25029350bd0f7dfeeb0ef3053adbd504e3e) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 91 ++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 55 deletions(-) flow_action_has_conj(const struct ovn_flow *f) @@ -831,27 +811,22 @@ flow_action_has_conj(const struct ovn_flow *f) /* Adds the desired flow to the list of desired flows that have same match * conditions as the installed flow. * - * If the newly added desired flow is the first one in the list, it is also set - * as the active one. - * * It is caller's responsibility to make sure the link between the pair didn't - * exist before. */ -static void + * exist before. + * + * Returns true if the newly added desired flow is selected to be the active + * one. + */ +static bool link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { - ovs_assert(i->desired_flow != d); - if (ovs_list_is_empty(&i->desired_refs)) { - ovs_assert(!i->desired_flow); - i->desired_flow = d; - } - ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); d->installed_flow = i; + ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); + return installed_flow_get_active(i) == d; } /* Replaces 'old_desired' with 'new_desired' in the list of desired flows * that have same match conditions as the installed flow. - * - * If 'old_desired' was the active flow, 'new_desired' becomes the active one. */ static void replace_installed_to_desired(struct installed_flow *i, @@ -863,24 +838,22 @@ replace_installed_to_desired(struct installed_flow *i, &old_desired->installed_ref_list_node); old_desired->installed_flow = NULL; new_desired->installed_flow = i; - if (i->desired_flow == old_desired) { - i->desired_flow = new_desired; - } } -static void +/* Removes the desired flow from the list of desired flows that have the same + * match conditions as the installed flow. + * + * Returns true if the desired flow was the previously active flow. + */ +static bool unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { - ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); + struct desired_flow *old_active = installed_flow_get_active(i); + ovs_assert(d && d->installed_flow == i); ovs_list_remove(&d->installed_ref_list_node); d->installed_flow = NULL; - if (i->desired_flow == d) { - i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : - CONTAINER_OF(ovs_list_front(&i->desired_refs), - struct desired_flow, - installed_ref_list_node); - } + return old_active == d; } static void @@ -1280,7 +1253,6 @@ installed_flow_dup(struct desired_flow *src) { struct installed_flow *dst = xmalloc(sizeof *dst); ovs_list_init(&dst->desired_refs); - dst->desired_flow = NULL; dst->flow.table_id = src->flow.table_id; dst->flow.priority = src->flow.priority; minimatch_clone(&dst->flow.match, &src->flow.match); @@ -1291,6 +1263,17 @@ installed_flow_dup(struct desired_flow *src) return dst; } +static struct desired_flow * +installed_flow_get_active(struct installed_flow *f) +{ + if (!ovs_list_is_empty(&f->desired_refs)) { + return CONTAINER_OF(ovs_list_front(&f->desired_refs), + struct desired_flow, + installed_ref_list_node); + } + return NULL; +} + static struct desired_flow * desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, const struct ovn_flow *target, @@ -1439,8 +1422,7 @@ static void installed_flow_destroy(struct installed_flow *f) { if (f) { - ovs_assert(ovs_list_is_empty(&f->desired_refs)); - ovs_assert(!f->desired_flow); + ovs_assert(!installed_flow_get_active(f)); ovn_flow_uninit(&f->flow); free(f); } @@ -1899,10 +1881,10 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, /* The desired flow was deleted */ if (f->installed_flow) { struct installed_flow *i = f->installed_flow; - bool was_active = desired_flow_is_active(f); - unlink_installed_to_desired(i, f); + bool was_active = unlink_installed_to_desired(i, f); + struct desired_flow *d = installed_flow_get_active(i); - if (!i->desired_flow) { + if (!d) { installed_flow_del(&i->flow, msgs); ovn_flow_log(&i->flow, "removing installed (tracked)"); @@ -1913,7 +1895,6 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, * installed flow, so update the OVS flow for the new * active flow (at least the cookie will be different, * even if the actions are the same). */ - struct desired_flow *d = i->desired_flow; ovn_flow_log(&i->flow, "updating installed (tracked)"); installed_flow_mod(&i->flow, &d->flow, msgs); } @@ -1932,7 +1913,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, hmap_insert(&installed_flows, &new_node->match_hmap_node, new_node->flow.hash); link_installed_to_desired(new_node, f); - } else if (desired_flow_is_active(f)) { + } else if (installed_flow_get_active(i) == f) { /* The installed flow is installed for f, but f has change * tracked, so it must have been modified. */ ovn_flow_log(&i->flow, "updating installed (tracked)"); diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 0262a5cc3..c03c2d7b4 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -188,6 +188,8 @@ struct sb_flow_ref { * relationship is 1 to N. A link is added when a flow addition is processed. * A link is removed when a flow deletion is processed, the desired flow * table is cleared, or the installed flow table is cleared. + * The first desired_flow in the list is the active one, the one that is + * actually installed. */ struct installed_flow { struct ovn_flow flow; @@ -199,11 +201,6 @@ struct installed_flow { * installed flow, e.g. when there are conflict/duplicated ACLs that * generates same match conditions). */ struct ovs_list desired_refs; - - /* The corresponding flow in desired table. It must be one of the flows in - * desired_refs list. If there are more than one flows in references list, - * this is the one that is actually installed. */ - struct desired_flow *desired_flow; }; typedef bool @@ -231,6 +228,7 @@ static struct installed_flow *installed_flow_lookup( const struct ovn_flow *target); static void installed_flow_destroy(struct installed_flow *); static struct installed_flow *installed_flow_dup(struct desired_flow *); +static struct desired_flow *installed_flow_get_active(struct installed_flow *); static uint32_t ovn_flow_match_hash(const struct ovn_flow *); static char *ovn_flow_to_string(const struct ovn_flow *); @@ -796,24 +794,6 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored"); } } - -/* Returns true if a desired flow is active (the one currently installed - * among the list of desired flows that are linked to the same installed - * flow). */ -static inline bool -desired_flow_is_active(struct desired_flow *d) -{ - return (d->installed_flow && d->installed_flow->desired_flow == d); -} - -/* Set a desired flow as the active one among the list of desired flows - * that are linked to the same installed flow. */ -static inline void -desired_flow_set_active(struct desired_flow *d) -{ - ovs_assert(d->installed_flow); - d->installed_flow->desired_flow = d; -} static bool From patchwork Tue Oct 13 09:04:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441567 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7MY3sWlz9sRf for ; Thu, 18 Feb 2021 19:35:53 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 535DD872C6; Thu, 18 Feb 2021 08:35:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id J+pGAvrQvFk7; Thu, 18 Feb 2021 08:35:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 9D6CF872B9; Thu, 18 Feb 2021 08:35:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6A9DBC0010; Thu, 18 Feb 2021 08:35: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 41310C000E for ; Thu, 18 Feb 2021 08:35:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 2D9378666A for ; Thu, 18 Feb 2021 08:35: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 W1kwpnlAYSZg for ; Thu, 18 Feb 2021 08:35:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id DBBB18613C for ; Thu, 18 Feb 2021 08:35:30 +0000 (UTC) X-Mailbox-Line: From 8c869e920f8d6f374075ab425496b85131ed69a1 Mon Sep 17 00:00:00 2001 Message-Id: <8c869e920f8d6f374075ab425496b85131ed69a1.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 13 Oct 2020 11:04:03 +0200 Subject: [ovs-dev] [PATCH ovn branch-20.06 11/15] ofctrl.c: Always log the most recent flow 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation by tracking.") Signed-off-by: Dumitru Ceara Signed-off-by: Han Zhou (cherry picked from commit 33c15c145988daa6172928dc870f3a0225515f50) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index c03c2d7b4..16dc10cfb 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1895,8 +1895,8 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, * installed flow, so update the OVS flow for the new * active flow (at least the cookie will be different, * even if the actions are the same). */ - ovn_flow_log(&i->flow, "updating installed (tracked)"); installed_flow_mod(&i->flow, &d->flow, msgs); + ovn_flow_log(&i->flow, "updating installed (tracked)"); } } desired_flow_destroy(f); @@ -1916,8 +1916,8 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, } else if (installed_flow_get_active(i) == f) { /* The installed flow is installed for f, but f has change * tracked, so it must have been modified. */ - ovn_flow_log(&i->flow, "updating installed (tracked)"); installed_flow_mod(&i->flow, &f->flow, msgs); + ovn_flow_log(&i->flow, "updating installed (tracked)"); } else { /* Adding a new flow that conflicts with an existing installed * flow, so just add it to the link. */ From patchwork Thu Oct 15 09:12:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441568 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7NW4G2Fz9sRf for ; Thu, 18 Feb 2021 19:36:43 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id DBF4D87256; Thu, 18 Feb 2021 08:36:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jfWl+iUydmku; Thu, 18 Feb 2021 08:36:40 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 7197A872A5; Thu, 18 Feb 2021 08:36:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 64FDEC000E; Thu, 18 Feb 2021 08:36:40 +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 14087C000D for ; Thu, 18 Feb 2021 08:36:39 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 010B886D3E for ; Thu, 18 Feb 2021 08:36:39 +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 P8uUiUu2sJYC for ; Thu, 18 Feb 2021 08:36:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id 0F44486DC0 for ; Thu, 18 Feb 2021 08:35:54 +0000 (UTC) X-Mailbox-Line: From 55b31d22b55f80acd95774629022c64757cf112b Mon Sep 17 00:00:00 2001 Message-Id: <55b31d22b55f80acd95774629022c64757cf112b.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 15 Oct 2020 11:12:30 +0200 Subject: [ovs-dev] [PATCH ovn branch-20.06 12/15] ofctrl.c: Add a predictable resolution for conflicting flow actions. 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" Until now, in case the ACL configuration generates openflows that have the same match but different actions, ovn-controller was using the following approach: 1. If the flow being added contains conjunctive actions, merge its actions with the already existing flow. 2. Otherwise, if the flow is being added incrementally (update_installed_flows_by_track), don't install the new flow but instead keep the old one. 3. Otherwise, (update_installed_flows_by_compare), don't install the new flow but instead keep the old one. Even though one can argue that having an ACL with a match that includes the match of another ACL is a misconfiguration, it can happen that the users provision OVN like this. Depending on the order of reading and installing the logical flows, the above operations can yield unpredictable results, e.g., allow specific traffic but then after ovn-controller is restarted (or a recompute happens) that specific traffic starts being dropped. A simple example of ACL configuration is: ovn-nbctl acl-add ls to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow ovn-nbctl acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow ovn-nbctl acl-add ls to-lport 2 'arp' allow ovn-nbctl acl-add ls to-lport 1 'ip4' drop This is a pattern used by most CMSs: - define a default deny policy. - punch holes in the default deny policy based on user specific configurations. Without this commit the behavior for traffic from 10.0.0.1 to 10.0.0.5 is unpredictable. Depending on the order of operations traffic might be dropped or allowed. It's also quite hard to force the CMS to ensure that such match overlaps never occur. To address this issue we now ensure that all desired flows refering the same installed flow are partially sorted in the following way: - first all flows with action "allow". - then all flows with action "drop". - then a single flow with action "conjunction" (resulting from merging all flows with the same match and action conjunction). This ensures that "allow" flows have precedence over "drop" flows which in turn have precedence over "conjunction" flows. Essentially less restrictive flows are always preferred over more restrictive flows whenever a match conflict happens. CC: Daniel Alvarez Reported-at: https://bugzilla.redhat.com/1871931 Signed-off-by: Dumitru Ceara Acked-by: Mark Gray Signed-off-by: Han Zhou (cherry picked from commit 986b3d5e4ad6f05245d021ba699c957246294a22) Signed-off-by: Frode Nordahl --- controller/ofctrl.c | 74 +++++++++++++-- tests/ovn.at | 214 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 283 insertions(+), 5 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 16dc10cfb..c1bbc589e 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -188,6 +188,14 @@ struct sb_flow_ref { * relationship is 1 to N. A link is added when a flow addition is processed. * A link is removed when a flow deletion is processed, the desired flow * table is cleared, or the installed flow table is cleared. + * + * To ensure predictable behavior, the list of desired flows is maintained + * partially sorted in the following way (from least restrictive to most + * restrictive wrt. match): + * - allow flows without action conjunction. + * - drop flows without action conjunction. + * - a single flow with action conjunction. + * * The first desired_flow in the list is the active one, the one that is * actually installed. */ @@ -795,6 +803,12 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } +static bool +flow_action_has_drop(const struct ovn_flow *f) +{ + return f->ofpacts_len == 0; +} + static bool flow_action_has_conj(const struct ovn_flow *f) { @@ -808,6 +822,33 @@ flow_action_has_conj(const struct ovn_flow *f) return false; } +static bool +flow_action_has_allow(const struct ovn_flow *f) +{ + return !flow_action_has_drop(f) && !flow_action_has_conj(f); +} + +/* Returns true if flow 'a' is preferred over flow 'b'. */ +static bool +flow_is_preferred(const struct ovn_flow *a, const struct ovn_flow *b) +{ + if (flow_action_has_allow(b)) { + return false; + } + if (flow_action_has_allow(a)) { + return true; + } + if (flow_action_has_drop(b)) { + return false; + } + if (flow_action_has_drop(a)) { + return true; + } + + /* Flows 'a' and 'b' should never both have action conjunction. */ + OVS_NOT_REACHED(); +} + /* Adds the desired flow to the list of desired flows that have same match * conditions as the installed flow. * @@ -820,8 +861,18 @@ flow_action_has_conj(const struct ovn_flow *f) static bool link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) { + struct desired_flow *f; + + /* Find first 'f' such that 'd' is preferred over 'f'. If no such desired + * flow exists then 'f' will point after the last element of the list. + */ + LIST_FOR_EACH (f, installed_ref_list_node, &i->desired_refs) { + if (flow_is_preferred(&d->flow, &f->flow)) { + break; + } + } + ovs_list_insert(&f->installed_ref_list_node, &d->installed_ref_list_node); d->installed_flow = i; - ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); return installed_flow_get_active(i) == d; } @@ -1790,8 +1841,14 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, link_installed_to_desired(i, d); } else if (!d->installed_flow) { /* This is a desired_flow that conflicts with one installed - * previously but not linked yet. */ - link_installed_to_desired(i, d); + * previously but not linked yet. However, if this flow becomes + * active, e.g., it is less restrictive than the previous active + * flow then modify the installed flow. + */ + if (link_installed_to_desired(i, d)) { + installed_flow_mod(&i->flow, &d->flow, msgs); + ovn_flow_log(&i->flow, "updating installed (conflict)"); + } } } } @@ -1920,8 +1977,15 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, ovn_flow_log(&i->flow, "updating installed (tracked)"); } else { /* Adding a new flow that conflicts with an existing installed - * flow, so just add it to the link. */ - link_installed_to_desired(i, f); + * flow, so add it to the link. If this flow becomes active, + * e.g., it is less restrictive than the previous active flow + * then modify the installed flow. + */ + if (link_installed_to_desired(i, f)) { + installed_flow_mod(&i->flow, &f->flow, msgs); + ovn_flow_log(&i->flow, + "updating installed (tracked conflict)"); + } } /* The track_list_node emptyness is used to check if the node is * already added to track list, so initialize it again here. */ diff --git a/tests/ovn.at b/tests/ovn.at index 745fcb8d0..8f7dec55d 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -13333,6 +13333,220 @@ grep conjunction.*conjunction.*conjunction | wc -l`]) OVN_CLEANUP([hv1]) AT_CLEANUP +AT_SETUP([ovn -- Superseeding ACLs with conjunction]) +ovn_start + +ovn-nbctl ls-add ls1 + +ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +ovn-nbctl lsp-add ls1 ls1-lp2 \ +-- lsp-set-addresses ls1-lp2 "f0:00:00:00:00:02" + +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-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +ovs-vsctl -- add-port br-int hv1-vif2 -- \ + set interface hv1-vif2 external-ids:iface-id=ls1-lp2 \ + options:tx_pcap=hv1/vif2-tx.pcap \ + options:rxq_pcap=hv1/vif2-rx.pcap \ + ofport-request=2 + +# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT... +# +# This shell function causes an ip packet to be received on INPORT. +# The packet's content has Ethernet destination DST and source SRC +# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits). +# The OUTPORTs (zero or more) list the VIFs on which the packet should +# be received. INPORT and the OUTPORTs are specified as logical switch +# port numbers, e.g. 11 for vif11. +test_ip() { + # This packet has bad checksums but logical L3 routing doesn't check. + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 + local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ +${dst_ip}0035111100080000 + shift; shift; shift; shift; shift + as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet + for outport; do + echo $packet >> $outport.expected + done +} + +ip_to_hex() { + printf "%02x%02x%02x%02x" "$@" +} + +reset_pcap_file() { + local iface=$1 + local pcap_file=$2 + ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ +options:rxq_pcap=dummy-rx.pcap + rm -f ${pcap_file}*.pcap + ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ +options:rxq_pcap=${pcap_file}-rx.pcap +} + +# Add a default deny ACL and an allow ACL for specific IP traffic. +ovn-nbctl acl-add ls1 to-lport 2 'arp' allow +ovn-nbctl acl-add ls1 to-lport 1 'ip4' drop +ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow +ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.42) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow +ovn-nbctl --wait=hv sync + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. +for src in `seq 1 2`; do + for dst in `seq 3 4`; do + sip=`ip_to_hex 10 0 0 $src` + dip=`ip_to_hex 10 0 0 $dst` + + test_ip 1 f00000000001 f00000000002 $sip $dip 2 + done +done + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped. +dip=`ip_to_hex 10 0 0 5` +for src in `seq 1 2`; do + sip=`ip_to_hex 10 0 0 $src` + + test_ip 1 f00000000001 f00000000002 $sip $dip +done + +cat 2.expected > expout +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets +AT_CHECK([cat 2.packets], [0], [expout]) +reset_pcap_file hv1-vif2 hv1/vif2 +rm -f 2.packets +> 2.expected + +# Add two less restrictive allow ACLs for src IP 10.0.0.1. +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' allow +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow +ovn-nbctl --wait=hv sync + +# Check OVS flows, the less restrictive flows should have been installed. +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ + grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +]) + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. +for src in `seq 1 2`; do + for dst in `seq 3 4`; do + sip=`ip_to_hex 10 0 0 $src` + dip=`ip_to_hex 10 0 0 $dst` + + test_ip 1 f00000000001 f00000000002 $sip $dip 2 + done +done + +# Traffic 10.0.0.2 -> 10.0.0.5 should be dropped. +sip=`ip_to_hex 10 0 0 2` +dip=`ip_to_hex 10 0 0 5` +test_ip 1 f00000000001 f00000000002 $sip $dip + +# Traffic 10.0.0.1 -> 10.0.0.5 should be allowed. +sip=`ip_to_hex 10 0 0 1` +dip=`ip_to_hex 10 0 0 5` +test_ip 1 f00000000001 f00000000002 $sip $dip 2 + +cat 2.expected > expout +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets +AT_CHECK([cat 2.packets], [0], [expout]) +reset_pcap_file hv1-vif2 hv1/vif2 +rm -f 2.packets +> 2.expected + +#sleep infinity + +# Remove the first less restrictive allow ACL. +ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' +ovn-nbctl --wait=hv sync + +# Check OVS flows, the second less restrictive allow ACL should have been installed. +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ + grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +]) + +# Remove the less restrictive allow ACL. +ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1' +ovn-nbctl --wait=hv sync + +# Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ + grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2),conjunction(3,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +]) + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. +for src in `seq 1 2`; do + for dst in `seq 3 4`; do + sip=`ip_to_hex 10 0 0 $src` + dip=`ip_to_hex 10 0 0 $dst` + + test_ip 1 f00000000001 f00000000002 $sip $dip 2 + done +done + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped. +dip=`ip_to_hex 10 0 0 5` +for src in `seq 1 2`; do + sip=`ip_to_hex 10 0 0 $src` + + test_ip 1 f00000000001 f00000000002 $sip $dip +done + +cat 2.expected > expout +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets +AT_CHECK([cat 2.packets], [0], [expout]) + +# Re-add the less restrictive allow ACL for src IP 10.0.0.1 +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow +ovn-nbctl --wait=hv sync + +# Check OVS flows, the less restrictive flows should have been installed. +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ + grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + # 3 hypervisors, one logical switch, 3 logical ports per hypervisor AT_SETUP([ovn -- L2 Drop and Allow ACL w/ Stateful ACL]) ovn_start From patchwork Tue Nov 10 09:37:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441569 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.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7P932DZz9sRf for ; Thu, 18 Feb 2021 19:37:17 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id D8CD086DE7; Thu, 18 Feb 2021 08:37:15 +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 xymtzQJU5W11; Thu, 18 Feb 2021 08:37:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id D61648666A; Thu, 18 Feb 2021 08:37:13 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B9A3BC0010; Thu, 18 Feb 2021 08:37:13 +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 74134C000D for ; Thu, 18 Feb 2021 08:37:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 62A6F8601E for ; Thu, 18 Feb 2021 08:37:12 +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 9vcy3yUUx3Mo for ; Thu, 18 Feb 2021 08:37:11 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id BBB8086DDB for ; Thu, 18 Feb 2021 08:36:48 +0000 (UTC) X-Mailbox-Line: From c7fa80e81e94a1e2562d8d43a4fc83e9b4fc3ce6 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 10 Nov 2020 10:37:34 +0100 Subject: [ovs-dev] [PATCH ovn branch-20.06 13/15] ovn.at: Make some of the tests more predictable. 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" Fix some of the race conditions that are present in the current OVN test suite. Signed-off-by: Dumitru Ceara Signed-off-by: Ben Pfaff (cherry picked from commit d6594a4695084e1b0f4fb3170547942876d3a81a) Signed-off-by: Frode Nordahl --- tests/ovn.at | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index 8f7dec55d..b38f6c9bc 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -4045,6 +4045,7 @@ for i in 1 2 3; do ovn-nbctl lsp-set-addresses lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr" ovn-nbctl lsp-set-port-security lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr" fi + OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp$i$j` = xup]) done done @@ -13435,14 +13436,15 @@ ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl + grep "priority=1003" | awk '{print $7 " " $8}' | \ + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. @@ -13480,14 +13482,15 @@ ovn-nbctl --wait=hv sync # Check OVS flows, the second less restrictive allow ACL should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl + grep "priority=1003" | awk '{print $7 " " $8}' | \ + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Remove the less restrictive allow ACL. @@ -13496,14 +13499,15 @@ ovn-nbctl --wait=hv sync # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl + grep "priority=1003" | awk '{print $7 " " $8}' | \ + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2),conjunction(3,2/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. @@ -13534,14 +13538,15 @@ ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl + grep "priority=1003" | awk '{print $7 " " $8}' | \ + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) OVN_CLEANUP([hv1]) @@ -14139,6 +14144,7 @@ ovn-nbctl set Logical_Switch_Port ls1-lr0 type=router \ # Create HA chassis group ovn-nbctl ha-chassis-group-add hagrp1 ovn-nbctl ha-chassis-group-add-chassis hagrp1 hv1 30 +ovn-nbctl --wait=sb sync hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name="hagrp1"` From patchwork Mon Dec 7 20:30:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441571 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7Pp4tB7z9sRf for ; Thu, 18 Feb 2021 19:37:50 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 31221872C9; Thu, 18 Feb 2021 08:37:49 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lDudtg+KCpSd; Thu, 18 Feb 2021 08:37:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id ABD3B872BF; Thu, 18 Feb 2021 08:37:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 908E6C000E; Thu, 18 Feb 2021 08:37:48 +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 81F7CC000D for ; Thu, 18 Feb 2021 08:37:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 6FC798666A for ; Thu, 18 Feb 2021 08:37:47 +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 TIaRQz3xzosW for ; Thu, 18 Feb 2021 08:37:47 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id BCDC386DDE for ; Thu, 18 Feb 2021 08:37:21 +0000 (UTC) X-Mailbox-Line: From 15a82bc8c4ef2b4a4cec1f87d95c788f0ee8fbd5 Mon Sep 17 00:00:00 2001 Message-Id: <15a82bc8c4ef2b4a4cec1f87d95c788f0ee8fbd5.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Mon, 7 Dec 2020 21:30:52 +0100 Subject: [ovs-dev] [PATCH ovn branch-20.06 14/15] tests: Add ofctl_strip_all() to filter OVS flow outputs. 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" Extend the already existing ofctl_strip() to also ignore n_packets, n_bytes, cookie. Use some helper functions from tests/system-userspace-packet-type-aware.at, which will be removed by a future commit. Signed-off-by: Dumitru Ceara Signed-off-by: Numan Siddique (cherry picked from commit 0879a0aa9f332bdc689769a1bc2f156f1c3149a5) Signed-off-by: Frode Nordahl --- tests/ofproto-macros.at | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index 6c4ff60e7..22999c1ca 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -1,12 +1,27 @@ m4_divert_push([PREPARE_TESTS]) [ +# Strips 'n_packets=...' from ovs-ofctl output. +strip_n_packets () { + sed 's/ n_packets=[0-9]*,//' +} + +# Strips 'n_bytes=...' from ovs-ofctl output. +strip_n_bytes () { + sed 's/ n_bytes=[0-9]*,//' +} + +# Strips 'cookie=...' from ovs-ofctl output. +strip_cookie () { + sed 's/ cookie=0x[0-9a-fA-F]*,//' +} + # Strips out uninteresting parts of ovs-ofctl output, as well as parts # that vary from one run to another. ofctl_strip () { sed ' s/ (xid=0x[0-9a-fA-F]*)// s/ duration=[0-9.]*s,// -s/ cookie=0x0,// +s/ cookie=0,// s/ table=0,// s/ n_packets=0,// s/ n_bytes=0,// @@ -19,6 +34,12 @@ s/dir\/[0-9]*\/br0.mgmt/dir\/XXXX\/br0.mgmt/ ' } +# Strips out uninteresting parts of ovs-ofctl output, including n_packets=.. +# n_bytes=.. +ofctl_strip_all () { + ofctl_strip | strip_n_packets | strip_n_bytes | strip_cookie +} + # Filter (multiline) vconn debug messages from ovs-vswitchd.log. # Use with vconn_sub() and ofctl_strip() print_vconn_debug () { awk -F\| < ovs-vswitchd.log ' From patchwork Mon Dec 7 20:31:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1441572 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.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Dh7QN1HZbz9sRf for ; Thu, 18 Feb 2021 19:38:20 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id A20A4872C3; Thu, 18 Feb 2021 08:38:18 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O8USoSrdcp32; Thu, 18 Feb 2021 08:38:17 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id A8E56872A6; Thu, 18 Feb 2021 08:38:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8C8FFC0010; Thu, 18 Feb 2021 08:38:17 +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 3D406C0010 for ; Thu, 18 Feb 2021 08:38:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 1CEFD86974 for ; Thu, 18 Feb 2021 08:38:17 +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 dPELopIRbEoJ for ; Thu, 18 Feb 2021 08:38:16 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from ti0189a330-0925.bb.online.no (ti0189a330-0925.bb.online.no [88.88.218.161]) by whitealder.osuosl.org (Postfix) with ESMTP id AD11D86D6F for ; Thu, 18 Feb 2021 08:37:55 +0000 (UTC) X-Mailbox-Line: From 8d93850d671218fd3737e8bd1b05e71d872be382 Mon Sep 17 00:00:00 2001 Message-Id: <8d93850d671218fd3737e8bd1b05e71d872be382.1613635760.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Dumitru Ceara To: dev@openvswitch.org Date: Mon, 7 Dec 2020 21:31:25 +0100 Subject: [ovs-dev] [PATCH ovn branch-20.06 15/15] tests: Fix test "ovn -- Superseding ACLs with conjunction". 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 test was checking the output of "ovs-ofctl dump-flows" without taking into account that some fields don't have predictable values, e.g., hard_age. Instead, use ofctl_strip_all(). Fixes: 986b3d5e4ad6 ("ofctrl.c: Add a predictable resolution for conflicting flow actions.") Reported-by: Numan Siddique Signed-off-by: Dumitru Ceara Signed-off-by: Numan Siddique (cherry picked from commit 6e3d69e6b5153e26c869a03ec7dd1aaa40488005) Signed-off-by: Frode Nordahl --- tests/ovn.at | 72 ++++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index b38f6c9bc..24f83f2c5 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -13435,16 +13435,16 @@ ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | \ +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ + grep "priority=1003" | \ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl -priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() + table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. @@ -13481,16 +13481,16 @@ ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' ovn-nbctl --wait=hv sync # Check OVS flows, the second less restrictive allow ACL should have been installed. -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | \ +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ + grep "priority=1003" | \ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl -priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() + table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Remove the less restrictive allow ACL. @@ -13498,16 +13498,16 @@ ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1' ovn-nbctl --wait=hv sync # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | \ +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ + grep "priority=1003" | \ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl -priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() + table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. @@ -13537,16 +13537,16 @@ ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow ovn-nbctl --wait=hv sync # Check OVS flows, the less restrictive flows should have been installed. -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ - grep "priority=1003" | awk '{print $7 " " $8}' | \ +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ + grep "priority=1003" | \ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl -priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) -priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() -priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() + table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() ]) OVN_CLEANUP([hv1])