From patchwork Wed Oct 23 11:45:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ales Musil X-Patchwork-Id: 2001018 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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=GNEck+0C; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XYS170nCTz1xw0 for ; Wed, 23 Oct 2024 22:46:01 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1C97E40539; Wed, 23 Oct 2024 11:45:59 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id TM8PMv7HVggr; Wed, 23 Oct 2024 11:45:57 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org F1CE44027F Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=GNEck+0C Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id F1CE44027F; Wed, 23 Oct 2024 11:45:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BFBCEC08A6; Wed, 23 Oct 2024 11:45:56 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 83339C08A3 for ; Wed, 23 Oct 2024 11:45:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 76ACB40511 for ; Wed, 23 Oct 2024 11:45:55 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id zja_APtZ_JZk for ; Wed, 23 Oct 2024 11:45:54 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=amusil@redhat.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org AF0EC4027F Authentication-Results: smtp4.osuosl.org; dmarc=pass (p=none dis=none) header.from=redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org AF0EC4027F Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id AF0EC4027F for ; Wed, 23 Oct 2024 11:45:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729683951; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PfPXoBy7bIRfZurbpKKKjG4UbSx7qw4rCCqm+Kf39Qw=; b=GNEck+0CD+UH3BA2cOfBWXUWrdOVJvU+qBWkJbEuoBzgoVX5rehqpjPowtkPOir8SiFQr1 6wxo2Xr9uixrahRgPiDjBoAL+SnBxtVZkvwbe5URp/WzhCvc1KXo0rriOLAfHV1IjTDuTc bRxt7s8iwyF2MJ8oWGV0XD4XjhhQCMU= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-7-tLsNRoyCOEqANdZeWODk7g-1; Wed, 23 Oct 2024 07:45:49 -0400 X-MC-Unique: tLsNRoyCOEqANdZeWODk7g-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 29A621955D71 for ; Wed, 23 Oct 2024 11:45:48 +0000 (UTC) Received: from amusil.redhat.com (unknown [10.45.225.11]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 5F8781956088; Wed, 23 Oct 2024 11:45:44 +0000 (UTC) From: Ales Musil To: dev@openvswitch.org Date: Wed, 23 Oct 2024 13:45:42 +0200 Message-ID: <20241023114542.529787-1-amusil@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] lex: Indicate that the template wasn't found during parsing. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dceara@redhat.com Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Indicate that the template variable wasn't found and hte expansion wasn't succesful. This way we can ignore lflows and LBs that have template variables, but the expansion for that variable wasn't provided. This prevents noisy errors especially for LBs: socket_util|ERR|NODEIP_IPv4_1:32358: bad IP address "NODEIP_IPv4_1" Reported-at: https://issues.redhat.com/browse/FDP-607 Signed-off-by: Ales Musil --- controller/lb.c | 26 +++++++++++++------------- controller/lflow.c | 24 ++++++++++++++++++------ controller/ofctrl.c | 8 +++++--- include/ovn/lex.h | 6 +++--- lib/lex.c | 37 +++++++++++++++++++++++++++++-------- tests/ovn.at | 5 ++--- tests/test-ovn.c | 10 ++++++++-- utilities/ovn-trace.c | 31 ++++++++++++++++++++++--------- 8 files changed, 100 insertions(+), 47 deletions(-) diff --git a/controller/lb.c b/controller/lb.c index 8f9f20ed5..f3e77872a 100644 --- a/controller/lb.c +++ b/controller/lb.c @@ -51,7 +51,6 @@ ovn_controller_lb_create(const struct sbrec_load_balancer *sbrec_lb, struct sset *template_vars_ref) { struct ovn_controller_lb *lb = xzalloc(sizeof *lb); - bool template = smap_get_bool(&sbrec_lb->options, "template", false); lb->slb = sbrec_lb; lb->n_vips = smap_count(&sbrec_lb->vips); @@ -63,18 +62,19 @@ ovn_controller_lb_create(const struct sbrec_load_balancer *sbrec_lb, SMAP_FOR_EACH (node, &sbrec_lb->vips) { struct ovn_lb_vip *lb_vip = &lb->vips[n_vips]; - struct lex_str key_s = template - ? lexer_parse_template_string(node->key, - template_vars, - template_vars_ref) - : lex_str_use(node->key); - struct lex_str value_s = template - ? lexer_parse_template_string(node->value, - template_vars, - template_vars_ref) - : lex_str_use(node->value); - char *error = ovn_lb_vip_init_explicit(lb_vip, - lex_str_get(&key_s), + struct lex_str key_s; + if (!lexer_parse_template_string(&key_s, node->key, template_vars, + template_vars_ref)) { + continue; + } + + struct lex_str value_s; + if (!lexer_parse_template_string(&value_s, node->value, template_vars, + template_vars_ref)) { + continue; + } + + char *error = ovn_lb_vip_init_explicit(lb_vip, lex_str_get(&key_s), lex_str_get(&value_s)); if (error) { free(error); diff --git a/controller/lflow.c b/controller/lflow.c index 13c3a0d73..522c04a89 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -336,9 +336,16 @@ lflow_parse_actions(const struct sbrec_logical_flow *lflow, .cur_ltable = lflow->table_id, }; - struct lex_str actions_s = - lexer_parse_template_string(lflow->actions, l_ctx_in->template_vars, - template_vars_ref); + struct lex_str actions_s; + if (!lexer_parse_template_string(&actions_s, lflow->actions, + l_ctx_in->template_vars, + template_vars_ref)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "error parsing actions \"%s\"", + lflow->actions); + return false; + } + char *error = ovnacts_parse_string(lex_str_get(&actions_s), &pp, ovnacts_out, prereqs_out); lex_str_free(&actions_s); @@ -974,9 +981,14 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref); char *error = NULL; - struct lex_str match_s = lexer_parse_template_string(lflow->match, - template_vars, - template_vars_ref); + struct lex_str match_s; + if (!lexer_parse_template_string(&match_s, lflow->match, template_vars, + template_vars_ref)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "error parsing match \"%s\"", + lflow->match); + return NULL; + } struct expr *e = expr_parse_string(lex_str_get(&match_s), &symtab, addr_sets, port_groups, &addr_sets_ref, &port_groups_ref, diff --git a/controller/ofctrl.c b/controller/ofctrl.c index f9387d375..b681bc368 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -3027,11 +3027,13 @@ ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const char *flow_s, if (version < 0) { return xstrdup("OpenFlow channel not ready."); } + struct lex_str flow_exp_s; + if (!lexer_parse_template_string(&flow_exp_s, flow_s, + template_vars, NULL)) { + return xasprintf("Could not parse flow \"%s\"", flow_s); + } struct flow uflow; - struct lex_str flow_exp_s = lexer_parse_template_string(flow_s, - template_vars, - NULL); char *error = expr_parse_microflow(lex_str_get(&flow_exp_s), &symtab, addr_sets, port_groups, ofctrl_lookup_port, br_int, &uflow); diff --git a/include/ovn/lex.h b/include/ovn/lex.h index 64d33361f..513dd5da9 100644 --- a/include/ovn/lex.h +++ b/include/ovn/lex.h @@ -196,7 +196,7 @@ lex_str_free(struct lex_str *ls) } } -struct lex_str lexer_parse_template_string(const char *s, - const struct smap *template_vars, - struct sset *template_vars_ref); +bool lexer_parse_template_string(struct lex_str *ls, const char *s, + const struct smap *template_vars, + struct sset *template_vars_ref); #endif /* ovn/lex.h */ diff --git a/lib/lex.c b/lib/lex.c index 06c40f7ae..0ed6f6311 100644 --- a/lib/lex.c +++ b/lib/lex.c @@ -1052,35 +1052,56 @@ lexer_steal_error(struct lexer *lexer) } /* Takes ownership of 's' and expands all templates that are encountered - * in the contents of 's', if possible. Adds the encountered template names - * to 'template_vars_ref'. + * in the contents of 's'. Adds the encountered template names + * to 'template_vars_ref'. If the expansion is not possible e.g. + * missing template var it, the function will return 'false'. + * The caller has to free the returned 'struct lex_str'. */ -struct lex_str -lexer_parse_template_string(const char *s, const struct smap *template_vars, +bool +lexer_parse_template_string(struct lex_str *ls, const char *s, + const struct smap *template_vars, struct sset *template_vars_ref) { /* No '^' means no templates. */ if (!strchr(s, LEX_TEMPLATE_PREFIX)) { - return lex_str_use(s); + *ls = lex_str_use(s); + return true; } + bool ok = true; struct ds expanded = DS_EMPTY_INITIALIZER; struct lexer lexer; lexer_init(&lexer, s); while (lexer_get(&lexer) != LEX_T_END) { + if (!ok) { + continue; + } + if (lexer.token.type == LEX_T_TEMPLATE) { - ds_put_cstr(&expanded, smap_get_def(template_vars, lexer.token.s, - lexer.token.s)); + const char *template_var = smap_get(template_vars, lexer.token.s); + if (template_var) { + ds_put_cstr(&expanded, template_var); + } + if (template_vars_ref) { sset_add(template_vars_ref, lexer.token.s); } + + ok &= !!template_var; } else { lex_token_format(&lexer.token, &expanded); } } lexer_destroy(&lexer); - return lex_str_steal(ds_steal_cstr(&expanded)); + + if (ok) { + *ls = lex_str_steal(ds_steal_cstr(&expanded)); + } else { + ds_destroy(&expanded); + *ls = lex_str_use(""); + } + return ok; } diff --git a/tests/ovn.at b/tests/ovn.at index c66dee6ac..ae152189f 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -35717,6 +35717,7 @@ AT_CHECK([ovs-ofctl dump-flows br-int | grep '42\.42\.42\.42'], [1], []) # Need to investigate whteher those errors are expected OVN_CLEANUP([hv1 /Syntax error at /d +/error parsing.*\^/d ]) AT_CLEANUP @@ -35889,11 +35890,9 @@ AT_CHECK([ovs-ofctl dump-groups br-int | grep 'nat'], [1], []) as hv1 AT_CHECK([ovs-ofctl dump-flows br-int | grep table=OFTABLE_CHK_LB_HAIRPIN | ofctl_strip_all], [0], []) -# Ignore errors such as "Syntax error at `BACKENDS31' expecting IP address" -# Need to investigate whteher those errors are expected OVN_CLEANUP([hv1 /bad port number/d -/Syntax error at/d +/error parsing.*\^/d ]) AT_CLEANUP diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 2ea68f212..5d2b132ca 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1326,6 +1326,13 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) puts(ds_cstr(&input)); + struct lex_str exp_input; + if (!lexer_parse_template_string(&exp_input, ds_cstr(&input), + &template_vars, NULL)) { + printf(" failed to parse %s\n", ds_cstr(&input)); + continue; + } + ofpbuf_init(&ovnacts, 0); const struct ovnact_parse_params pp = { @@ -1337,8 +1344,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .n_tables = 24, .cur_ltable = 10, }; - struct lex_str exp_input = - lexer_parse_template_string(ds_cstr(&input), &template_vars, NULL); + error = ovnacts_parse_string(lex_str_get(&exp_input), &pp, &ovnacts, &prereqs); if (!error) { diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index f9cc2463b..656196db1 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -972,10 +972,12 @@ parse_lflow_for_datapath(const struct sbrec_logical_flow *sblf, } char *error; + struct lex_str match_s; + if (!lexer_parse_template_string(&match_s, sblf->match, + &template_vars, NULL)) { + VLOG_WARN("%s: parsing expression failed", sblf->match); + } struct expr *match; - struct lex_str match_s = lexer_parse_template_string(sblf->match, - &template_vars, - NULL); match = expr_parse_string(lex_str_get(&match_s), &symtab, &address_sets, &port_groups, NULL, NULL, dp->tunnel_key, &error); @@ -1003,8 +1005,13 @@ parse_lflow_for_datapath(const struct sbrec_logical_flow *sblf, uint64_t stub[1024 / 8]; struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(stub); struct expr *prereqs; - struct lex_str actions_s = - lexer_parse_template_string(sblf->actions, &template_vars, NULL); + struct lex_str actions_s; + if (!lexer_parse_template_string(&actions_s, sblf->actions, + &template_vars, NULL)) { + VLOG_WARN("%s: parsing actions failed", sblf->actions); + expr_destroy(match); + return; + } error = ovnacts_parse_string(lex_str_get(&actions_s), &pp, &ovnacts, &prereqs); lex_str_free(&actions_s); @@ -3577,8 +3584,11 @@ trace_parse(const char *dp_s, const char *flow_s, * * First make sure that the expression parses. */ char *error; - struct lex_str flow_exp_s = - lexer_parse_template_string(flow_s, &template_vars, NULL); + struct lex_str flow_exp_s; + if (!lexer_parse_template_string(&flow_exp_s, flow_s, + &template_vars, NULL)) { + return xasprintf("failed to parse flow expression \"%s\"", flow_s); + } struct expr *e = expr_parse_string(lex_str_get(&flow_exp_s), &symtab, &address_sets, &port_groups, NULL, NULL, 0, &error); @@ -3607,8 +3617,11 @@ trace_parse(const char *dp_s, const char *flow_s, free(port_name); } - struct lex_str flow_exp_s = - lexer_parse_template_string(flow_s, &template_vars, NULL); + struct lex_str flow_exp_s; + if (!lexer_parse_template_string(&flow_exp_s, flow_s, + &template_vars, NULL)) { + return xasprintf("failed to parse flow expression \"%s\"", flow_s); + } char *error = expr_parse_microflow(lex_str_get(&flow_exp_s), &symtab, &address_sets, &port_groups, ovntrace_lookup_port, *dpp, uflow);