From patchwork Wed Feb 9 06:37:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590198 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=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtqv04BjHz9sCD for ; Wed, 9 Feb 2022 17:37:46 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 2BB328319F; Wed, 9 Feb 2022 06:37:44 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id in4sPa8k476o; Wed, 9 Feb 2022 06:37:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 5BF3083133; Wed, 9 Feb 2022 06:37:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 30F4CC0011; Wed, 9 Feb 2022 06:37:42 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id A156DC000B for ; Wed, 9 Feb 2022 06:37:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 8F2A983143 for ; Wed, 9 Feb 2022 06:37:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FlRhC8iOln_L for ; Wed, 9 Feb 2022 06:37:40 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp1.osuosl.org (Postfix) with ESMTPS id 5AC6883133 for ; Wed, 9 Feb 2022 06:37:40 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 8A388FF804; Wed, 9 Feb 2022 06:37:37 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:16 -0800 Message-Id: <20220209063726.1134827-2-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 01/11] expr.c: Use expr_destroy and expr_clone instead of free and xmemdup. 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" Use the functions for safer memory operation. Signed-off-by: Han Zhou --- lib/expr.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/expr.c b/lib/expr.c index e3f6bb892..5fc6c1ce9 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -186,7 +186,7 @@ expr_combine(enum expr_type type, struct expr *a, struct expr *b) } else if (a->type == type) { if (b->type == type) { ovs_list_splice(&a->andor, b->andor.next, &b->andor); - free(b); + expr_destroy(b); } else { ovs_list_push_back(&a->andor, &b->node); } @@ -210,7 +210,7 @@ expr_insert_andor(struct expr *andor, struct expr *before, struct expr *new) /* Conjunction junction, what's your function? */ } ovs_list_splice(&before->node, new->andor.next, &new->andor); - free(new); + expr_destroy(new); } else { ovs_list_insert(&before->node, &new->node); } @@ -276,11 +276,11 @@ expr_fix_andor(struct expr *expr, bool short_circuit) if (ovs_list_is_short(&expr->andor)) { if (ovs_list_is_empty(&expr->andor)) { - free(expr); + expr_destroy(expr); return expr_create_boolean(!short_circuit); } else { - sub = expr_from_node(ovs_list_front(&expr->andor)); - free(expr); + sub = expr_from_node(ovs_list_pop_front(&expr->andor)); + expr_destroy(expr); return sub; } } else { @@ -2096,7 +2096,7 @@ expr_simplify_relational(struct expr *expr) * and similarly for "tcp.dst <= 1234". */ struct expr *new = NULL; if (eq) { - new = xmemdup(expr, sizeof *expr); + new = expr_clone(expr); new->cmp.relop = EXPR_R_EQ; } @@ -2105,7 +2105,7 @@ expr_simplify_relational(struct expr *expr) z = bitwise_scan(value, sizeof *value, lt, z + 1, end)) { struct expr *e; - e = xmemdup(expr, sizeof *expr); + e = expr_clone(expr); e->cmp.relop = EXPR_R_EQ; bitwise_toggle_bit(&e->cmp.value, sizeof e->cmp.value, z); bitwise_zero(&e->cmp.value, sizeof e->cmp.value, start, z - start); @@ -2324,7 +2324,7 @@ crush_and_string(struct expr *expr, const struct expr_symbol *symbol) expr_destroy(expr); return new; } - free(new); + expr_destroy(new); break; case EXPR_T_CONDITION: OVS_NOT_REACHED(); @@ -2463,14 +2463,14 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) expr_destroy(sub); } } - free(disjuncts); - free(expr); + expr_destroy(disjuncts); + expr_destroy(expr); if (ovs_list_is_empty(&or->andor)) { - free(or); + expr_destroy(or); return expr_create_boolean(false); } else if (ovs_list_is_short(&or->andor)) { struct expr *cmp = expr_from_node(ovs_list_pop_front(&or->andor)); - free(or); + expr_destroy(or); return cmp; } else { return crush_cmps(or, symbol); @@ -2517,15 +2517,15 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) } expr_destroy(as); expr_destroy(bs); - free(new); + expr_destroy(new); if (ovs_list_is_empty(&or->andor)) { expr_destroy(expr); - free(or); + expr_destroy(or); return expr_create_boolean(false); } else if (ovs_list_is_short(&or->andor)) { struct expr *cmp = expr_from_node(ovs_list_pop_front(&or->andor)); - free(or); + expr_destroy(or); if (ovs_list_is_empty(&expr->andor)) { expr_destroy(expr); return crush_cmps(cmp, symbol); @@ -2675,7 +2675,7 @@ expr_sort(struct expr *expr) qsort(subs, n, sizeof *subs, compare_expr_sort); ovs_list_init(&expr->andor); - free(expr); + expr_destroy(expr); expr = NULL; for (i = 0; i < n; ) { @@ -2708,7 +2708,7 @@ expr_sort(struct expr *expr) expr = crushed; break; } else { - free(crushed); + expr_destroy(crushed); } } else { expr = expr_combine(EXPR_T_AND, expr, crushed); @@ -2754,8 +2754,8 @@ expr_normalize_and(struct expr *expr) } } if (ovs_list_is_short(&expr->andor)) { - struct expr *sub = expr_from_node(ovs_list_front(&expr->andor)); - free(expr); + struct expr *sub = expr_from_node(ovs_list_pop_front(&expr->andor)); + expr_destroy(expr); return sub; } @@ -2813,7 +2813,7 @@ expr_normalize_or(struct expr *expr) expr_destroy(expr); return new; } - free(new); + expr_destroy(new); } else { expr_insert_andor(expr, next, new); } @@ -2823,12 +2823,12 @@ expr_normalize_or(struct expr *expr) } } if (ovs_list_is_empty(&expr->andor)) { - free(expr); + expr_destroy(expr); return expr_create_boolean(false); } if (ovs_list_is_short(&expr->andor)) { struct expr *e = expr_from_node(ovs_list_pop_front(&expr->andor)); - free(expr); + expr_destroy(expr); return e; } From patchwork Wed Feb 9 06:37:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590199 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=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtqv33hcWz9sCD for ; Wed, 9 Feb 2022 17:37:51 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id F3F4F60F7E; Wed, 9 Feb 2022 06:37:47 +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 ukLqOddEUOZK; Wed, 9 Feb 2022 06:37:46 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 1589660F75; Wed, 9 Feb 2022 06:37:45 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 54FFEC007E; Wed, 9 Feb 2022 06:37:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id A3AF4C0039 for ; Wed, 9 Feb 2022 06:37:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 7E23860EB2 for ; Wed, 9 Feb 2022 06:37:42 +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 KO9cn_MGlXTT for ; Wed, 9 Feb 2022 06:37:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp3.osuosl.org (Postfix) with ESMTPS id 5FA2460E2A for ; Wed, 9 Feb 2022 06:37:41 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id EDA1BFF802; Wed, 9 Feb 2022 06:37:38 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:17 -0800 Message-Id: <20220209063726.1134827-3-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 02/11] ofctrl.c: Combine remove_flows_from_sb_to_flow and ofctrl_flood_remove_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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" There are redundant logic between these functions. Refactor and combine them. Signed-off-by: Han Zhou --- controller/ofctrl.c | 128 ++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 08fcfed8b..bcd6cea79 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -257,6 +257,11 @@ static uint32_t ovn_flow_match_hash(const struct ovn_flow *); static char *ovn_flow_to_string(const struct ovn_flow *); static void ovn_flow_log(const struct ovn_flow *, const char *action); +static void remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *, + struct sb_to_flow *, + const char *log_msg, + struct hmap *flood_remove_nodes); + /* OpenFlow connection to the switch. */ static struct rconn *swconn; @@ -1165,36 +1170,6 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, } } -/* Remove ovn_flows for the given "sb_to_flow" node in the uuid_flow_table. - * Optionally log the message for each flow that is acturally removed, if - * log_msg is not NULL. */ -static void -remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, - struct sb_to_flow *stf, - const char *log_msg) -{ - struct sb_flow_ref *sfr, *next; - LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { - ovs_list_remove(&sfr->sb_list); - ovs_list_remove(&sfr->flow_list); - struct desired_flow *f = sfr->flow; - mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr); - free(sfr); - - if (ovs_list_is_empty(&f->references)) { - if (log_msg) { - ovn_flow_log(&f->flow, log_msg); - } - hmap_remove(&flow_table->match_flow_table, - &f->match_hmap_node); - track_or_destroy_for_flow_del(flow_table, f); - } - } - hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); - mem_stats.sb_flow_ref_usage -= sb_to_flow_size(stf); - free(stf); -} - void ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, const struct uuid *sb_uuid) @@ -1202,7 +1177,8 @@ ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, sb_uuid); if (stf) { - remove_flows_from_sb_to_flow(flow_table, stf, "ofctrl_remove_flow"); + remove_flows_from_sb_to_flow(flow_table, stf, "ofctrl_remove_flow", + NULL); } /* remove any related group and meter info */ @@ -1243,32 +1219,73 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, return; } + remove_flows_from_sb_to_flow(flow_table, stf, "flood remove", + flood_remove_nodes); +} + +void +ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, + struct hmap *flood_remove_nodes) +{ + struct ofctrl_flood_remove_node *ofrn; + int i, n = 0; + + /* flood_remove_flows_for_sb_uuid() will modify the 'flood_remove_nodes' + * hash map by inserting new items, so we can't use it for iteration. + * Copying the sb_uuids into an array. */ + struct uuid *sb_uuids; + sb_uuids = xmalloc(hmap_count(flood_remove_nodes) * sizeof *sb_uuids); + HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { + sb_uuids[n++] = ofrn->sb_uuid; + } + + for (i = 0; i < n; i++) { + flood_remove_flows_for_sb_uuid(flow_table, &sb_uuids[i], + flood_remove_nodes); + } + free(sb_uuids); + + /* remove any related group and meter info */ + HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { + ovn_extend_table_remove_desired(groups, &ofrn->sb_uuid); + ovn_extend_table_remove_desired(meters, &ofrn->sb_uuid); + } +} + +/* Remove ovn_flows for the given "sb_to_flow" node in the uuid_flow_table. + * Optionally log the message for each flow that is acturally removed, if + * log_msg is not NULL. */ +static void +remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, + struct sb_to_flow *stf, + const char *log_msg, + struct hmap *flood_remove_nodes) +{ /* ovn_flows that have other references and waiting to be removed. */ struct ovs_list to_be_removed = OVS_LIST_INITIALIZER(&to_be_removed); /* 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 desired_flow *f = sfr->flow; - ovn_flow_log(&f->flow, "flood remove"); - ovs_list_remove(&sfr->sb_list); ovs_list_remove(&sfr->flow_list); + struct desired_flow *f = sfr->flow; mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr); free(sfr); ovs_assert(ovs_list_is_empty(&f->list_node)); if (ovs_list_is_empty(&f->references)) { - /* This is to optimize the case when most flows have only - * one referencing sb_uuid, so to_be_removed list should - * be empty in most cases. */ + if (log_msg) { + ovn_flow_log(&f->flow, log_msg); + } hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); track_or_destroy_for_flow_del(flow_table, f); - } else { + } else if (flood_remove_nodes) { ovs_list_insert(&to_be_removed, &f->list_node); } } + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); mem_stats.sb_flow_ref_usage -= sb_to_flow_size(stf); free(stf); @@ -1298,40 +1315,13 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, free(sfr); } ovs_list_remove(&f->list_node); + if (log_msg) { + ovn_flow_log(&f->flow, log_msg); + } hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); track_or_destroy_for_flow_del(flow_table, f); } - -} - -void -ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, - struct hmap *flood_remove_nodes) -{ - struct ofctrl_flood_remove_node *ofrn; - int i, n = 0; - - /* flood_remove_flows_for_sb_uuid() will modify the 'flood_remove_nodes' - * hash map by inserting new items, so we can't use it for iteration. - * Copying the sb_uuids into an array. */ - struct uuid *sb_uuids; - sb_uuids = xmalloc(hmap_count(flood_remove_nodes) * sizeof *sb_uuids); - HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { - sb_uuids[n++] = ofrn->sb_uuid; - } - - for (i = 0; i < n; i++) { - flood_remove_flows_for_sb_uuid(flow_table, &sb_uuids[i], - flood_remove_nodes); - } - free(sb_uuids); - - /* remove any related group and meter info */ - HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { - ovn_extend_table_remove_desired(groups, &ofrn->sb_uuid); - ovn_extend_table_remove_desired(meters, &ofrn->sb_uuid); - } } /* flow operations. */ @@ -1607,7 +1597,7 @@ ovn_desired_flow_table_clear(struct ovn_desired_flow_table *flow_table) struct sb_to_flow *stf, *next; HMAP_FOR_EACH_SAFE (stf, next, hmap_node, &flow_table->uuid_flow_table) { - remove_flows_from_sb_to_flow(flow_table, stf, NULL); + remove_flows_from_sb_to_flow(flow_table, stf, NULL, NULL); } } From patchwork Wed Feb 9 06:37:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590210 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=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtr5v5Wyvz9sCD for ; Wed, 9 Feb 2022 17:47:15 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 2E32683265; Wed, 9 Feb 2022 06:47:13 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Srs-_enXtb9V; Wed, 9 Feb 2022 06:47:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 6E6BC8317B; Wed, 9 Feb 2022 06:47:08 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7A16EC0072; Wed, 9 Feb 2022 06:47:06 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 97A79C000B for ; Wed, 9 Feb 2022 06:47:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 860E082BE5 for ; Wed, 9 Feb 2022 06:47:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yIzNYprzL_jB for ; Wed, 9 Feb 2022 06:47:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from mslow1.mail.gandi.net (mslow1.mail.gandi.net [217.70.178.240]) by smtp1.osuosl.org (Postfix) with ESMTPS id C394082BC3 for ; Wed, 9 Feb 2022 06:47:02 +0000 (UTC) Received: from relay9-d.mail.gandi.net (unknown [IPv6:2001:4b98:dc4:8::229]) by mslow1.mail.gandi.net (Postfix) with ESMTP id C0689C58B4 for ; Wed, 9 Feb 2022 06:37:45 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 54BA4FF805; Wed, 9 Feb 2022 06:37:40 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:18 -0800 Message-Id: <20220209063726.1134827-4-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 03/11] ovn-controller: Track individual IP information of address set during lflow parsing. 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" This patch tracks individual address information when parsing address sets from logical flows, and link to the corresponding desired flow resulted from the IP. Signed-off-by: Han Zhou --- controller/lflow.c | 19 ++++-- controller/ofctrl.c | 130 ++++++++++++++++++++++++++++++++++++++---- controller/ofctrl.h | 19 ++++-- controller/physical.c | 2 +- include/ovn/expr.h | 9 +++ lib/expr.c | 81 ++++++++++++++++++++------ 6 files changed, 224 insertions(+), 36 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index b976c7d56..965e91cce 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -692,13 +692,23 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, } } } + + struct addrset_info as_info = { + .name = m->as_name, + .ip = m->as_ip, + .mask = m->as_mask + }; if (!m->n) { ofctrl_add_flow_metered(l_ctx_out->flow_table, ptable, lflow->priority, lflow->header_.uuid.parts[0], &m->match, &ofpacts, &lflow->header_.uuid, - ctrl_meter_id); + ctrl_meter_id, + as_info.name ? &as_info : NULL); } else { + if (m->n > 1) { + ovs_assert(!as_info.name); + } uint64_t conj_stubs[64 / 8]; struct ofpbuf conj; @@ -716,7 +726,8 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, ofctrl_add_or_append_flow(l_ctx_out->flow_table, ptable, lflow->priority, 0, &m->match, &conj, &lflow->header_.uuid, - ctrl_meter_id); + ctrl_meter_id, + as_info.name ? &as_info : NULL); ofpbuf_uninit(&conj); } } @@ -1524,7 +1535,7 @@ add_lb_ct_snat_hairpin_dp_flows(struct ovn_controller_lb *lb, ofctrl_add_or_append_flow(flow_table, OFTABLE_CT_SNAT_HAIRPIN, 200, lb->slb->header_.uuid.parts[0], &dp_match, &dp_acts, &lb->slb->header_.uuid, - NX_CTLR_NO_METER); + NX_CTLR_NO_METER, NULL); } ofpbuf_uninit(&dp_acts); @@ -1698,7 +1709,7 @@ add_lb_ct_snat_hairpin_vip_flow(struct ovn_controller_lb *lb, ofctrl_add_or_append_flow(flow_table, OFTABLE_CT_SNAT_HAIRPIN, priority, lb->slb->header_.uuid.parts[0], &match, &ofpacts, &lb->slb->header_.uuid, - NX_CTLR_NO_METER); + NX_CTLR_NO_METER, NULL); ofpbuf_uninit(&ofpacts); } diff --git a/controller/ofctrl.c b/controller/ofctrl.c index bcd6cea79..7671a3b7a 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -165,15 +165,35 @@ struct sb_to_flow { 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 addrsets; /* A list of struct sb_addrset_ref. */ }; struct sb_flow_ref { 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 ovs_list flow_list; /* List node in sb_to_flow.flows. */ + struct ovs_list as_ip_flow_list; /* List node in ip_to_flow_node.flows. */ struct desired_flow *flow; struct uuid sb_uuid; }; +struct sb_addrset_ref { + struct ovs_list list_node; /* List node in sb_to_flow.addrsets. */ + char *name; /* Name of the address set. */ + struct hmap ip_to_flow_map; /* map from IPs in the address set to flows. + Each node is struct ip_to_flow_node. */ +}; + +struct ip_to_flow_node { + struct hmap_node hmap_node; /* Node in sb_addrset_ref.ip_to_flow_map. */ + struct in6_addr as_ip; + struct in6_addr as_mask; + + /* A list of struct sb_flow_ref. A single IP in an address set can be + * used by multiple flows. e.g., in match: + * ip.src == $as1 && ip.dst == $as1. */ + struct ovs_list flows; +}; + /* An installed flow, in static variable installed_lflows/installed_pflows. * * Installed flows are updated in ofctrl_put for maintaining the flow @@ -1030,9 +1050,26 @@ sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) return NULL; } +static struct ip_to_flow_node * +ip_to_flow_find(struct hmap *ip_to_flow_map, const struct in6_addr *as_ip, + const struct in6_addr *as_mask) +{ + uint32_t hash = hash_bytes(as_ip, sizeof *as_ip, 0); + + struct ip_to_flow_node *itfn; + HMAP_FOR_EACH_WITH_HASH (itfn, hmap_node, hash, ip_to_flow_map) { + if (ipv6_addr_equals(&itfn->as_ip, as_ip) + && ipv6_addr_equals(&itfn->as_mask, as_mask)) { + return itfn; + } + } + return NULL; +} + static void link_flow_to_sb(struct ovn_desired_flow_table *flow_table, - struct desired_flow *f, const struct uuid *sb_uuid) + struct desired_flow *f, const struct uuid *sb_uuid, + const struct addrset_info *as_info) { struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); mem_stats.sb_flow_ref_usage += sb_flow_ref_size(sfr); @@ -1046,10 +1083,48 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table, mem_stats.sb_flow_ref_usage += sb_to_flow_size(stf); stf->sb_uuid = *sb_uuid; ovs_list_init(&stf->flows); + ovs_list_init(&stf->addrsets); hmap_insert(&flow_table->uuid_flow_table, &stf->hmap_node, uuid_hash(sb_uuid)); } ovs_list_insert(&stf->flows, &sfr->flow_list); + + if (!as_info) { + ovs_list_init(&sfr->as_ip_flow_list); + return; + } + + /* link flow to address_set + ip */ + struct sb_addrset_ref *sar; + bool found = false; + LIST_FOR_EACH (sar, list_node, &stf->addrsets) { + if (!strcmp(sar->name, as_info->name)) { + found = true; + break; + } + } + if (!found) { + sar = xmalloc(sizeof *sar); + mem_stats.sb_flow_ref_usage += sizeof *sar; + sar->name = xstrdup(as_info->name); + hmap_init(&sar->ip_to_flow_map); + ovs_list_insert(&stf->addrsets, &sar->list_node); + } + + struct ip_to_flow_node * itfn = ip_to_flow_find(&sar->ip_to_flow_map, + &as_info->ip, + &as_info->mask); + if (!itfn) { + itfn = xmalloc(sizeof *itfn); + mem_stats.sb_flow_ref_usage += sizeof *itfn; + itfn->as_ip = as_info->ip; + itfn->as_mask = as_info->mask; + ovs_list_init(&itfn->flows); + uint32_t hash = hash_bytes(&as_info->ip, sizeof as_info->ip, 0); + hmap_insert(&sar->ip_to_flow_map, &itfn->hmap_node, hash); + } + + ovs_list_insert(&itfn->flows, &sfr->as_ip_flow_list); } /* Flow table interfaces to the rest of ovn-controller. */ @@ -1068,13 +1143,17 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table, void ofctrl_check_and_add_flow_metered(struct ovn_desired_flow_table *flow_table, uint8_t table_id, uint16_t priority, - uint64_t cookie, const struct match *match, + uint64_t cookie, + const struct match *match, const struct ofpbuf *actions, const struct uuid *sb_uuid, - uint32_t meter_id, bool log_duplicate_flow) + uint32_t meter_id, + const struct addrset_info *as_info, + bool log_duplicate_flow) { struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, - match, actions, meter_id); + match, actions, + meter_id); if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { if (log_duplicate_flow) { @@ -1091,7 +1170,7 @@ ofctrl_check_and_add_flow_metered(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); + link_flow_to_sb(flow_table, f, sb_uuid, as_info); track_flow_add_or_modify(flow_table, f); ovn_flow_log(&f->flow, "ofctrl_add_flow"); } @@ -1103,7 +1182,7 @@ ofctrl_add_flow(struct ovn_desired_flow_table *desired_flows, const struct uuid *sb_uuid) { ofctrl_add_flow_metered(desired_flows, table_id, priority, cookie, - match, actions, sb_uuid, NX_CTLR_NO_METER); + match, actions, sb_uuid, NX_CTLR_NO_METER, NULL); } void @@ -1111,11 +1190,12 @@ ofctrl_add_flow_metered(struct ovn_desired_flow_table *desired_flows, uint8_t table_id, uint16_t priority, uint64_t cookie, const struct match *match, const struct ofpbuf *actions, - const struct uuid *sb_uuid, uint32_t meter_id) + const struct uuid *sb_uuid, uint32_t meter_id, + const struct addrset_info *as_info) { ofctrl_check_and_add_flow_metered(desired_flows, table_id, priority, cookie, match, actions, sb_uuid, - meter_id, true); + meter_id, as_info, true); } /* Either add a new flow, or append actions on an existing flow. If the @@ -1127,7 +1207,8 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, const struct match *match, const struct ofpbuf *actions, const struct uuid *sb_uuid, - uint32_t meter_id) + uint32_t meter_id, + const struct addrset_info *as_info) { struct desired_flow *existing; struct desired_flow *f; @@ -1156,11 +1237,20 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, ofpbuf_uninit(&compound); desired_flow_destroy(f); f = existing; + + /* Remove as_info tracking for the existing flow. */ + struct sb_flow_ref *sfr; + LIST_FOR_EACH (sfr, sb_list, &f->references) { + ovs_list_remove(&sfr->as_ip_flow_list); + ovs_list_init(&sfr->as_ip_flow_list); + } + /* Link to sb but don't track the as_info. */ + link_flow_to_sb(desired_flows, f, sb_uuid, NULL); } else { hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node, f->flow.hash); + link_flow_to_sb(desired_flows, f, sb_uuid, as_info); } - link_flow_to_sb(desired_flows, f, sb_uuid); track_flow_add_or_modify(desired_flows, f); if (existing) { @@ -1269,6 +1359,7 @@ 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); + ovs_list_remove(&sfr->as_ip_flow_list); struct desired_flow *f = sfr->flow; mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr); free(sfr); @@ -1286,6 +1377,22 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, } } + struct sb_addrset_ref *sar, *next_sar; + LIST_FOR_EACH_SAFE (sar, next_sar, list_node, &stf->addrsets) { + ovs_list_remove(&sar->list_node); + struct ip_to_flow_node *itfn, *itfn_next; + HMAP_FOR_EACH_SAFE (itfn, itfn_next, hmap_node, &sar->ip_to_flow_map) { + hmap_remove(&sar->ip_to_flow_map, &itfn->hmap_node); + ovs_assert(ovs_list_is_empty(&itfn->flows)); + mem_stats.sb_flow_ref_usage -= sizeof *itfn; + free(itfn); + } + hmap_destroy(&sar->ip_to_flow_map); + mem_stats.sb_flow_ref_usage -= (sizeof *sar + strlen(sar->name) + 1); + free(sar->name); + free(sar); + } + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); mem_stats.sb_flow_ref_usage -= sb_to_flow_size(stf); free(stf); @@ -1300,6 +1407,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, ovs_assert(!ovs_list_is_empty(&f->references)); LIST_FOR_EACH (sfr, sb_list, &f->references) { ovs_list_remove(&sfr->flow_list); + ovs_list_remove(&sfr->as_ip_flow_list); } } LIST_FOR_EACH_SAFE (f, f_next, list_node, &to_be_removed) { diff --git a/controller/ofctrl.h b/controller/ofctrl.h index 014de210d..4ec328c24 100644 --- a/controller/ofctrl.h +++ b/controller/ofctrl.h @@ -71,24 +71,34 @@ char *ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const struct shash *port_groups); /* Flow table interfaces to the rest of ovn-controller. */ + +/* Information of IP of an address set used to track a flow that is generated + * from a logical flow referencing address set(s). */ +struct addrset_info { + const char *name; /* The address set's name. */ + struct in6_addr ip; /* An IP in the address set. */ + struct in6_addr mask; /* The mask of the IP. */ +}; void ofctrl_add_flow(struct ovn_desired_flow_table *, uint8_t table_id, uint16_t priority, uint64_t cookie, const struct match *, const struct ofpbuf *ofpacts, const struct uuid *); -void ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, +void ofctrl_add_or_append_flow(struct ovn_desired_flow_table *, uint8_t table_id, uint16_t priority, - uint64_t cookie, const struct match *match, + uint64_t cookie, const struct match *, const struct ofpbuf *actions, const struct uuid *sb_uuid, - uint32_t meter_id); + uint32_t meter_id, + const struct addrset_info *); void ofctrl_add_flow_metered(struct ovn_desired_flow_table *desired_flows, uint8_t table_id, uint16_t priority, uint64_t cookie, const struct match *match, const struct ofpbuf *actions, const struct uuid *sb_uuid, - uint32_t meter_id); + uint32_t meter_id, + const struct addrset_info *); /* Removes a bundles of flows from the flow table for a specific sb_uuid. The * flows are removed only if they are not referenced by any other sb_uuid(s). @@ -123,6 +133,7 @@ void ofctrl_check_and_add_flow_metered(struct ovn_desired_flow_table *, uint64_t cookie, const struct match *, const struct ofpbuf *ofpacts, const struct uuid *, uint32_t meter_id, + const struct addrset_info *, bool log_duplicate_flow); diff --git a/controller/physical.c b/controller/physical.c index 6bfa2304d..033828d57 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -835,7 +835,7 @@ put_local_common_flows(uint32_t dp_key, put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(ofpacts_p)); ofctrl_check_and_add_flow_metered(flow_table, OFTABLE_SAVE_INPORT, 100, 0, &match, ofpacts_p, hc_uuid, - NX_CTLR_NO_METER, false); + NX_CTLR_NO_METER, NULL, false); } } diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 3b5653f7b..5572a1071 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -367,6 +367,8 @@ bool expr_relop_from_token(enum lex_type type, enum expr_relop *relop); struct expr { struct ovs_list node; /* In parent EXPR_T_AND or EXPR_T_OR if any. */ enum expr_type type; /* Expression type. */ + char *as_name; /* Address set name. Null if it is not an + address set. */ union { /* EXPR_T_CMP. @@ -469,6 +471,11 @@ struct expr_match { struct match match; struct cls_conjunction *conjunctions; size_t n, allocated; + + /* Tracked address set information. */ + char *as_name; + struct in6_addr as_ip; + struct in6_addr as_mask; }; uint32_t expr_to_matches(const struct expr *, @@ -526,6 +533,8 @@ struct expr_constant_set { size_t n_values; /* Number of constants. */ enum expr_constant_type type; /* Type of the constants. */ bool in_curlies; /* Whether the constants were in {}. */ + char *as_name; /* Name of an address set. It is NULL if not + an address set. */ }; bool expr_constant_set_parse(struct lexer *, struct expr_constant_set *); diff --git a/lib/expr.c b/lib/expr.c index 5fc6c1ce9..af083190f 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -160,7 +160,7 @@ expr_relop_test(enum expr_relop relop, int cmp) struct expr * expr_create_andor(enum expr_type type) { - struct expr *e = xmalloc(sizeof *e); + struct expr *e = xzalloc(sizeof *e); e->type = type; ovs_list_init(&e->andor); return e; @@ -190,9 +190,17 @@ expr_combine(enum expr_type type, struct expr *a, struct expr *b) } else { ovs_list_push_back(&a->andor, &b->node); } + if (a->as_name) { + free(a->as_name); + a->as_name = NULL; + } return a; } else if (b->type == type) { ovs_list_push_front(&b->andor, &a->node); + if (b->as_name) { + free(b->as_name); + b->as_name = NULL; + } return b; } else { struct expr *e = expr_create_andor(type); @@ -220,7 +228,7 @@ expr_insert_andor(struct expr *andor, struct expr *before, struct expr *new) struct expr * expr_create_boolean(bool b) { - struct expr *e = xmalloc(sizeof *e); + struct expr *e = xzalloc(sizeof *e); e->type = EXPR_T_BOOLEAN; e->boolean = b; return e; @@ -680,6 +688,10 @@ make_cmp(struct expr_context *ctx, e = expr_combine(r == EXPR_R_EQ ? EXPR_T_OR : EXPR_T_AND, e, make_cmp__(f, r, &cs->values[i])); } + /* Track address set */ + if (r == EXPR_R_EQ && e->type == EXPR_T_OR && cs->as_name) { + e->as_name = xstrdup(cs->as_name); + } exit: expr_constant_set_destroy(cs); return e; @@ -802,6 +814,10 @@ parse_addr_sets(struct expr_context *ctx, struct expr_constant_set *cs, return false; } + if (!cs->n_values) { + cs->as_name = xstrdup(ctx->lexer->token.s); + } + size_t n_values = cs->n_values + addr_sets->n_values; if (n_values >= *allocated_values) { cs->values = xrealloc(cs->values, n_values * sizeof *cs->values); @@ -875,6 +891,13 @@ parse_constant(struct expr_context *ctx, struct expr_constant_set *cs, sizeof *cs->values); } + if (cs->as_name) { + /* Combining other values to the constant set that is tracking an + * address set, so untrack it. */ + free(cs->as_name); + cs->as_name = NULL; + } + if (ctx->lexer->token.type == LEX_T_STRING) { if (!assign_constant_set_type(ctx, cs, EXPR_C_STRING)) { return false; @@ -1057,6 +1080,7 @@ expr_constant_set_destroy(struct expr_constant_set *cs) } } free(cs->values); + free(cs->as_name); } } @@ -1244,6 +1268,7 @@ expr_parse_primary(struct expr_context *ctx, bool *atomic) c.values = cst; c.n_values = 1; c.in_curlies = false; + c.as_name = NULL; return make_cmp(ctx, &f, EXPR_R_NE, &c); } else if (parse_relop(ctx, &r) && parse_constant_set(ctx, &c)) { return make_cmp(ctx, &f, r, &c); @@ -1709,6 +1734,7 @@ expr_symtab_destroy(struct shash *symtab) static struct expr * expr_clone_cmp(struct expr *expr) { + ovs_assert(!expr->as_name); struct expr *new = xmemdup(expr, sizeof *expr); if (!new->cmp.symbol->width) { new->cmp.string = xstrdup(new->cmp.string); @@ -1732,6 +1758,7 @@ expr_clone_andor(struct expr *expr) static struct expr * expr_clone_condition(struct expr *expr) { + ovs_assert(!expr->as_name); struct expr *new = xmemdup(expr, sizeof *expr); new->cond.string = xstrdup(new->cond.string); return new; @@ -1767,6 +1794,8 @@ expr_destroy(struct expr *expr) return; } + free(expr->as_name); + struct expr *sub, *next; switch (expr->type) { @@ -2373,7 +2402,7 @@ crush_and_string(struct expr *expr, const struct expr_symbol *symbol) const char *string; SSET_FOR_EACH (string, &result) { - sub = xmalloc(sizeof *sub); + sub = xzalloc(sizeof *sub); sub->type = EXPR_T_CMP; sub->cmp.relop = EXPR_R_EQ; sub->cmp.symbol = symbol; @@ -2432,7 +2461,7 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) return expr_create_boolean(true); } else { struct expr *cmp; - cmp = xmalloc(sizeof *cmp); + cmp = xzalloc(sizeof *cmp); cmp->type = EXPR_T_CMP; cmp->cmp.symbol = symbol; cmp->cmp.relop = EXPR_R_EQ; @@ -2447,7 +2476,7 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) struct expr *disjuncts = expr_from_node(ovs_list_pop_front(&expr->andor)); struct expr *or; - or = xmalloc(sizeof *or); + or = xzalloc(sizeof *or); or->type = EXPR_T_OR; ovs_list_init(&or->andor); @@ -2483,7 +2512,7 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) struct expr *new = NULL; struct expr *or; - or = xmalloc(sizeof *or); + or = xzalloc(sizeof *or); or->type = EXPR_T_OR; ovs_list_init(&or->andor); @@ -2502,7 +2531,7 @@ crush_and_numeric(struct expr *expr, const struct expr_symbol *symbol) LIST_FOR_EACH (b, node, &bs->andor) { ovs_assert(b->type == EXPR_T_CMP); if (!new) { - new = xmalloc(sizeof *new); + new = xzalloc(sizeof *new); new->type = EXPR_T_CMP; new->cmp.symbol = symbol; new->cmp.relop = EXPR_R_EQ; @@ -2608,6 +2637,9 @@ crush_or(struct expr *expr, const struct expr_symbol *symbol) ovs_list_push_back(&expr->andor, &b->node); } else { expr_destroy(b); + /* Member modified, so untrack address set. */ + free(expr->as_name); + expr->as_name = NULL; } } free(subs); @@ -2659,7 +2691,7 @@ expr_sort(struct expr *expr) ovs_assert(expr->type == EXPR_T_AND); size_t n = ovs_list_size(&expr->andor); - struct expr_sort *subs = xmalloc(n * sizeof *subs); + struct expr_sort *subs = xzalloc(n * sizeof *subs); struct expr *sub; size_t i; @@ -2884,7 +2916,7 @@ static struct expr_match * expr_match_new(const struct match *m, uint8_t clause, uint8_t n_clauses, uint32_t conj_id) { - struct expr_match *match = xmalloc(sizeof *match); + struct expr_match *match = xzalloc(sizeof *match); if (m) { match->match = *m; } else { @@ -2905,6 +2937,14 @@ expr_match_new(const struct match *m, uint8_t clause, uint8_t n_clauses, return match; } +static void +expr_match_destroy(struct expr_match *match) +{ + free(match->as_name); + free(match->conjunctions); + free(match); +} + /* Adds 'match' to hash table 'matches', which becomes the new owner of * 'match'. * @@ -2932,8 +2972,12 @@ expr_match_add(struct hmap *matches, struct expr_match *match) } m->conjunctions[m->n++] = match->conjunctions[0]; } - free(match->conjunctions); - free(match); + if (m->as_name) { + /* m is combined with match. so untracked the address set. */ + free(m->as_name); + m->as_name = NULL; + } + expr_match_destroy(match); return; } } @@ -2994,12 +3038,18 @@ add_disjunction(const struct expr *or, LIST_FOR_EACH (sub, node, &or->andor) { struct expr_match *match = expr_match_new(m, clause, n_clauses, conj_id); + if (or->as_name) { + ovs_assert(sub->type == EXPR_T_CMP); + ovs_assert(sub->cmp.symbol->width); + match->as_name = xstrdup(or->as_name); + match->as_ip = sub->cmp.value.ipv6; + match->as_mask = sub->cmp.mask.ipv6; + } if (constrain_match(sub, lookup_port, aux, &match->match)) { expr_match_add(matches, match); n++; } else { - free(match->conjunctions); - free(match); + expr_match_destroy(match); } } @@ -3082,7 +3132,7 @@ add_cmp_flow(const struct expr *cmp, if (constrain_match(cmp, lookup_port, aux, &m->match)) { expr_match_add(matches, m); } else { - free(m); + expr_match_destroy(m); } } @@ -3214,8 +3264,7 @@ expr_matches_destroy(struct hmap *matches) } HMAP_FOR_EACH_POP (m, hmap_node, matches) { - free(m->conjunctions); - free(m); + expr_match_destroy(m); } hmap_destroy(matches); } From patchwork Wed Feb 9 06:37:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590201 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=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4JtqvB4fPkz9sCD for ; Wed, 9 Feb 2022 17:37:58 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 6D81340961; Wed, 9 Feb 2022 06:37:55 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uUiGAXmbilWt; Wed, 9 Feb 2022 06:37:53 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id E28684091A; Wed, 9 Feb 2022 06:37:51 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id DD1D8C0039; Wed, 9 Feb 2022 06:37:50 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 06D66C000B for ; Wed, 9 Feb 2022 06:37:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 35A4541512 for ; Wed, 9 Feb 2022 06:37:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NG-BsOPN9BOl for ; Wed, 9 Feb 2022 06:37:46 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp4.osuosl.org (Postfix) with ESMTPS id 6CCAA4155A for ; Wed, 9 Feb 2022 06:37:46 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id D8E80FF806; Wed, 9 Feb 2022 06:37:41 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:19 -0800 Message-Id: <20220209063726.1134827-5-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 04/11] ovn-controller.c: Remove unnecessary asserts and useless variables. 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" Signed-off-by: Han Zhou --- controller/ovn-controller.c | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 8631bccbc..28e370c0b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2418,19 +2418,6 @@ en_lflow_output_run(struct engine_node *node, void *data) static bool lflow_output_sb_logical_flow_handler(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ovsrec_open_vswitch_table *ovs_table = - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( - engine_get_input("OVS_open_vswitch", node)); - struct ovsrec_bridge_table *bridge_table = - (struct ovsrec_bridge_table *)EN_OVSDB_GET( - engine_get_input("OVS_bridge", node)); - const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); - ovs_assert(br_int); - struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; @@ -2524,26 +2511,6 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, struct ed_type_port_groups *pg_data = engine_get_input_data("port_groups", node); - struct ovsrec_open_vswitch_table *ovs_table = - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( - engine_get_input("OVS_open_vswitch", node)); - struct ovsrec_bridge_table *bridge_table = - (struct ovsrec_bridge_table *)EN_OVSDB_GET( - engine_get_input("OVS_bridge", node)); - const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); - const char *chassis_id = get_ovs_chassis_id(ovs_table); - - struct ovsdb_idl_index *sbrec_chassis_by_name = - engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); - const struct sbrec_chassis *chassis = NULL; - if (chassis_id) { - chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); - } - - ovs_assert(br_int && chassis); - struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; From patchwork Wed Feb 9 06:37:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590200 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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtqv65MGVz9sCD for ; Wed, 9 Feb 2022 17:37:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 8AB4760F7A; Wed, 9 Feb 2022 06:37:51 +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 yGVk-G1a5W8a; Wed, 9 Feb 2022 06:37:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 98B5B60F7F; Wed, 9 Feb 2022 06:37:49 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 26F31C000B; Wed, 9 Feb 2022 06:37:49 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 20B79C000B for ; Wed, 9 Feb 2022 06:37:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 19BBE41549 for ; Wed, 9 Feb 2022 06:37:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8e0GjOvuSL5C for ; Wed, 9 Feb 2022 06:37:46 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp4.osuosl.org (Postfix) with ESMTPS id 104DE41512 for ; Wed, 9 Feb 2022 06:37:45 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 4D292FF803; Wed, 9 Feb 2022 06:37:42 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:20 -0800 Message-Id: <20220209063726.1134827-6-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 05/11] ovn-controller.c: Refactor init_lflow_ctx. 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" Signed-off-by: Han Zhou --- controller/ovn-controller.c | 56 ++++++++++--------------------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 28e370c0b..e73523ce2 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2191,8 +2191,6 @@ struct ed_type_lflow_output { static void init_lflow_ctx(struct engine_node *node, - struct ed_type_runtime_data *rt_data, - struct ed_type_non_vif_data *non_vif_data, struct ed_type_lflow_output *fo, struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) @@ -2274,6 +2272,12 @@ init_lflow_ctx(struct engine_node *node, ovs_assert(chassis); + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_non_vif_data *non_vif_data = + engine_get_input_data("non_vif_data", node); + struct ed_type_addr_sets *as_data = engine_get_input_data("addr_sets", node); struct shash *addr_sets = &as_data->addr_sets; @@ -2360,11 +2364,6 @@ en_lflow_output_cleanup(void *data) static void en_lflow_output_run(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ovsrec_open_vswitch_table *ovs_table = (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( engine_get_input("OVS_open_vswitch", node)); @@ -2409,7 +2408,7 @@ en_lflow_output_run(struct engine_node *node, void *data) struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); lflow_run(&l_ctx_in, &l_ctx_out); engine_set_node_state(node, EN_UPDATED); @@ -2421,7 +2420,7 @@ lflow_output_sb_logical_flow_handler(struct engine_node *node, void *data) struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); bool handled = lflow_handle_changed_flows(&l_ctx_in, &l_ctx_out); @@ -2457,16 +2456,11 @@ lflow_output_sb_mac_binding_handler(struct engine_node *node, void *data) static bool lflow_output_sb_multicast_group_handler(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ed_type_lflow_output *lfo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, lfo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, lfo, &l_ctx_in, &l_ctx_out); if (!lflow_handle_changed_mc_groups(&l_ctx_in, &l_ctx_out)) { return false; } @@ -2478,16 +2472,11 @@ lflow_output_sb_multicast_group_handler(struct engine_node *node, void *data) static bool lflow_output_sb_port_binding_handler(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ed_type_lflow_output *lfo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, lfo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, lfo, &l_ctx_in, &l_ctx_out); if (!lflow_handle_changed_port_bindings(&l_ctx_in, &l_ctx_out)) { return false; } @@ -2500,11 +2489,6 @@ static bool _lflow_output_resource_ref_handler(struct engine_node *node, void *data, enum ref_type ref_type) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ed_type_addr_sets *as_data = engine_get_input_data("addr_sets", node); @@ -2515,7 +2499,7 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); bool changed; const char *ref_name; @@ -2602,8 +2586,6 @@ lflow_output_runtime_data_handler(struct engine_node *node, { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); /* There is no tracked data. Fall back to full recompute of * flow_output. */ @@ -2623,7 +2605,7 @@ lflow_output_runtime_data_handler(struct engine_node *node, struct lflow_ctx_out l_ctx_out; struct ed_type_lflow_output *fo = data; struct hmap *lbs = NULL; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); struct tracked_datapath *tdp; HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { @@ -2661,15 +2643,10 @@ lflow_output_runtime_data_handler(struct engine_node *node, static bool lflow_output_sb_load_balancer_handler(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); bool handled = lflow_handle_changed_lbs(&l_ctx_in, &l_ctx_out); @@ -2680,15 +2657,10 @@ lflow_output_sb_load_balancer_handler(struct engine_node *node, void *data) static bool lflow_output_sb_fdb_handler(struct engine_node *node, void *data) { - struct ed_type_runtime_data *rt_data = - engine_get_input_data("runtime_data", node); - struct ed_type_non_vif_data *non_vif_data = - engine_get_input_data("non_vif_data", node); - struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; struct lflow_ctx_out l_ctx_out; - init_lflow_ctx(node, rt_data, non_vif_data, fo, &l_ctx_in, &l_ctx_out); + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); bool handled = lflow_handle_changed_fdbs(&l_ctx_in, &l_ctx_out); From patchwork Wed Feb 9 06:37:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590202 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=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4JtqvC6jHQz9sCD for ; Wed, 9 Feb 2022 17:37:59 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 701608333F; Wed, 9 Feb 2022 06:37:57 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qFeu5KYoRE82; Wed, 9 Feb 2022 06:37:55 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 1DD48832C2; Wed, 9 Feb 2022 06:37:54 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C6F71C0011; Wed, 9 Feb 2022 06:37:53 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 28896C000B for ; Wed, 9 Feb 2022 06:37:52 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 3201660F75 for ; Wed, 9 Feb 2022 06:37:49 +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 txssSFbbcV3v for ; Wed, 9 Feb 2022 06:37:47 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp3.osuosl.org (Postfix) with ESMTPS id 7F52B60F7F for ; Wed, 9 Feb 2022 06:37:47 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id AC830FF804; Wed, 9 Feb 2022 06:37:44 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:21 -0800 Message-Id: <20220209063726.1134827-7-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 06/11] ovn-controller: Tracking SB address set updates. 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" This patch tracks the added and removed addresses for each updated address sets. Signed-off-by: Han Zhou --- controller/ovn-controller.c | 174 +++++++++++++++++++++--------------- include/ovn/expr.h | 10 ++- lib/expr.c | 133 ++++++++++++++++++++++++--- 3 files changed, 234 insertions(+), 83 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index e73523ce2..24e09f78f 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1393,7 +1393,7 @@ struct ed_type_addr_sets { bool change_tracked; struct sset new; struct sset deleted; - struct sset updated; + struct shash updated; }; static void * @@ -1406,19 +1406,44 @@ en_addr_sets_init(struct engine_node *node OVS_UNUSED, as->change_tracked = false; sset_init(&as->new); sset_init(&as->deleted); - sset_init(&as->updated); + shash_init(&as->updated); return as; } +struct addr_set_diff { + struct expr_constant_set *added; + struct expr_constant_set *deleted; +}; + +static void +en_addr_sets_clear_tracked_data(void *data) +{ + struct ed_type_addr_sets *as = data; + sset_clear(&as->new); + sset_clear(&as->deleted); + struct shash_node *node, *next; + SHASH_FOR_EACH_SAFE (node, next, &as->updated) { + struct addr_set_diff *asd = node->data; + expr_constant_set_destroy(asd->added); + free(asd->added); + expr_constant_set_destroy(asd->deleted); + free(asd->deleted); + } + shash_clear_free_data(&as->updated); + as->change_tracked = false; +} + static void en_addr_sets_cleanup(void *data) { + en_addr_sets_clear_tracked_data(data); + struct ed_type_addr_sets *as = data; expr_const_sets_destroy(&as->addr_sets); shash_destroy(&as->addr_sets); sset_destroy(&as->new); sset_destroy(&as->deleted); - sset_destroy(&as->updated); + shash_destroy(&as->updated); } /* Iterate address sets in the southbound database. Create and update the @@ -1437,8 +1462,8 @@ addr_sets_init(const struct sbrec_address_set_table *address_set_table, static void addr_sets_update(const struct sbrec_address_set_table *address_set_table, - struct shash *addr_sets, struct sset *new, - struct sset *deleted, struct sset *updated) + struct shash *addr_sets, struct sset *added, + struct sset *deleted, struct shash *updated) { const struct sbrec_address_set *as; SBREC_ADDRESS_SET_TABLE_FOR_EACH_TRACKED (as, address_set_table) { @@ -1446,13 +1471,23 @@ addr_sets_update(const struct sbrec_address_set_table *address_set_table, expr_const_sets_remove(addr_sets, as->name); sset_add(deleted, as->name); } else { - expr_const_sets_add_integers(addr_sets, as->name, - (const char *const *) as->addresses, - as->n_addresses); - if (sbrec_address_set_is_new(as)) { - sset_add(new, as->name); + struct expr_constant_set *cs_old = shash_find_data(addr_sets, + as->name); + if (!cs_old) { + sset_add(added, as->name); + expr_const_sets_add_integers(addr_sets, as->name, + (const char *const *) as->addresses, as->n_addresses); } else { - sset_add(updated, as->name); + /* Find out the diff for the updated address set. */ + struct expr_constant_set *cs_new = + expr_constant_set_create_integers( + (const char *const *) as->addresses, as->n_addresses); + struct addr_set_diff *as_diff = xmalloc(sizeof *as_diff); + expr_constant_set_integers_diff(cs_old, cs_new, + &as_diff->added, + &as_diff->deleted); + shash_add(updated, as->name, as_diff); + expr_const_sets_add(addr_sets, as->name, cs_new); } } } @@ -1463,9 +1498,6 @@ en_addr_sets_run(struct engine_node *node, void *data) { struct ed_type_addr_sets *as = data; - sset_clear(&as->new); - sset_clear(&as->deleted); - sset_clear(&as->updated); expr_const_sets_destroy(&as->addr_sets); struct sbrec_address_set_table *as_table = @@ -1483,10 +1515,6 @@ addr_sets_sb_address_set_handler(struct engine_node *node, void *data) { struct ed_type_addr_sets *as = data; - sset_clear(&as->new); - sset_clear(&as->deleted); - sset_clear(&as->updated); - struct sbrec_address_set_table *as_table = (struct sbrec_address_set_table *)EN_OVSDB_GET( engine_get_input("SB_address_set", node)); @@ -1495,7 +1523,7 @@ addr_sets_sb_address_set_handler(struct engine_node *node, void *data) &as->deleted, &as->updated); if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) || - !sset_is_empty(&as->updated)) { + !shash_is_empty(&as->updated)) { engine_set_node_state(node, EN_UPDATED); } else { engine_set_node_state(node, EN_UNCHANGED); @@ -2486,15 +2514,11 @@ lflow_output_sb_port_binding_handler(struct engine_node *node, void *data) } static bool -_lflow_output_resource_ref_handler(struct engine_node *node, void *data, - enum ref_type ref_type) +lflow_output_addr_sets_handler(struct engine_node *node, void *data) { struct ed_type_addr_sets *as_data = engine_get_input_data("addr_sets", node); - struct ed_type_port_groups *pg_data = - engine_get_input_data("port_groups", node); - struct ed_type_lflow_output *fo = data; struct lflow_ctx_in l_ctx_in; @@ -2503,42 +2527,13 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, bool changed; const char *ref_name; - struct sset *new, *updated, *deleted; - - switch (ref_type) { - case REF_TYPE_ADDRSET: - /* XXX: The change_tracked check may be added to inc-proc - * framework. */ - if (!as_data->change_tracked) { - return false; - } - new = &as_data->new; - updated = &as_data->updated; - deleted = &as_data->deleted; - break; - case REF_TYPE_PORTGROUP: - if (!pg_data->change_tracked) { - return false; - } - new = &pg_data->new; - updated = &pg_data->updated; - deleted = &pg_data->deleted; - break; - case REF_TYPE_PORTBINDING: - /* This ref type is handled in: - * - flow_output_runtime_data_handler - * - flow_output_sb_port_binding_handler. */ - case REF_TYPE_MC_GROUP: - /* This ref type is handled in the - * flow_output_sb_multicast_group_handler. */ - default: - OVS_NOT_REACHED(); + if (!as_data->change_tracked) { + return false; } - - SSET_FOR_EACH (ref_name, deleted) { - if (!lflow_handle_changed_ref(ref_type, ref_name, &l_ctx_in, + SSET_FOR_EACH (ref_name, &as_data->deleted) { + if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name, &l_ctx_in, &l_ctx_out, &changed)) { return false; } @@ -2546,17 +2541,18 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, engine_set_node_state(node, EN_UPDATED); } } - SSET_FOR_EACH (ref_name, updated) { - if (!lflow_handle_changed_ref(ref_type, ref_name, &l_ctx_in, - &l_ctx_out, &changed)) { + struct shash_node *shash_node; + SHASH_FOR_EACH (shash_node, &as_data->updated) { + if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, shash_node->name, + &l_ctx_in, &l_ctx_out, &changed)) { return false; } if (changed) { engine_set_node_state(node, EN_UPDATED); } } - SSET_FOR_EACH (ref_name, new) { - if (!lflow_handle_changed_ref(ref_type, ref_name, &l_ctx_in, + SSET_FOR_EACH (ref_name, &as_data->new) { + if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name, &l_ctx_in, &l_ctx_out, &changed)) { return false; } @@ -2568,16 +2564,54 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, return true; } -static bool -lflow_output_addr_sets_handler(struct engine_node *node, void *data) -{ - return _lflow_output_resource_ref_handler(node, data, REF_TYPE_ADDRSET); -} - static bool lflow_output_port_groups_handler(struct engine_node *node, void *data) { - return _lflow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); + struct ed_type_port_groups *pg_data = + engine_get_input_data("port_groups", node); + + struct ed_type_lflow_output *fo = data; + + struct lflow_ctx_in l_ctx_in; + struct lflow_ctx_out l_ctx_out; + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out); + + bool changed; + const char *ref_name; + + if (!pg_data->change_tracked) { + return false; + } + + SSET_FOR_EACH (ref_name, &pg_data->deleted) { + if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, &l_ctx_in, + &l_ctx_out, &changed)) { + return false; + } + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } + } + SSET_FOR_EACH (ref_name, &pg_data->updated) { + if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, &l_ctx_in, + &l_ctx_out, &changed)) { + return false; + } + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } + } + SSET_FOR_EACH (ref_name, &pg_data->new) { + if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, &l_ctx_in, + &l_ctx_out, &changed)) { + return false; + } + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } + } + + return true; } static bool @@ -3170,7 +3204,7 @@ main(int argc, char *argv[]) ENGINE_NODE(pflow_output, "physical_flow_output"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lflow_output, "logical_flow_output"); ENGINE_NODE(flow_output, "flow_output"); - ENGINE_NODE(addr_sets, "addr_sets"); + ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups"); #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 5572a1071..5b232c246 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -540,6 +540,13 @@ struct expr_constant_set { bool expr_constant_set_parse(struct lexer *, struct expr_constant_set *); void expr_constant_set_format(const struct expr_constant_set *, struct ds *); void expr_constant_set_destroy(struct expr_constant_set *cs); +struct expr_constant_set * expr_constant_set_create_integers( + const char *const *values, size_t n_values); +void expr_constant_set_integers_diff( + struct expr_constant_set *old, + struct expr_constant_set *new, + struct expr_constant_set **p_diff_added, + struct expr_constant_set **p_diff_deleted); /* Constant sets. @@ -553,7 +560,8 @@ void expr_constant_set_destroy(struct expr_constant_set *cs); * integer/masked-integer values. The values that don't qualify * are ignored. */ - +void expr_const_sets_add(struct shash *const_sets, const char *name, + struct expr_constant_set *); void expr_const_sets_add_integers(struct shash *const_sets, const char *name, const char * const *values, size_t n_values); void expr_const_sets_add_strings(struct shash *const_sets, const char *name, diff --git a/lib/expr.c b/lib/expr.c index af083190f..2fdb7e3bb 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -1084,17 +1084,33 @@ expr_constant_set_destroy(struct expr_constant_set *cs) } } -/* Adds an constant set named 'name' to 'const_sets', replacing any existing - * constant set entry with the given name. The 'values' must be strings that +static int +compare_expr_constant_integer_cb(const void *a_, const void *b_) +{ + const union expr_constant *a = a_; + const union expr_constant *b = b_; + + int d = memcmp(&a->value, &b->value, sizeof a->value); + if (d) { + return d; + } + + if (!a->masked && !b->masked) { + return 0; + } else if (a->masked && !b->masked) { + return -1; + } else if (!a->masked && b->masked) { + return 1; + } + return memcmp(&a->mask, &b->mask, sizeof a->mask); +} + +/* Create an integer type constant set. The 'values' must be strings that * can be converted to integers or masked integers, such as IP addresses. * Values that can't be converted are skipped. */ -void -expr_const_sets_add_integers(struct shash *const_sets, const char *name, - const char *const *values, size_t n_values) +struct expr_constant_set * +expr_constant_set_create_integers(const char *const *values, size_t n_values) { - /* Replace any existing entry for this name. */ - expr_const_sets_remove(const_sets, name); - struct expr_constant_set *cs = xzalloc(sizeof *cs); cs->in_curlies = true; cs->n_values = 0; @@ -1122,9 +1138,105 @@ expr_const_sets_add_integers(struct shash *const_sets, const char *name, lexer_destroy(&lex); } + /* Sort the result, so that it is efficient to generate diffs in the + * function expr_constant_set_diff */ + qsort(cs->values, cs->n_values, sizeof *cs->values, + compare_expr_constant_integer_cb); + + return cs; +} + +static void +expr_constant_set_add_value(struct expr_constant_set **p_cs, + union expr_constant *c, size_t *allocated) +{ + struct expr_constant_set *cs = *p_cs; + if (!cs) { + cs = xzalloc(sizeof *cs); + *p_cs = cs; + } + + if (cs->n_values >= *allocated) { + cs->values = x2nrealloc(cs->values, allocated, + sizeof *cs->values); + } + cs->values[cs->n_values++] = *c; +} + +/* Find the differences between old and new. Both old and new must be integer + * type and must be sorted (which is true if they are generated by + * expr_constant_set_create_integers() or expr_const_sets_add_integers(). + * + * The differences, added and deleted elements, are stored in p_diff_added and + * p_diff_deleted respectively. Caller takes the ownership of these. + * + * *p_diff_added and *p_diff_deleted can be NULL, if no such elements found. */ +void +expr_constant_set_integers_diff(struct expr_constant_set *old, + struct expr_constant_set *new, + struct expr_constant_set **p_diff_added, + struct expr_constant_set **p_diff_deleted) +{ + struct expr_constant_set *diff_added = NULL; + struct expr_constant_set *diff_deleted = NULL; + + size_t oi, ni, added_n_allocated, deleted_n_allocated; + added_n_allocated = deleted_n_allocated = 0; + + for (oi = ni = 0; oi < old->n_values && ni < new->n_values;) { + int d = compare_expr_constant_integer_cb(&old->values[oi], + &new->values[ni]); + if (d < 0) { + expr_constant_set_add_value(&diff_deleted, &old->values[oi], + &deleted_n_allocated); + oi++; + } else if (d > 0) { + expr_constant_set_add_value(&diff_added, &new->values[ni], + &added_n_allocated); + ni++; + } else { + oi++; ni++; + } + } + + for (; oi < old->n_values; oi++) { + expr_constant_set_add_value(&diff_deleted, &old->values[oi], + &deleted_n_allocated); + } + + for (; ni < new->n_values; ni++) { + expr_constant_set_add_value(&diff_added, &new->values[ni], + &added_n_allocated); + } + + *p_diff_added = diff_added; + *p_diff_deleted = diff_deleted; +} + + +/* Adds an constant set named 'name' to 'const_sets', replacing any existing + * constant set entry with the given name. */ +void +expr_const_sets_add(struct shash *const_sets, const char *name, + struct expr_constant_set *cs) +{ + expr_const_sets_remove(const_sets, name); shash_add(const_sets, name, cs); } +/* Adds an constant set named 'name' to 'const_sets', replacing any existing + * constant set entry with the given name. The 'values' must be strings that + * can be converted to integers or masked integers, such as IP addresses. + * Values that can't be converted are skipped. */ +void +expr_const_sets_add_integers(struct shash *const_sets, const char *name, + const char *const *values, size_t n_values) +{ + struct expr_constant_set *cs = expr_constant_set_create_integers(values, + n_values); + expr_const_sets_add(const_sets, name, cs); +} + /* Adds an constant set named 'name' to 'const_sets', replacing any existing * constant set entry with the given name. Unlike expr_const_sets_add_integers, * the 'values' will not be converted but stored as is. @@ -1135,9 +1247,6 @@ expr_const_sets_add_strings(struct shash *const_sets, const char *name, const char *const *values, size_t n_values, const struct sset *filter) { - /* Replace any existing entry for this name. */ - expr_const_sets_remove(const_sets, name); - struct expr_constant_set *cs = xzalloc(sizeof *cs); cs->in_curlies = true; cs->n_values = 0; @@ -1154,7 +1263,7 @@ expr_const_sets_add_strings(struct shash *const_sets, const char *name, c->string = xstrdup(values[i]); } - shash_add(const_sets, name, cs); + expr_const_sets_add(const_sets, name, cs); } void From patchwork Wed Feb 9 06:37:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590203 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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4JtqvF6wVFz9sFq for ; Wed, 9 Feb 2022 17:38:01 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id D1B4C60F9B; Wed, 9 Feb 2022 06:37:59 +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 J9-yIrAG0z9M; Wed, 9 Feb 2022 06:37:58 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 3E0E960F9D; Wed, 9 Feb 2022 06:37:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 00C45C007B; Wed, 9 Feb 2022 06:37:56 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 35BD8C007A for ; Wed, 9 Feb 2022 06:37:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1674841549 for ; Wed, 9 Feb 2022 06:37:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7Ft-4Cyc0EA8 for ; Wed, 9 Feb 2022 06:37:50 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp4.osuosl.org (Postfix) with ESMTPS id 5E8E541560 for ; Wed, 9 Feb 2022 06:37:50 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 15D12FF805; Wed, 9 Feb 2022 06:37:45 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:22 -0800 Message-Id: <20220209063726.1134827-8-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 07/11] lflow.c: Set "changed" properly in lflow_handle_changed_ref(). 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" The current code sets changed to true only when a lflow is reprocessed, if the lflow is deleted (not found) the changed would be false, even if we have deleted desired flows. Fortunately this should not cause real problems yet because if a lflow is deleted it should later trigger the lflow_handle_changed_flows(), which always update the engine state. But it is still better to set it properly in lflow_handle_changed_ref() in the first placer. Signed-off-by: Han Zhou --- controller/lflow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/lflow.c b/controller/lflow.c index 965e91cce..0390e348a 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -516,6 +516,7 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, if (ovs_list_is_empty(&lflows_todo)) { return true; } + *changed = true; struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts); struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts); @@ -584,7 +585,6 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, &nd_ra_opts, &controller_event_opts, false, l_ctx_in, l_ctx_out); - *changed = true; } HMAP_FOR_EACH_SAFE (ofrn, ofrn_next, hmap_node, &flood_remove_nodes) { hmap_remove(&flood_remove_nodes, &ofrn->hmap_node); From patchwork Wed Feb 9 06:37:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590211 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=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtr616jVcz9sCD for ; Wed, 9 Feb 2022 17:47:21 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 209D883339; Wed, 9 Feb 2022 06:47:20 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aYUIyNn3fCv6; Wed, 9 Feb 2022 06:47:12 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 14CDB8328E; Wed, 9 Feb 2022 06:47:10 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 66DF9C0082; Wed, 9 Feb 2022 06:47:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE9E4C0072 for ; Wed, 9 Feb 2022 06:47:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 9E09982BFD for ; Wed, 9 Feb 2022 06:47:05 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EIsfVK20JOF6 for ; Wed, 9 Feb 2022 06:47:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from mslow1.mail.gandi.net (mslow1.mail.gandi.net [217.70.178.240]) by smtp1.osuosl.org (Postfix) with ESMTPS id 7586D81ABB for ; Wed, 9 Feb 2022 06:47:02 +0000 (UTC) Received: from relay9-d.mail.gandi.net (unknown [217.70.183.199]) by mslow1.mail.gandi.net (Postfix) with ESMTP id A13F6C5A27 for ; Wed, 9 Feb 2022 06:37:52 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 78E3AFF802; Wed, 9 Feb 2022 06:37:47 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:23 -0800 Message-Id: <20220209063726.1134827-9-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 08/11] ovn-controller: Add tests for different ACL address set usage patterns. 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" Add test cases to prepare for fine-grained address set incremental processing. Also add coverage counters for consider_logical_flow and check in the test cases. Signed-off-by: Han Zhou --- controller/lflow.c | 2 + tests/ovn-controller.at | 1177 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 1179 insertions(+) diff --git a/controller/lflow.c b/controller/lflow.c index 0390e348a..d0b335893 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -41,6 +41,7 @@ VLOG_DEFINE_THIS_MODULE(lflow); COVERAGE_DEFINE(lflow_run); +COVERAGE_DEFINE(consider_logical_flow); /* Symbol table. */ @@ -1081,6 +1082,7 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, } ovs_assert(!dp_group || !dp); + COVERAGE_INC(consider_logical_flow); if (!is_recompute) { ovs_assert(!lflows_processed_find(l_ctx_out->lflows_processed, &lflow->header_.uuid)); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 2f39e5f3e..b16162ef4 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -853,3 +853,1180 @@ OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int | grep table=38 | grep -q "re OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +AT_SETUP([ovn-controller - I-P for address set update: no conjunction]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as1' drop + +# Add IPs to as1 for 10 times, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=drop +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$i +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 9; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}'], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10 actions=drop +]) + fi + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((10 - $i)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add IPs to as1 for 10 times, 2 IPs each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i,10.0.1.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.2 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3 actions=drop +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i * 2)) +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add and remove IPs at the same time. + +# Add 2 and remove 1 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.21,10.0.0.22 -- \ + remove address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 1 and remove 2 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl remove address_set as1 addresses 10.0.0.21,10.0.0.22 -- \ + add address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.21], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.22], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.10], [0], [1 +]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 1 and remove 1 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.21 -- \ + remove address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 2 and remove 2 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.22,10.0.0.23 -- \ + remove address_set as1 addresses 10.0.0.9,10.0.0.8 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.23], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.8], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +# This is similar to the above test but to test conjunction +AT_SETUP([ovn-controller - I-P for address set update: with conjunction]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as1 && tcp && tcp.dst == {111, 222, 333}' drop + +# Add IPs to as1 for 10 times, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 1; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,tp_dst=111 actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,tp_dst=222 actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,tp_dst=333 actions=drop +]) + else + # (1 conj_id flow + 3 tp_dst flows) = 4 extra flows + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i + 4)) +]) + fi + + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=111 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=222 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333 actions=conjunction,2/2) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + elif test "$i" = 9; then + # no conjunction left + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,tp_dst=111 actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,tp_dst=222 actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,tp_dst=333 actions=drop +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((14 - $i)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add IPs to as1 for 10 times, 2 IPs each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i,10.0.1.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.1 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.2 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=111 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=222 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333 actions=conjunction,2/2) +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i * 2 + 4)) +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add and remove IPs at the same time. + +# Add 2 and remove 1 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.21,10.0.0.22 -- \ + remove address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 1 and remove 2 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl remove address_set as1 addresses 10.0.0.21,10.0.0.22 -- \ + add address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.21], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.22], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.10], [0], [1 +]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 1 and remove 1 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.21 -- \ + remove address_set as1 addresses 10.0.0.10 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Add 2 and remove 2 +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.22,10.0.0.23 -- \ + remove address_set as1 addresses 10.0.0.9,10.0.0.8 +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.23], [0], [1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.8], [1], [ignore]) +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9], [1], [ignore]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: multiple ASes used by same lflow]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +ovn-nbctl create address_set name=as2 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as1 && ip4.dst == $as2' drop + +# Add IPs to as1 and as2, with some of the IPs overlapping +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl add address_set as1 addresses 10.0.0.$i -- \ + add address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 1; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,nw_dst=10.0.0.6 actions=drop +]) + else + # (1 conj_id + nw_src * i + nw_dst * i) = 1 + i*2 flows + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i*2 + 1)) +]) + fi + + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.6 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.7 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.8 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,2/2) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1 and as2, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i -- \ + remove address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + elif test "$i" = 9; then + # no conjunction left + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10.0.0.15 actions=drop +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((21 - $i*2)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add 1 IP back to both ASes +check ovn-nbctl add address_set as1 addresses 10.0.0.1 -- \ + add address_set as2 addresses 10.0.0.6 +check ovn-nbctl --wait=hv sync + +# Add IPs to as1 only +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 2 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,nw_dst=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2,nw_dst=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3,nw_dst=10.0.0.6 actions=drop +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$i +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [9 +]) + +# Add 1 more IP back to as2 +check ovn-nbctl add address_set as2 addresses 10.0.0.7 +check ovn-nbctl --wait=hv sync + +# Remove IPs from as1 only +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 9; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}'], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10.0.0.7 actions=drop +]) + elif test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + else + # 2 dst + (10 - i) src + 1 conj_id + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((10 - $i + 3)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: OR on multiple ASes, different fields]) +AT_KEYWORDS([as-i-p]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +ovn-nbctl create address_set name=as2 + +# OR on different fields +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && (ip4.src == $as1 || ip4.dst == $as2)' drop + +# Add IPs to as1 and as2, with some of the IPs overlapping +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl add address_set as1 addresses 10.0.0.$i -- \ + add address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 1; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i*2)) +]) + fi + + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.7 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.8 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=drop +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1 and as2, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i -- \ + remove address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((20 - $i*2)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: OR on multiple ASes, same field]) +AT_KEYWORDS([as-i-p]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +ovn-nbctl create address_set name=as2 + +# OR on the same field +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == {$as1, $as2}' drop + +# Add IPs to as1 and as2, with some of the IPs overlapping +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl add address_set as1 addresses 10.0.0.$i -- \ + add address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 1; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.6 actions=drop +]) + elif test "$i" -lt 6; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i*2)) +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((5 + $i)) +]) + fi + + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.6 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.7 actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.8 actions=drop +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1 and as2, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + j=$(($i + 5)) + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i -- \ + remove address_set as2 addresses 10.0.0.$j + check ovn-nbctl --wait=hv sync + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + elif test "$i" -lt 6; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((15 - $i)) +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((10 - ($i - 5)*2)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: same AS used twice in same lflow]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as1 && ip4.dst == $as1' drop + +# Add IPs to as1 for 10 times, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 1; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1,nw_dst=10.0.0.1 actions=drop +]) + else + # (1 conj_id + nw_src * i + nw_dst * i) = 1 + i*2 flows + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i*2 + 1)) +]) + fi + + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.1 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.2 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.3 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,2/2) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Remove the IPs from as1, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl remove address_set as1 addresses 10.0.0.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 10; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + elif test "$i" = 9; then + # no conjunction left + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10.0.0.10 actions=drop +]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((21 - $i*2)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Add IPs to as1 for 10 times, 2 IPs each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 10); do + check ovn-nbctl add address_set as1 addresses 10.0.0.$i,10.0.1.$i + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.1 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.2 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.3 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.1.1 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.1.2 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.1.3 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3 actions=conjunction,2/2) +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$(($i * 4 + 1)) +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +]) + +# Test the case when there are two references to the same AS but one of the +# references is combined with another IP. +check ovn-nbctl acl-del ls1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == {$as1, 10.10.10.10} && ip4.dst == $as1' drop + +# Reset as1 to 3 IPs +check ovn-nbctl set address_set as1 addresses=10.0.0.1,10.0.0.2,10.0.0.3 +check ovn-nbctl --wait=hv sync + +# Add 2 IPs +reprocess_count_old=$(read_counter consider_logical_flow) +check ovn-nbctl add address_set as1 addresses 10.0.0.4,10.0.0.5 +check ovn-nbctl --wait=hv sync +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.1 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.2 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.3 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.4 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.5 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.4 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.5 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.10.10.10 actions=conjunction,2/2) +]) +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +# Delete 2 IPs +reprocess_count_old=$(read_counter consider_logical_flow) +check ovn-nbctl --wait=hv remove address_set as1 addresses 10.0.0.4,10.0.0.5 +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.*,/conjunction,/' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.1 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.2 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_dst=10.0.0.3 actions=conjunction,1/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=conjunction,2/2) +priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.10.10.10 actions=conjunction,2/2) +]) +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: conjunctions overlaping with other lflows]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +# Initial state: +# 2 ASes, each has 3 IPs, no overlapping. +# 2 ACLs, each should generate a conjunction, and 1 overlapping tcp.dst +# generating a flow with combined conjunctions. +ovn-nbctl create address_set name=as1 addresses=10.0.0.11,10.0.0.12,10.0.0.13 +ovn-nbctl create address_set name=as2 addresses=10.0.0.21,10.0.0.22,10.0.0.23 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as1 && tcp && tcp.dst == {101, 102}' drop +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src == $as2 && tcp && tcp.dst == {201, 202}' drop + +check ovn-nbctl --wait=hv sync +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.[[0-9]]*,/conjunction,/g' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.11 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.12 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.13 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.21 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.22 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.23 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=101 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=102 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=201 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202 actions=conjunction,2/2) +]) + +# Add 2 IPs to each AS, one of the IPs overlapping, should generate combined +# conjunctions +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl add address_set as1 addresses 10.0.0.14,10.0.0.33 -- \ + add address_set as2 addresses 10.0.0.24,10.0.0.33 +check ovn-nbctl --wait=hv sync +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.[[0-9]]*,/conjunction,/g' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.11 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.12 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.13 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.14 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.21 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.22 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.23 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.24 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.33 actions=conjunction,1/2),conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=101 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=102 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=201 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202 actions=conjunction,2/2) +]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 +]) + +# Remove those 2 IPs from each AS, should return to the initial state +reprocess_count_old=$(read_counter consider_logical_flow) + +check ovn-nbctl remove address_set as1 addresses 10.0.0.14,10.0.0.33 -- \ + remove address_set as2 addresses 10.0.0.24,10.0.0.33 +check ovn-nbctl --wait=hv sync +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | \ + sed -r 's/conjunction.[[0-9]]*,/conjunction,/g' | \ + sed -r 's/conj_id=.*,/conj_id=,/' | sort], [0], [dnl +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,conj_id=,metadata=0x$dp_key actions=drop +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.11 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.12 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.13 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.21 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.22 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.23 actions=conjunction,1/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=101 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=102 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=201 actions=conjunction,2/2) +priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202 actions=conjunction,2/2) +]) + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: mac]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && eth.src == $as1' drop + +# Add MACs to as1 for 5 times. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 5); do + check ovn-nbctl add address_set as1 addresses "aa\:aa\:aa\:aa\:aa\:0$i" + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:01 actions=drop +priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:02 actions=drop +priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:03 actions=drop +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$i +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +]) + +# Remove the MACs from as1. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 5); do + check ovn-nbctl remove address_set as1 addresses "aa\:aa\:aa\:aa\:aa\:0$i" + check ovn-nbctl --wait=hv sync + ovs-ofctl dump-flows br-int table=44 | grep "priority=1100" + if test "$i" = 4; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}'], [0], [dnl +priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:05 actions=drop +]) + fi + if test "$i" = 5; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((5 - $i)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP + +AT_SETUP([ovn-controller - I-P for address set update: ipv6]) +AT_KEYWORDS([as-i-p]) + +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +check ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 + +check ovn-nbctl ls-add ls1 + +check ovn-nbctl lsp-add ls1 ls1-lp1 \ +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" + +wait_for_ports_up +ovn-appctl -t ovn-controller vlog/set file:dbg + +dp_key=$(printf "%x" $(fetch_column datapath tunnel_key external_ids:name=ls1)) +port_key=$(printf "%x" $(fetch_column port_binding tunnel_key logical_port=ls1-lp1)) + +read_counter() { + ovn-appctl -t ovn-controller coverage/read-counter $1 +} + +ovn-nbctl create address_set name=as1 +check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip6.src == $as1' drop + +# Add IPs to as1 for 5 times, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 5); do + check ovn-nbctl add address_set as1 addresses "ff\:\:0$i" + check ovn-nbctl --wait=hv sync + if test "$i" = 3; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl +priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::1 actions=drop +priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::2 actions=drop +priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::3 actions=drop +]) + fi + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$i +]) +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +]) + +# Remove the IPs from as1, 1 IP each time. +reprocess_count_old=$(read_counter consider_logical_flow) + +for i in $(seq 5); do + check ovn-nbctl remove address_set as1 addresses "ff\:\:0$i" + check ovn-nbctl --wait=hv sync + if test "$i" = 4; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44,reg15=0x$port_key | \ + grep -v reply | awk '{print $7, $8}'], [0], [dnl +priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::5 actions=drop +]) + fi + if test "$i" = 5; then + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep "priority=1100"], [1], [ignore]) + else + AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=44 | grep -c "priority=1100"], [0], [$((5 - $i)) +]) + fi +done + +reprocess_count_new=$(read_counter consider_logical_flow) +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP From patchwork Wed Feb 9 06:37:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590204 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=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4JtqvK1t76z9sCD for ; Wed, 9 Feb 2022 17:38:05 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id EB14C60FBC; Wed, 9 Feb 2022 06:38:01 +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 Xh-Fpqp3_RkN; Wed, 9 Feb 2022 06:38:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 5A9EB60F84; Wed, 9 Feb 2022 06:37:58 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id EC14DC0039; Wed, 9 Feb 2022 06:37:57 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1EC58C0070 for ; Wed, 9 Feb 2022 06:37:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C205983298 for ; Wed, 9 Feb 2022 06:37:52 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bzVRXXutiDr9 for ; Wed, 9 Feb 2022 06:37:51 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp1.osuosl.org (Postfix) with ESMTPS id 62DF783260 for ; Wed, 9 Feb 2022 06:37:51 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 111E3FF806; Wed, 9 Feb 2022 06:37:48 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:24 -0800 Message-Id: <20220209063726.1134827-10-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 09/11] lflow: Track reference count of address sets when parsing lflows. 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" Not only track the address set references but also track how many times each address set is referenced when parsing an lflow. This count will be needed by address set incremental processing. Signed-off-by: Han Zhou --- controller/lflow.c | 32 ++++++++++++++++++-------------- controller/lflow.h | 3 +++ include/ovn/expr.h | 4 ++-- lib/expr.c | 16 ++++++++++++---- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index d0b335893..66c31db9e 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -90,7 +90,8 @@ static void lflows_processed_add(struct hmap *lflows_processed, static void lflows_processed_remove(struct hmap *lflows_processed, struct lflow_processed_node *node); static void lflow_resource_add(struct lflow_resource_ref *, enum ref_type, - const char *ref_name, const struct uuid *); + const char *ref_name, const struct uuid *, + size_t ref_count); static struct ref_lflow_node *ref_lflow_lookup(struct hmap *ref_lflow_table, enum ref_type, const char *ref_name); @@ -115,7 +116,7 @@ lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp) * in the future when the lport's port binding changes, the logical flow * that references this lport can be reprocessed. */ lflow_resource_add(aux->lfrr, REF_TYPE_PORTBINDING, port_name, - &aux->lflow->header_.uuid); + &aux->lflow->header_.uuid, 0); const struct sbrec_port_binding *pb = lport_lookup_by_name(aux->sbrec_port_binding_by_name, port_name); @@ -131,7 +132,7 @@ lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp) struct ds mg_key = DS_EMPTY_INITIALIZER; get_mc_group_key(port_name, aux->dp->tunnel_key, &mg_key); lflow_resource_add(aux->lfrr, REF_TYPE_MC_GROUP, ds_cstr(&mg_key), - &aux->lflow->header_.uuid); + &aux->lflow->header_.uuid, 0); ds_destroy(&mg_key); const struct sbrec_multicast_group *mg = mcgroup_lookup_by_dp_name( @@ -173,7 +174,7 @@ is_chassis_resident_cb(const void *c_aux_, const char *port_name) * that in the future when the lport's port-binding changes the logical * flow that references this lport can be reprocessed. */ lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, port_name, - &c_aux->lflow->header_.uuid); + &c_aux->lflow->header_.uuid, 0); const struct sbrec_port_binding *pb = lport_lookup_by_name(c_aux->sbrec_port_binding_by_name, port_name); @@ -266,7 +267,8 @@ lflow_ref_lookup(struct hmap *lflow_ref_table, static void lflow_resource_add(struct lflow_resource_ref *lfrr, enum ref_type type, - const char *ref_name, const struct uuid *lflow_uuid) + const char *ref_name, const struct uuid *lflow_uuid, + size_t ref_count) { struct ref_lflow_node *rlfn = ref_lflow_lookup(&lfrr->ref_lflow_table, type, ref_name); @@ -302,6 +304,7 @@ lflow_resource_add(struct lflow_resource_ref *lfrr, enum ref_type type, struct lflow_ref_list_node *lrln = xzalloc(sizeof *lrln); lrln->lflow_uuid = *lflow_uuid; + lrln->ref_count = ref_count; lrln->rlfn = rlfn; hmap_insert(&rlfn->lflow_uuids, &lrln->hmap_node, uuid_hash(lflow_uuid)); ovs_list_push_back(&lfrn->lflow_ref_head, &lrln->list_node); @@ -750,7 +753,7 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, struct lflow_resource_ref *lfrr, bool *pg_addr_set_ref) { - struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref); + struct shash addr_sets_ref = SHASH_INITIALIZER(&addr_sets_ref); struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref); char *error = NULL; @@ -758,22 +761,23 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, port_groups, &addr_sets_ref, &port_groups_ref, dp->tunnel_key, &error); - const char *addr_set_name; - SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { - lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name, - &lflow->header_.uuid); + struct shash_node *addr_sets_ref_node; + SHASH_FOR_EACH (addr_sets_ref_node, &addr_sets_ref) { + lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_sets_ref_node->name, + &lflow->header_.uuid, + *(size_t *)addr_sets_ref_node->data); } const char *port_group_name; SSET_FOR_EACH (port_group_name, &port_groups_ref) { lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name, - &lflow->header_.uuid); + &lflow->header_.uuid, 0); } if (pg_addr_set_ref) { *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) || - !sset_is_empty(&addr_sets_ref)); + !shash_is_empty(&addr_sets_ref)); } - sset_destroy(&addr_sets_ref); + shash_destroy(&addr_sets_ref); sset_destroy(&port_groups_ref); if (!error) { @@ -812,7 +816,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, const char *io_port = smap_get(&lflow->tags, "in_out_port"); if (io_port) { lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, io_port, - &lflow->header_.uuid); + &lflow->header_.uuid, 0); const struct sbrec_port_binding *pb = lport_lookup_by_name(l_ctx_in->sbrec_port_binding_by_name, io_port); diff --git a/controller/lflow.h b/controller/lflow.h index d6dad17ec..b131b6cb6 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -105,6 +105,9 @@ struct lflow_ref_list_node { struct ovs_list list_node; /* node in lflow_ref_node.lflow_ref_head. */ struct hmap_node hmap_node; /* node in ref_lflow_node.lflow_uuids. */ struct uuid lflow_uuid; + size_t ref_count; /* Reference count of the resource by this lflow. + Currently used for the resource type REF_TYPE_ADDRSET + only, and for other types it is always 0. */ struct ref_lflow_node *rlfn; }; diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 5b232c246..cb3baf001 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -419,13 +419,13 @@ size_t expr_size(const struct expr *); struct expr *expr_parse(struct lexer *, const struct shash *symtab, const struct shash *addr_sets, const struct shash *port_groups, - struct sset *addr_sets_ref, + struct shash *addr_sets_ref, struct sset *port_groups_ref, int64_t dp_id); struct expr *expr_parse_string(const char *, const struct shash *symtab, const struct shash *addr_sets, const struct shash *port_groups, - struct sset *addr_sets_ref, + struct shash *addr_sets_ref, struct sset *port_groups_ref, int64_t dp_id, char **errorp); diff --git a/lib/expr.c b/lib/expr.c index 2fdb7e3bb..2c178d87f 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -519,7 +519,7 @@ struct expr_context { const struct shash *symtab; /* Symbol table. */ const struct shash *addr_sets; /* Address set table. */ const struct shash *port_groups; /* Port group table. */ - struct sset *addr_sets_ref; /* The set of address set referenced. */ + struct shash *addr_sets_ref; /* The set of address set referenced. */ struct sset *port_groups_ref; /* The set of port groups referenced. */ int64_t dp_id; /* The tunnel_key of the datapath for which we're parsing the current @@ -798,7 +798,15 @@ parse_addr_sets(struct expr_context *ctx, struct expr_constant_set *cs, size_t *allocated_values) { if (ctx->addr_sets_ref) { - sset_add(ctx->addr_sets_ref, ctx->lexer->token.s); + size_t *ref_count = shash_find_data(ctx->addr_sets_ref, + ctx->lexer->token.s); + if (!ref_count) { + ref_count = xmalloc(sizeof *ref_count); + *ref_count = 1; + shash_add(ctx->addr_sets_ref, ctx->lexer->token.s, ref_count); + } else { + (*ref_count)++; + } } struct expr_constant_set *addr_sets @@ -1513,7 +1521,7 @@ struct expr * expr_parse(struct lexer *lexer, const struct shash *symtab, const struct shash *addr_sets, const struct shash *port_groups, - struct sset *addr_sets_ref, + struct shash *addr_sets_ref, struct sset *port_groups_ref, int64_t dp_id) { @@ -1537,7 +1545,7 @@ struct expr * expr_parse_string(const char *s, const struct shash *symtab, const struct shash *addr_sets, const struct shash *port_groups, - struct sset *addr_sets_ref, + struct shash *addr_sets_ref, struct sset *port_groups_ref, int64_t dp_id, char **errorp) From patchwork Wed Feb 9 06:37:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590205 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=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4JtqvP6pdBz9sCD for ; Wed, 9 Feb 2022 17:38:09 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id D8A57414E6; Wed, 9 Feb 2022 06:38:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BhrC2kbQwmYj; Wed, 9 Feb 2022 06:38:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 7FA4D415A5; Wed, 9 Feb 2022 06:38:04 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3BF70C000B; Wed, 9 Feb 2022 06:38:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 23834C0031 for ; Wed, 9 Feb 2022 06:38:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2E70A4156A for ; Wed, 9 Feb 2022 06:37:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xWoAwcsnuYUM for ; Wed, 9 Feb 2022 06:37:53 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp4.osuosl.org (Postfix) with ESMTPS id CBABD41559 for ; Wed, 9 Feb 2022 06:37:52 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id 6F45BFF808; Wed, 9 Feb 2022 06:37:50 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:25 -0800 Message-Id: <20220209063726.1134827-11-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 10/11] ovn-controller: Handle addresses deletion in address set incrementally. 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" The cost of reprocessing a lflow referencing a big address set can be very high. Now a single address change in an address set would cause the related logical flows being reprocessed. When the change rate of an address set is high, ovn-controller would be busy reprocessing lflows. This patch handles addresses deletion incrementally. It deletes the related flows for the deleted addresses only, without deleting and recreating unrelated flows unnecessarily. Scale test shows obvious performance gains because the time complexity changed from O(n) to O(1). The bigger the size of address set, the more CPU savings. With the AS size of 10k, the test shows ~40x speed up. Test setup: CPU: Intel(R) Core(TM) i9-7920X CPU @ 2.90GHz. 5 ACL all referencing an address set of 10,000 IPs. Measure the time spent by ovn-controller for handling one IP deletion from the address set. Before: ~400ms After: 11-12ms There is memory cost increase, due to the index built to track each individual IP. The total memory cost for the OF flows in ovn-controller increased ~20% in the 10k AS size test. Before: ofctrl_desired_flow_usage-KB:22248 ofctrl_installed_flow_usage-KB:14850 ofctrl_sb_flow_ref_usage-KB:7208 After: ofctrl_desired_flow_usage-KB:22248 ofctrl_installed_flow_usage-KB:14850 ofctrl_sb_flow_ref_usage-KB:15551 Signed-off-by: Han Zhou --- controller/lflow.c | 149 ++++++++++++++++++++++++++++++++++++ controller/lflow.h | 10 +++ controller/ofctrl.c | 71 +++++++++++++++++ controller/ofctrl.h | 5 ++ controller/ovn-controller.c | 25 ++++-- tests/ovn-controller.at | 20 +++-- 6 files changed, 265 insertions(+), 15 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 66c31db9e..055afba64 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -485,6 +485,155 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in, return ret; } +static bool +as_info_from_expr_const(const char *as_name, const union expr_constant *c, + struct addrset_info *as_info) +{ + as_info->name = as_name; + as_info->ip = c->value.ipv6; + if (c->masked) { + as_info->mask = c->mask.ipv6; + } else { + /* Generate mask so that it is the same as what's added for + * expr->cmp.mask. See make_cmp__() in expr.c. */ + union mf_subvalue mask; + memset(&mask, 0, sizeof mask); + if (c->format == LEX_F_IPV4) { + mask.ipv4 = be32_prefix_mask(32); + } else if (c->format == LEX_F_IPV6) { + mask.ipv6 = ipv6_create_mask(128); + } else if (c->format == LEX_F_ETHERNET) { + mask.mac = eth_addr_exact; + } else { + /* Not an address */ + return false; + } + as_info->mask = mask.ipv6; + } + return true; +} + +static bool +as_update_can_be_handled(const char *as_name, struct addr_set_diff *as_diff, + struct lflow_ctx_in *l_ctx_in) +{ + struct expr_constant_set *as = shash_find_data(l_ctx_in->addr_sets, + as_name); + ovs_assert(as); + size_t n_added = as_diff->added ? as_diff->added->n_values : 0; + size_t n_deleted = as_diff->deleted ? as_diff->deleted->n_values : 0; + size_t old_as_size = as->n_values + n_deleted - n_added; + + /* If the change may impact n_conj, i.e. the template of the flows would + * change, we must reprocess the lflow. */ + if (old_as_size <= 1 || as->n_values <= 1) { + return false; + } + + /* If the size of the diff is too big, reprocessing may be more + * efficient than incrementally processing the diffs. */ + if ((n_added + n_deleted) >= as->n_values) { + return false; + } + + return true; +} + +/* Handles address set update incrementally - processes only the diff + * (added/deleted) addresses in the address set. If it cannot handle the update + * incrementally, returns false, so that the caller will trigger reprocessing + * for the lflow. + * + * The reasons that the function returns false are: + * + * - The size of the address set changed to/from 0 or 1, which means the + * 'template' of the lflow translation is changed. In this case reprocessing + * doesn't impact performance because the size of the address set is already + * very small. + * + * - The size of the change is equal or bigger than the new size. In this case + * it doesn't make sense to incrementally processing the changes because + * reprocessing can be faster. + * + * - When the address set information couldn't be properly tracked during lflow + * parsing. The typical cases are: + * + * - The relational operator to the address set is not '=='. In this case + * there is no 1-1 mapping between the addresses and the flows + * generated. + * + * - The sub expression of the address set is combined with other sub- + * expressions/constants, usually because of disjunctions between + * sub-expressions/constants, e.g.: + * + * ip.src == $as1 || ip.dst == $as2 + * ip.src == {$as1, $as2} + * ip.src == {$as1, ip1} + * + * All these could have been split into separate lflows. + * + * - Conjunctions overlapping between lflows, which can be caused by + * overlapping address sets or same address set used by multiple lflows + * that could have been combined. e.g.: + * + * lflow1: ip.src == $as1 && tcp.dst == {p1, p2} + * lflow2: ip.src == $as1 && tcp.dst == {p3, p4} + * + * It could have been combined as: + * + * ip.src == $as1 && tcp.dst == {p1, p2, p3, p4} + */ +bool +lflow_handle_addr_set_update(const char *as_name, + struct addr_set_diff *as_diff, + struct lflow_ctx_in *l_ctx_in OVS_UNUSED, + struct lflow_ctx_out *l_ctx_out, + bool *changed) +{ + if (as_diff->added) { + return false; + } + ovs_assert(as_diff->deleted); + + if (!as_update_can_be_handled(as_name, as_diff, l_ctx_in)) { + return false; + } + + struct ref_lflow_node *rlfn = + ref_lflow_lookup(&l_ctx_out->lfrr->ref_lflow_table, REF_TYPE_ADDRSET, + as_name); + if (!rlfn) { + *changed = false; + return true; + } + + *changed = false; + + struct lflow_ref_list_node *lrln; + HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) { + if (lflows_processed_find(l_ctx_out->lflows_processed, + &lrln->lflow_uuid)) { + VLOG_DBG("lflow "UUID_FMT"has been processed, skip.", + UUID_ARGS(&lrln->lflow_uuid)); + continue; + } + for (size_t i = 0; i < as_diff->deleted->n_values; i++) { + union expr_constant *c = &as_diff->deleted->values[i]; + struct addrset_info as_info; + if (!as_info_from_expr_const(as_name, c, &as_info)) { + continue; + } + if (!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table, + &lrln->lflow_uuid, &as_info, + lrln->ref_count)) { + return false; + } + *changed = true; + } + } + return true; +} + bool lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, struct lflow_ctx_in *l_ctx_in, diff --git a/controller/lflow.h b/controller/lflow.h index b131b6cb6..d61733bc2 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -181,6 +181,16 @@ bool lflow_handle_changed_flows(struct lflow_ctx_in *, struct lflow_ctx_out *); bool lflow_handle_changed_ref(enum ref_type, const char *ref_name, struct lflow_ctx_in *, struct lflow_ctx_out *, bool *changed); + +struct addr_set_diff { + struct expr_constant_set *added; + struct expr_constant_set *deleted; +}; +bool lflow_handle_addr_set_update(const char *as_name, struct addr_set_diff *, + struct lflow_ctx_in *, + struct lflow_ctx_out *, + bool *changed); + void lflow_handle_changed_neighbors( struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_mac_binding_table *, diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 7671a3b7a..8eb2949f7 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1342,6 +1342,77 @@ ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, } } +/* Remove desired flows related to the specified 'addrset_info' for the + * 'lflow_uuid'. Returns true if it can be processed completely, otherwise + * returns false, which would trigger a reprocessing of the lflow of + * 'lflow_uuid'. The expected_count is checked against the actual flows + * deleted, and if it doesn't match, return false, too. */ +bool +ofctrl_remove_flows_for_as_ip(struct ovn_desired_flow_table *flow_table, + const struct uuid *lflow_uuid, + const struct addrset_info *as_info, + size_t expected_count) +{ + struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, + lflow_uuid); + if (!stf) { + /* No such flow, nothing needs to be done. */ + return true; + } + + struct sb_addrset_ref *sar; + bool found = false; + LIST_FOR_EACH (sar, list_node, &stf->addrsets) { + if (!strcmp(sar->name, as_info->name)) { + found = true; + break; + } + } + if (!found) { + /* No address set tracking infomation found, can't perform the + * deletion. */ + return false; + } + + struct ip_to_flow_node *itfn = ip_to_flow_find(&sar->ip_to_flow_map, + &as_info->ip, + &as_info->mask); + if (!itfn) { + /* This ip wasn't tracked, probably because it maps to a flow that has + * compound conjunction actions for the same ip from multiple address + * sets. */ + return false; + } + struct sb_flow_ref *sfr, *next; + size_t count = 0; + LIST_FOR_EACH_SAFE (sfr, next, as_ip_flow_list, &itfn->flows) { + /* If the desired flow is referenced by multiple sb lflows, it + * shouldn't have been indexed by address set. */ + ovs_assert(ovs_list_is_short(&sfr->sb_list)); + + ovs_list_remove(&sfr->sb_list); + ovs_list_remove(&sfr->flow_list); + ovs_list_remove(&sfr->as_ip_flow_list); + + struct desired_flow *f = sfr->flow; + mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr); + free(sfr); + + ovs_assert(ovs_list_is_empty(&f->list_node)); + ovs_assert(ovs_list_is_empty(&f->references)); + ovn_flow_log(&f->flow, "remove_flows_for_as_ip"); + hmap_remove(&flow_table->match_flow_table, + &f->match_hmap_node); + track_or_destroy_for_flow_del(flow_table, f); + count++; + } + + hmap_remove(&sar->ip_to_flow_map, &itfn->hmap_node); + mem_stats.sb_flow_ref_usage -= sizeof *itfn; + free(itfn); + return (count == expected_count); +} + /* Remove ovn_flows for the given "sb_to_flow" node in the uuid_flow_table. * Optionally log the message for each flow that is acturally removed, if * log_msg is not NULL. */ diff --git a/controller/ofctrl.h b/controller/ofctrl.h index 4ec328c24..ad8f4be65 100644 --- a/controller/ofctrl.h +++ b/controller/ofctrl.h @@ -124,6 +124,11 @@ 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); +bool ofctrl_remove_flows_for_as_ip(struct ovn_desired_flow_table *, + const struct uuid *lflow_uuid, + const struct addrset_info *, + size_t expected_count); + 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 *); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 24e09f78f..5119a3277 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1410,11 +1410,6 @@ en_addr_sets_init(struct engine_node *node OVS_UNUSED, return as; } -struct addr_set_diff { - struct expr_constant_set *added; - struct expr_constant_set *deleted; -}; - static void en_addr_sets_clear_tracked_data(void *data) { @@ -1486,6 +1481,14 @@ addr_sets_update(const struct sbrec_address_set_table *address_set_table, expr_constant_set_integers_diff(cs_old, cs_new, &as_diff->added, &as_diff->deleted); + if (!as_diff->added && !as_diff->deleted) { + /* The address set may have been updated, but the change + * doesn't has any impact to the generated constant-set. + * For example, ff::01 is changed to ff::00:01. */ + free(as_diff); + expr_constant_set_destroy(cs_new); + continue; + } shash_add(updated, as->name, as_diff); expr_const_sets_add(addr_sets, as->name, cs_new); } @@ -2543,9 +2546,15 @@ lflow_output_addr_sets_handler(struct engine_node *node, void *data) } struct shash_node *shash_node; SHASH_FOR_EACH (shash_node, &as_data->updated) { - if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, shash_node->name, - &l_ctx_in, &l_ctx_out, &changed)) { - return false; + struct addr_set_diff *as_diff = shash_node->data; + if (!lflow_handle_addr_set_update(shash_node->name, as_diff, &l_ctx_in, + &l_ctx_out, &changed)) { + VLOG_DBG("Can't incrementally handle the change of address set %s." + " Reprocess related lflows.", shash_node->name); + if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, shash_node->name, + &l_ctx_in, &l_ctx_out, &changed)) { + return false; + } } if (changed) { engine_set_node_state(node, EN_UPDATED); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index b16162ef4..b5f19b442 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -928,7 +928,9 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10 actions=d done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +# when the old/new AS size is smaller than 2, fallback to reprocessing, so +# there are still 2 reprocessing when the AS size is below 2. +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Add IPs to as1 for 10 times, 2 IPs each time. @@ -1117,7 +1119,7 @@ priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,tp_dst=3 done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Add IPs to as1 for 10 times, 2 IPs each time. @@ -1312,7 +1314,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10 done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Add 1 IP back to both ASes @@ -1368,7 +1370,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10 done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) OVN_CLEANUP([hv1]) @@ -1462,6 +1464,8 @@ for i in $(seq 10); do done reprocess_count_new=$(read_counter consider_logical_flow) +# In this case the two sub expr for as1 and as2 are merged, so we lose track of +# address set information - can't handle deletion incrementally. AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 ]) @@ -1562,6 +1566,8 @@ for i in $(seq 10); do done reprocess_count_new=$(read_counter consider_logical_flow) +# In this case the as1 and as2 are merged to a single OR expr, so we lose track of +# address set information - can't handle deletion incrementally. AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 ]) @@ -1657,7 +1663,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10,nw_dst=10 done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Add IPs to as1 for 10 times, 2 IPs each time. @@ -1945,7 +1951,7 @@ priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:05 acti done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) OVN_CLEANUP([hv1]) @@ -2025,7 +2031,7 @@ priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::5 actions=d done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) OVN_CLEANUP([hv1]) From patchwork Wed Feb 9 06:37:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1590206 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=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jtqvk6Hpyz9sCD for ; Wed, 9 Feb 2022 17:38:26 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 78A3783E11; Wed, 9 Feb 2022 06:38:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YvvWW2JZaz_i; Wed, 9 Feb 2022 06:38:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id BF760831B0; Wed, 9 Feb 2022 06:38:20 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83FBBC0011; Wed, 9 Feb 2022 06:38:20 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6EF2BC000B for ; Wed, 9 Feb 2022 06:38:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 713C160F6E for ; Wed, 9 Feb 2022 06:38:00 +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 saI9EjPRBnqV for ; Wed, 9 Feb 2022 06:37:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp3.osuosl.org (Postfix) with ESMTPS id 5DC4C60FA8 for ; Wed, 9 Feb 2022 06:37:56 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id D61A1FF809; Wed, 9 Feb 2022 06:37:51 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 8 Feb 2022 22:37:26 -0800 Message-Id: <20220209063726.1134827-12-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220209063726.1134827-1-hzhou@ovn.org> References: <20220209063726.1134827-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 11/11] ovn-controller: Handle addresses addition in address set incrementally. 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" To avoid reprocessing the lflow when a referenced address set has new addresses added, this patch generates a fake address set that only contains the added addresses for flow generation, and then eliminates the flows that are not related to the newly added addresses. Scale test shows obvious performance gains because the time complexity changed from O(n) to O(1). The bigger the size of address set, the more CPU savings. With the AS size of 10k, the test shows ~40x speed up. Test setup: CPU: Intel(R) Core(TM) i9-7920X CPU @ 2.90GHz. 5 ACL all referencing an address set of 10,000 IPs. Measure the time spent by ovn-controller for handling one IP addition from the address set. Before: ~400ms After: 11-12ms Signed-off-by: Han Zhou --- controller/lflow-conj-ids.c | 20 ++ controller/lflow-conj-ids.h | 1 + controller/lflow.c | 360 ++++++++++++++++++++++++++++++++++-- controller/ovn-controller.c | 8 + include/ovn/expr.h | 1 + lib/expr.c | 2 +- tests/ovn-controller.at | 41 ++-- 7 files changed, 397 insertions(+), 36 deletions(-) diff --git a/controller/lflow-conj-ids.c b/controller/lflow-conj-ids.c index bfe63862a..372391e5a 100644 --- a/controller/lflow-conj-ids.c +++ b/controller/lflow-conj-ids.c @@ -157,6 +157,26 @@ lflow_conj_ids_alloc_specified(struct conj_ids *conj_ids, return true; } +/* Find and return the start id that is allocated to the logical flow. Return + * 0 if not found. */ +uint32_t +lflow_conj_ids_find(struct conj_ids *conj_ids, const struct uuid *lflow_uuid) +{ + struct lflow_conj_node *lflow_conj; + bool found = false; + HMAP_FOR_EACH_WITH_HASH (lflow_conj, hmap_node, uuid_hash(lflow_uuid), + &conj_ids->lflow_conj_ids) { + if (uuid_equals(&lflow_conj->lflow_uuid, lflow_uuid)) { + found = true; + break; + } + } + if (!found) { + return 0; + } + return lflow_conj->start_conj_id; +} + /* Frees the conjunction IDs used by lflow_uuid. */ void lflow_conj_ids_free(struct conj_ids *conj_ids, const struct uuid *lflow_uuid) diff --git a/controller/lflow-conj-ids.h b/controller/lflow-conj-ids.h index 6da0a612c..a1f7a95a5 100644 --- a/controller/lflow-conj-ids.h +++ b/controller/lflow-conj-ids.h @@ -35,6 +35,7 @@ bool lflow_conj_ids_alloc_specified(struct conj_ids *, const struct uuid *lflow_uuid, uint32_t start_conj_id, uint32_t n_conjs); void lflow_conj_ids_free(struct conj_ids *, const struct uuid *lflow_uuid); +uint32_t lflow_conj_ids_find(struct conj_ids *, const struct uuid *lflow_uuid); void lflow_conj_ids_init(struct conj_ids *); void lflow_conj_ids_destroy(struct conj_ids *); void lflow_conj_ids_clear(struct conj_ids *); diff --git a/controller/lflow.c b/controller/lflow.c index 055afba64..f860e4a6b 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -74,6 +74,19 @@ struct condition_aux { struct lflow_resource_ref *lfrr; }; +static struct expr * +convert_match_to_expr(const struct sbrec_logical_flow *, + const struct sbrec_datapath_binding *, + struct expr **prereqs, const struct shash *addr_sets, + const struct shash *port_groups, + struct lflow_resource_ref *, bool *pg_addr_set_ref); +static void +add_matches_to_flow_table(const struct sbrec_logical_flow *, + const struct sbrec_datapath_binding *, + struct hmap *matches, uint8_t ptable, + uint8_t output_ptable, struct ofpbuf *ovnacts, + bool ingress, struct lflow_ctx_in *, + struct lflow_ctx_out *); static void consider_logical_flow(const struct sbrec_logical_flow *lflow, struct hmap *dhcp_opts, struct hmap *dhcpv6_opts, @@ -513,6 +526,262 @@ as_info_from_expr_const(const char *as_name, const union expr_constant *c, return true; } +/* Parses the lflow regarding the changed address set 'as_name', and gererates + * ovs flows for the newly added addresses in 'as_diff_added' only. It is + * similar to consider_logical_flow__, with the below differences: + * + * - It has one more arg 'as_ref_count' to deduce how many flows are expected + * to be added. + * - It uses a small fake address set that contains only the added addresses + * to replace the original address set temporarily and restores it after + * parsing. + * - It doesn't check or touch lflow-cache, because lflow-cache is disabled + * when address-sets/port-groups are used. + * - It doesn't check non-local lports because it should have been checked + * when the lflow is initially parsed, and if it is non-local and skipped + * then it wouldn't have the address set parsed and referenced. + * + * Because of these differences, it is just cleaner to keep it as a separate + * function. */ +static bool +consider_lflow_for_added_as_ips__( + const struct sbrec_logical_flow *lflow, + const struct sbrec_datapath_binding *dp, + const char *as_name, + size_t as_ref_count, + const struct expr_constant_set *as_diff_added, + struct hmap *dhcp_opts, + struct hmap *dhcpv6_opts, + struct hmap *nd_ra_opts, + struct controller_event_options *controller_event_opts, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + bool handled = true; + if (!get_local_datapath(l_ctx_in->local_datapaths, dp->tunnel_key)) { + VLOG_DBG("Skip lflow "UUID_FMT" for non-local datapath %"PRId64, + UUID_ARGS(&lflow->header_.uuid), dp->tunnel_key); + return true; + } + + /* Determine translation of logical table IDs to physical table IDs. */ + bool ingress = !strcmp(lflow->pipeline, "ingress"); + + /* Determine translation of logical table IDs to physical table IDs. */ + uint8_t first_ptable = (ingress + ? OFTABLE_LOG_INGRESS_PIPELINE + : OFTABLE_LOG_EGRESS_PIPELINE); + uint8_t ptable = first_ptable + lflow->table_id; + uint8_t output_ptable = (ingress + ? OFTABLE_REMOTE_OUTPUT + : OFTABLE_SAVE_INPORT); + + uint64_t ovnacts_stub[1024 / 8]; + struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub); + struct ovnact_parse_params pp = { + .symtab = &symtab, + .dhcp_opts = dhcp_opts, + .dhcpv6_opts = dhcpv6_opts, + .nd_ra_opts = nd_ra_opts, + .controller_event_opts = controller_event_opts, + + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, + .n_tables = LOG_PIPELINE_LEN, + .cur_ltable = lflow->table_id, + }; + struct expr *prereqs = NULL; + char *error; + + error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts, &prereqs); + if (error) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", + lflow->actions, error); + free(error); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + return true; + } + + struct lookup_port_aux aux = { + .sbrec_multicast_group_by_name_datapath + = l_ctx_in->sbrec_multicast_group_by_name_datapath, + .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name, + .dp = dp, + .lflow = lflow, + .lfrr = l_ctx_out->lfrr, + }; + struct condition_aux cond_aux = { + .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name, + .dp = dp, + .chassis = l_ctx_in->chassis, + .active_tunnels = l_ctx_in->active_tunnels, + .lflow = lflow, + .lfrr = l_ctx_out->lfrr, + }; + + struct hmap matches = HMAP_INITIALIZER(&matches); + const struct expr_constant_set *fake_as = as_diff_added; + struct expr_constant_set *new_fake_as = NULL; + struct in6_addr dummy_ip; + bool has_dummy_ip = false; + ovs_assert(as_diff_added->n_values); + + /* When there is only 1 element, we append a dummy address and create a + * fake address set with 2 elements, so that the lflow parsing would + * generate exactly the same format of flows as it would when parsing with + * the original address set. */ + if (as_diff_added->n_values == 1) { + new_fake_as = xzalloc(sizeof *new_fake_as); + new_fake_as->values = xzalloc(sizeof *new_fake_as->values * 2); + new_fake_as->n_values = 2; + new_fake_as->values[0] = new_fake_as->values[1] = + as_diff_added->values[0]; + /* Make a dummy ip that is different from the real one. */ + new_fake_as->values[1].value.u8_val++; + dummy_ip = new_fake_as->values[1].value.ipv6; + has_dummy_ip = true; + fake_as = new_fake_as; + } + + /* Temporarily replace the address set in addr_sets with the fake_as, so + * that the cost of lflow parsing is related to the delta but not the + * original size of the address set. It is possible that there are other + * address sets used by this logical flow and their size can be big. In + * such case the parsing cost is still high. In practice, big address + * sets are likely to be updated more frequently that small address sets, + * so this approach should still be effetive overall. + * + * XXX: if necessary, we can optimize this by checking all the address set + * references in this lflow, and replace all the "big" address sets with a + * small faked one. */ + struct expr_constant_set *real_as = + shash_replace((struct shash *)l_ctx_in->addr_sets, as_name, fake_as); + /* We are here because of the address set update, so it must be found. */ + ovs_assert(real_as); + + struct expr *expr = convert_match_to_expr(lflow, dp, &prereqs, + l_ctx_in->addr_sets, + l_ctx_in->port_groups, + l_ctx_out->lfrr, NULL); + shash_replace((struct shash *)l_ctx_in->addr_sets, as_name, real_as); + if (new_fake_as) { + expr_constant_set_destroy(new_fake_as); + free(new_fake_as); + } + if (!expr) { + goto done; + } + + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, + &cond_aux); + expr = expr_normalize(expr); + + uint32_t start_conj_id = 0; + uint32_t n_conjs = 0; + n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, &matches); + if (hmap_is_empty(&matches)) { + VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", + UUID_ARGS(&lflow->header_.uuid)); + goto done; + } + + /* Discard the matches unrelated to the added addresses in the AS + * 'as_name'. */ + struct expr_match *m, *m_next; + HMAP_FOR_EACH_SAFE (m, m_next, hmap_node, &matches) { + if (!m->as_name || strcmp(m->as_name, as_name) || + (has_dummy_ip && !memcmp(&m->as_ip, &dummy_ip, sizeof dummy_ip))) { + hmap_remove(&matches, &m->hmap_node); + expr_match_destroy(m); + continue; + } + } + + /* The number of matches generated by the new addresses should match the + * number of items in the as_diff_added and the reference count of the AS + * in this lflow. Otherwise, it means we hit some complex/corner cases that + * the generated matches can't be mapped from the items in the + * as_diff_added. So we need to fall back to reprocessing the lflow. + */ + if (hmap_count(&matches) != as_ref_count * as_diff_added->n_values) { + VLOG_DBG("lflow "UUID_FMT", addrset %s: Generated flows count " + "(%"PRIuSIZE") " "doesn't match added addresses count " + "(%"PRIuSIZE") and ref_count (%"PRIuSIZE"). " + "Need reprocessing.", + UUID_ARGS(&lflow->header_.uuid), as_name, + hmap_count(&matches), as_diff_added->n_values, as_ref_count); + handled = false; + goto done; + } + if (n_conjs) { + start_conj_id = lflow_conj_ids_find(l_ctx_out->conj_ids, + &lflow->header_.uuid); + if (!start_conj_id) { + VLOG_DBG("lflow "UUID_FMT" didn't have conjunctions. " + "Need reprocessing", UUID_ARGS(&lflow->header_.uuid)); + handled = false; + goto done; + } + expr_matches_prepare(&matches, start_conj_id - 1); + } + add_matches_to_flow_table(lflow, dp, &matches, ptable, output_ptable, + &ovnacts, ingress, l_ctx_in, l_ctx_out); +done: + expr_destroy(prereqs); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + expr_destroy(expr); + expr_matches_destroy(&matches); + return handled; +} + +static bool +consider_lflow_for_added_as_ips( + const struct sbrec_logical_flow *lflow, + const char *as_name, + size_t as_ref_count, + const struct expr_constant_set *as_diff_added, + struct hmap *dhcp_opts, + struct hmap *dhcpv6_opts, + struct hmap *nd_ra_opts, + struct controller_event_options *controller_event_opts, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + const struct sbrec_logical_dp_group *dp_group = lflow->logical_dp_group; + const struct sbrec_datapath_binding *dp = lflow->logical_datapath; + + if (!dp_group && !dp) { + VLOG_DBG("lflow "UUID_FMT" has no datapath binding, skip", + UUID_ARGS(&lflow->header_.uuid)); + return true; + } + ovs_assert(!dp_group || !dp); + + if (dp) { + return consider_lflow_for_added_as_ips__(lflow, dp, as_name, + as_ref_count, as_diff_added, + dhcp_opts, dhcpv6_opts, + nd_ra_opts, + controller_event_opts, + l_ctx_in, l_ctx_out); + } + for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) { + if (!consider_lflow_for_added_as_ips__(lflow, dp_group->datapaths[i], + as_name, as_ref_count, + as_diff_added, dhcp_opts, + dhcpv6_opts, nd_ra_opts, + controller_event_opts, l_ctx_in, + l_ctx_out)) { + return false; + } + } + return true; +} + +/* Check if an address set update can be handled without reprocessing the + * lflow. */ static bool as_update_can_be_handled(const char *as_name, struct addr_set_diff *as_diff, struct lflow_ctx_in *l_ctx_in) @@ -582,19 +851,18 @@ as_update_can_be_handled(const char *as_name, struct addr_set_diff *as_diff, * It could have been combined as: * * ip.src == $as1 && tcp.dst == {p1, p2, p3, p4} + * + * Note: addresses additions still can be processed incrementally in + * this case, although deletions cannot. */ bool lflow_handle_addr_set_update(const char *as_name, struct addr_set_diff *as_diff, - struct lflow_ctx_in *l_ctx_in OVS_UNUSED, + struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out, bool *changed) { - if (as_diff->added) { - return false; - } - ovs_assert(as_diff->deleted); - + ovs_assert(as_diff->added || as_diff->deleted); if (!as_update_can_be_handled(as_name, as_diff, l_ctx_in)) { return false; } @@ -609,6 +877,31 @@ lflow_handle_addr_set_update(const char *as_name, *changed = false; + struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts); + struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts); + struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts); + struct controller_event_options controller_event_opts; + + if (as_diff->added) { + const struct sbrec_dhcp_options *dhcp_opt_row; + SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, + l_ctx_in->dhcp_options_table) { + dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code, + dhcp_opt_row->type); + } + + const struct sbrec_dhcpv6_options *dhcpv6_opt_row; + SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH(dhcpv6_opt_row, + l_ctx_in->dhcpv6_options_table) { + dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name, + dhcpv6_opt_row->code, dhcpv6_opt_row->type); + } + + nd_ra_opts_init(&nd_ra_opts); + controller_event_opts_init(&controller_event_opts); + } + + bool ret = true; struct lflow_ref_list_node *lrln; HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) { if (lflows_processed_find(l_ctx_out->lflows_processed, @@ -617,21 +910,56 @@ lflow_handle_addr_set_update(const char *as_name, UUID_ARGS(&lrln->lflow_uuid)); continue; } - for (size_t i = 0; i < as_diff->deleted->n_values; i++) { - union expr_constant *c = &as_diff->deleted->values[i]; + const struct sbrec_logical_flow *lflow = + sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table, + &lrln->lflow_uuid); + if (!lflow) { + /* lflow deletion should be handled in the corresponding input + * handler, so we can skip here. */ + VLOG_DBG("lflow "UUID_FMT" not found while handling updates of " + "address set %s, skip.", + UUID_ARGS(&lrln->lflow_uuid), as_name); + continue; + } + *changed = true; + + if (as_diff->deleted) { struct addrset_info as_info; - if (!as_info_from_expr_const(as_name, c, &as_info)) { - continue; + for (size_t i = 0; i < as_diff->deleted->n_values; i++) { + union expr_constant *c = &as_diff->deleted->values[i]; + if (!as_info_from_expr_const(as_name, c, &as_info)) { + continue; + } + if (!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table, + &lrln->lflow_uuid, &as_info, + lrln->ref_count)) { + ret = false; + goto done; + } } - if (!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table, - &lrln->lflow_uuid, &as_info, - lrln->ref_count)) { - return false; + } + + if (as_diff->added) { + if (!consider_lflow_for_added_as_ips(lflow, as_name, + lrln->ref_count, + as_diff->added, &dhcp_opts, + &dhcpv6_opts, &nd_ra_opts, + &controller_event_opts, + l_ctx_in, l_ctx_out)) { + ret = false; + goto done; } - *changed = true; } } - return true; + +done: + if (as_diff->added) { + dhcp_opts_destroy(&dhcp_opts); + dhcp_opts_destroy(&dhcpv6_opts); + nd_ra_opts_destroy(&nd_ra_opts); + controller_event_opts_destroy(&controller_event_opts); + } + return ret; } bool diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 5119a3277..b8cd11378 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -3263,6 +3263,14 @@ main(int argc, char *argv[]) engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL); engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL); + /* Keep en_addr_sets before en_runtime_data because + * lflow_output_runtime_data_handler may *partially* reprocess a lflow when + * the lflow is attached to a DP group and a new DP in that DP group is + * added locally, i.e. reprocessing the lflow for the new DP only but not + * for the other DPs in the group. If we handle en_addr_sets after this, + * incrementally processing an updated address set for the added IPs may + * end up adding redundant flows/conjunctions for the lflow agaist the new + * DP because it has been processed on the DP already. */ engine_add_input(&en_lflow_output, &en_addr_sets, lflow_output_addr_sets_handler); engine_add_input(&en_lflow_output, &en_port_groups, diff --git a/include/ovn/expr.h b/include/ovn/expr.h index cb3baf001..3b141b034 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -484,6 +484,7 @@ uint32_t expr_to_matches(const struct expr *, unsigned int *portp), const void *aux, struct hmap *matches); +void expr_match_destroy(struct expr_match *); void expr_matches_destroy(struct hmap *matches); size_t expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs); void expr_matches_print(const struct hmap *matches, FILE *); diff --git a/lib/expr.c b/lib/expr.c index 2c178d87f..47ef6108e 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -3054,7 +3054,7 @@ expr_match_new(const struct match *m, uint8_t clause, uint8_t n_clauses, return match; } -static void +void expr_match_destroy(struct expr_match *match) { free(match->as_name); diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index b5f19b442..260986f15 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -904,7 +904,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=dr done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the IPs from as1, 1 IP each time. @@ -955,7 +955,8 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3 actions=dr done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +# When change from 0 to 2, still reprocessing. +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 ]) # Add and remove IPs at the same time. @@ -973,7 +974,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 1 and remove 2 @@ -988,7 +989,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.10], [0], [1 ]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 1 and remove 1 @@ -1002,7 +1003,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 2 and remove 2 @@ -1019,7 +1020,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.8], [1], [ignore AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) OVN_CLEANUP([hv1]) @@ -1093,7 +1094,7 @@ priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333 actions=conjun done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the IPs from as1, 1 IP each time. @@ -1150,7 +1151,7 @@ priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=333 actions=conjun done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 ]) # Add and remove IPs at the same time. @@ -1168,7 +1169,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.22], [0], [1 AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 1 and remove 2 @@ -1183,7 +1184,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.10], [0], [1 ]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 1 and remove 1 @@ -1197,7 +1198,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep -c 10\.0\.0\.21], [0], [1 AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.10], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Add 2 and remove 2 @@ -1214,7 +1215,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.8], [1], [ignore AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10\.0\.0\.9], [1], [ignore]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) OVN_CLEANUP([hv1]) @@ -1288,7 +1289,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=co done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the IPs from as1 and as2, 1 IP each time. @@ -1341,7 +1342,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3,nw_dst=10. done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [9 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 ]) # Add 1 more IP back to as2 @@ -1639,7 +1640,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3 actions=co done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the IPs from as1, 1 IP each time. @@ -1697,7 +1698,7 @@ priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.3 actions=co done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [10 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [1 ]) # Test the case when there are two references to the same AS but one of the @@ -1842,7 +1843,7 @@ priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202 actions=conjun ]) reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [0 ]) # Remove those 2 IPs from each AS, should return to the initial state @@ -1870,6 +1871,8 @@ priority=1100,tcp,reg15=0x$port_key,metadata=0x$dp_key,tp_dst=202 actions=conjun ]) reprocess_count_new=$(read_counter consider_logical_flow) +# Because of the combined conjunction, AS cannot be tracked for the flow for +# 10.0.0.33, so removing would trigger reprocessing. AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) @@ -1926,7 +1929,7 @@ priority=1100,reg15=0x$port_key,metadata=0x$dp_key,dl_src=aa:aa:aa:aa:aa:03 acti done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the MACs from as1. @@ -2007,7 +2010,7 @@ priority=1100,ipv6,reg15=0x$port_key,metadata=0x$dp_key,ipv6_src=ff::3 actions=d done reprocess_count_new=$(read_counter consider_logical_flow) -AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [5 +AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2 ]) # Remove the IPs from as1, 1 IP each time.