From patchwork Thu Oct 1 11:22:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1375034 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=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ycl5qivI; dkim-atps=neutral Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C29hg0R5mz9sSC for ; Thu, 1 Oct 2020 21:22:40 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 208FA230FE; Thu, 1 Oct 2020 11:22:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id T6mnMoB31lEN; Thu, 1 Oct 2020 11:22:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 906AB204A9; Thu, 1 Oct 2020 11:22:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 73C14C016F; Thu, 1 Oct 2020 11:22:31 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id A7F25C0051 for ; Thu, 1 Oct 2020 11:22:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 8DF68204A9 for ; Thu, 1 Oct 2020 11:22:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id joDJQzQDxVq5 for ; Thu, 1 Oct 2020 11:22:28 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by silver.osuosl.org (Postfix) with ESMTPS id BE6F2204A2 for ; Thu, 1 Oct 2020 11:22:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1601551346; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type; bh=u224TaAyqIllaQcymH//LQ12TQaz2zzdpaCrcKN1nOc=; b=Ycl5qivItqyOwvoHtPYCmKVAFdNhusnL6iZywkNZy9Ltewr3BAvsO3xFt3RDPwtqYBk9sN t8v2Z7kJ2v0hglCJpAqluW9vBmuGOrvid/I90/H8baCYVyQIyRijxtYJ8a4lfrk1Ts7eU7 08KETSjca0wO0UlTPXXmLNCOIBk6ugM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-411-DVohtp6WN8uh-tyRNpudmQ-1; Thu, 01 Oct 2020 07:22:23 -0400 X-MC-Unique: DVohtp6WN8uh-tyRNpudmQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A91F31007466; Thu, 1 Oct 2020 11:22:22 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-78.ams2.redhat.com [10.36.114.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4DAE660BF1; Thu, 1 Oct 2020 11:22:18 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 1 Oct 2020 13:22:13 +0200 Message-Id: <1601551333-25368-1-git-send-email-dceara@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH ovn v2] lflow.c: Refactor function convert_match_to_expr. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" To make it easier to understand what the function does we now explicitly pass the input/output arguments instead of the complete lflow_ctx_in/lflow_ctx_out context. Also: - change the error handling to avoid forcing the caller to supply (and free) an error string pointer. - update function comments in expr.c to mention that port_groups sets are also used for parsing. CC: Han Zhou CC: Numan Siddique Signed-off-by: Dumitru Ceara Acked-by: Mark Michelson Acked-by: Mark Gray --- v2: Fix typos and comments as suggested by Mark Gray. --- controller/lflow.c | 68 +++++++++++++++++++++++++----------------------------- lib/expr.c | 18 +++++++-------- 2 files changed, 40 insertions(+), 46 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 4d71dfd..f631679 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -755,41 +755,48 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, ofpbuf_uninit(&ofpacts); } -/* Converts the match and returns the simplified expre tree. - * The caller should evaluate the conditions and normalize the - * expr tree. */ +/* Converts the match and returns the simplified expr tree. + * + * The caller should evaluate the conditions and normalize the expr tree. + */ static struct expr * convert_match_to_expr(const struct sbrec_logical_flow *lflow, struct expr *prereqs, - struct lflow_ctx_in *l_ctx_in, - struct lflow_ctx_out *l_ctx_out, - bool *pg_addr_set_ref, char **errorp) + const struct shash *addr_sets, + const struct shash *port_groups, + struct lflow_resource_ref *lfrr, + bool *pg_addr_set_ref) { struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref); struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref); - char *error; + char *error = NULL; - struct expr *e = expr_parse_string(lflow->match, &symtab, - l_ctx_in->addr_sets, - l_ctx_in->port_groups, &addr_sets_ref, + struct expr *e = expr_parse_string(lflow->match, &symtab, addr_sets, + port_groups, &addr_sets_ref, &port_groups_ref, lflow->logical_datapath->tunnel_key, &error); const char *addr_set_name; SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { - lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_ADDRSET, addr_set_name, + lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name, &lflow->header_.uuid); } const char *port_group_name; SSET_FOR_EACH (port_group_name, &port_groups_ref) { - lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTGROUP, - port_group_name, &lflow->header_.uuid); + lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name, + &lflow->header_.uuid); } + if (pg_addr_set_ref) { + *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) || + !sset_is_empty(&addr_sets_ref)); + } + sset_destroy(&addr_sets_ref); + sset_destroy(&port_groups_ref); + if (!error) { if (prereqs) { e = expr_combine(EXPR_T_AND, e, prereqs); - prereqs = NULL; } e = expr_annotate(e, &symtab, &error); } @@ -797,24 +804,11 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s", lflow->match, error); - sset_destroy(&addr_sets_ref); - sset_destroy(&port_groups_ref); - *errorp = error; + free(error); return NULL; } - e = expr_simplify(e); - - if (pg_addr_set_ref) { - *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) || - !sset_is_empty(&addr_sets_ref)); - } - - sset_destroy(&addr_sets_ref); - sset_destroy(&port_groups_ref); - - *errorp = NULL; - return e; + return expr_simplify(e); } static bool @@ -896,13 +890,13 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, struct expr *expr = NULL; if (!l_ctx_out->lflow_cache_map) { /* Caching is disabled. */ - expr = convert_match_to_expr(lflow, prereqs, l_ctx_in, - l_ctx_out, NULL, &error); - if (error) { + expr = convert_match_to_expr(lflow, prereqs, l_ctx_in->addr_sets, + l_ctx_in->port_groups, l_ctx_out->lfrr, + NULL); + if (!expr) { expr_destroy(prereqs); ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); - free(error); return true; } @@ -959,13 +953,13 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, bool pg_addr_set_ref = false; if (!expr) { - expr = convert_match_to_expr(lflow, prereqs, l_ctx_in, l_ctx_out, - &pg_addr_set_ref, &error); - if (error) { + expr = convert_match_to_expr(lflow, prereqs, l_ctx_in->addr_sets, + l_ctx_in->port_groups, l_ctx_out->lfrr, + &pg_addr_set_ref); + if (!expr) { expr_destroy(prereqs); ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); - free(error); return true; } } else { diff --git a/lib/expr.c b/lib/expr.c index f6b22cf..acb1f3a 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -1317,9 +1317,9 @@ expr_parse__(struct expr_context *ctx) } /* Parses an expression from 'lexer' using the symbols in 'symtab' and - * address set table in 'addr_sets'. If successful, returns the new - * expression; on failure, returns NULL. Returns nonnull if and only if - * lexer->error is NULL. */ + * address set table in 'addr_sets' and 'port_groups'. If successful, returns + * the new expression; on failure, returns NULL. Returns nonnull if and only + * if lexer->error is NULL. */ struct expr * expr_parse(struct lexer *lexer, const struct shash *symtab, const struct shash *addr_sets, @@ -1339,9 +1339,9 @@ expr_parse(struct lexer *lexer, const struct shash *symtab, } /* Parses the expression in 's' using the symbols in 'symtab' and - * address set table in 'addr_sets'. If successful, returns the new - * expression and sets '*errorp' to NULL. On failure, returns NULL and - * sets '*errorp' to an explanatory error message. The caller must + * address set table in 'addr_sets' and 'port_groups'. If successful, returns + * the new expression and sets '*errorp' to NULL. On failure, returns NULL + * and sets '*errorp' to an explanatory error message. The caller must * eventually free the returned expression (with expr_destroy()) or * error (with free()). */ struct expr * @@ -3481,9 +3481,9 @@ expr_parse_microflow__(struct lexer *lexer, } /* Parses 's' as a microflow, using symbols from 'symtab', address set - * table from 'addr_sets', and looking up port numbers using 'lookup_port' - * and 'aux'. On success, stores the result in 'uflow' and returns - * NULL, otherwise zeros 'uflow' and returns an error message that the + * table from 'addr_sets' and 'port_groups', and looking up port numbers using + * 'lookup_port' and 'aux'. On success, stores the result in 'uflow' and + * returns NULL, otherwise zeros 'uflow' and returns an error message that the * caller must free(). * * A "microflow" is a description of a single stream of packets, such as half a