From patchwork Tue Jun 23 14:29:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1315278 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49rpZ646Fqz9sTb for ; Wed, 24 Jun 2020 00:29:16 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 6621D89665; Tue, 23 Jun 2020 14:29:14 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wj2swlx8FzIe; Tue, 23 Jun 2020 14:29:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id EA70E8951E; Tue, 23 Jun 2020 14:29:12 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE79CC088E; Tue, 23 Jun 2020 14:29:12 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0A454C016F for ; Tue, 23 Jun 2020 14:29:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id E47AF871E0 for ; Tue, 23 Jun 2020 14:29:11 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v0ISWRMDdN8D for ; Tue, 23 Jun 2020 14:29:10 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by whitealder.osuosl.org (Postfix) with ESMTPS id 3A7C087C4E for ; Tue, 23 Jun 2020 14:29:09 +0000 (UTC) X-Originating-IP: 27.7.188.184 Received: from nusiddiq.home.org.com (unknown [27.7.188.184]) (Authenticated sender: numans@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id A380F4000F; Tue, 23 Jun 2020 14:29:06 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Tue, 23 Jun 2020 19:59:01 +0530 Message-Id: <20200623142901.1919822-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v2] ovn-nbctl: Enhance lr-policy-add to set the options. 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" From: Numan Siddique The commit [1] added a new column - 'options' to Logical_Router_Policy NB DB table. This patch enhances the lr-policy-add command to set the options as key=value pairs. For nbctl_lr_policy_add(), this patch now returns after ctl_error() as there is no point continuing further and the comments in the ctl_error() implementation says so. [1] - a123ef0fb8fd("Support packet metadata marking for logical router policies.") Signed-off-by: Numan Siddique Acked-by: Mark Michelson --- tests/ovn-nbctl.at | 15 +++++++++--- tests/ovn.at | 8 ++----- utilities/ovn-nbctl.8.xml | 11 ++++++++- utilities/ovn-nbctl.c | 48 ++++++++++++++++++++++++++++++++++----- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 14de1a8bf..3f874721c 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1590,11 +1590,20 @@ AT_CHECK([ovn-nbctl lr-add lr0]) dnl Add policies with allow and drop actions AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop]) -AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.2.0/24" allow]) +AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.2.0/24" allow pkt_mark=100,foo=bar]) AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.1.0/24" allow]) AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.2.0/24" drop]) AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip6.src == 2002::/64" drop]) +dnl Incomplete option set. +AT_CHECK([ovn-nbctl lr-policy-add lr0 200 "ip4.src == 1.1.4.0/24" reroute 192.168.0.10 foo], [1], [], + [ovn-nbctl: No value specified for the option : foo +]) + +AT_CHECK([ovn-nbctl lr-policy-add lr0 200 "ip4.src == 1.1.4.0/24" allow bar=], [1], [], + [ovn-nbctl: No value specified for the option : bar +]) + dnl Add duplicated policy AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop], [1], [], [ovn-nbctl: Same routing policy already existed on the logical router lr0. @@ -1612,14 +1621,14 @@ Routing Policies 101 ip4.src == 2.1.1.0/24 allow 101 ip4.src == 2.1.2.0/24 drop 101 ip6.src == 2002::/64 drop - 100 ip4.src == 1.1.2.0/24 allow + 100 ip4.src == 1.1.2.0/24 allow pkt_mark=100,foo=bar ]) dnl Delete all policies for given priority AT_CHECK([ovn-nbctl lr-policy-del lr0 101]) AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl Routing Policies - 100 ip4.src == 1.1.2.0/24 allow + 100 ip4.src == 1.1.2.0/24 allow pkt_mark=100,foo=bar ]) diff --git a/tests/ovn.at b/tests/ovn.at index 1ff795294..00ed875ff 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -19993,20 +19993,16 @@ static_routes @lrt ovn-nbctl --wait=hv sync # Add logical router policy and set pkt_mark on it. -ovn-nbctl lr-policy-add lr0 2000 "ip4.src == 10.0.0.3" allow +ovn-nbctl lr-policy-add lr0 2000 "ip4.src == 10.0.0.3" allow pkt_mark=100 ovn-nbctl lr-policy-add lr0 1000 "ip4.src == 10.0.0.4" allow -ovn-nbctl lr-policy-add lr0 900 "ip4.src == 10.0.0.5" reroute 172.168.0.200 +ovn-nbctl lr-policy-add lr0 900 "ip4.src == 10.0.0.5" reroute 172.168.0.200 pkt_mark=3 ovn-nbctl lr-policy-add lr0 2001 "ip6.dst == bef0::5" reroute bef0::6 ovn-nbctl lr-policy-add lr0 1001 "ip6" allow - pol1=$(ovn-nbctl --bare --columns _uuid find logical_router_policy priority=2000) -pol3=$(ovn-nbctl --bare --columns _uuid find logical_router_policy priority=900) pol4=$(ovn-nbctl --bare --columns _uuid find logical_router_policy priority=2001) pol5=$(ovn-nbctl --bare --columns _uuid find logical_router_policy priority=1001) -ovn-nbctl set logical_router_policy $pol1 options:pkt_mark=100 -ovn-nbctl set logical_router_policy $pol3 options:pkt_mark=3 ovn-nbctl set logical_router_policy $pol4 options:pkt_mark=4 ovn-nbctl set logical_router_policy $pol5 options:pkt_mark=5 ovn-nbctl --wait=hv sync diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index d265c7fcc..de86b70e6 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -721,7 +721,8 @@
lr-policy-add router priority - match action [nexthop]
+ match action [nexthop] + [options key=value]]

Add Policy to router which provides a way to configure @@ -732,6 +733,8 @@ only when action is reroute. A policy is uniquely identified by priority and match. Multiple policies can have the same priority. + options sets the router policy options as key-value pair. + The supported option is : pkt_mark.

@@ -743,6 +746,12 @@ lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 drop.

+

+ + lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 allow + pkt_mark=100 + . +

lr-policy-del router [{priority | uuid} diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 6ccc7025d..ee13423ce 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -694,7 +694,8 @@ Route commands:\n\ lr-route-list ROUTER print routes for ROUTER\n\ \n\ Policy commands:\n\ - lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\ + lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP] \ +[OPTIONS KEY=VALUE ...] \n\ add a policy to router\n\ lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\ remove policies from ROUTER\n\ @@ -3552,16 +3553,19 @@ nbctl_lr_policy_add(struct ctl_context *ctx) const char *action = ctx->argv[4]; char *next_hop = NULL; + bool reroute = false; /* Validate action. */ if (strcmp(action, "allow") && strcmp(action, "drop") && strcmp(action, "reroute")) { ctl_error(ctx, "%s: action must be one of \"allow\", \"drop\", " "and \"reroute\"", action); + return; } if (!strcmp(action, "reroute")) { if (ctx->argc < 6) { ctl_error(ctx, "Nexthop is required when action is reroute."); } + reroute = true; } /* Check if same routing policy already exists. @@ -3572,12 +3576,14 @@ nbctl_lr_policy_add(struct ctl_context *ctx) !strcmp(policy->match, ctx->argv[3])) { ctl_error(ctx, "Same routing policy already existed on the " "logical router %s.", ctx->argv[1]); + return; } } - if (ctx->argc == 6) { + if (reroute) { next_hop = normalize_prefix_str(ctx->argv[5]); if (!next_hop) { ctl_error(ctx, "bad next hop argument: %s", ctx->argv[5]); + return; } } @@ -3586,9 +3592,28 @@ nbctl_lr_policy_add(struct ctl_context *ctx) nbrec_logical_router_policy_set_priority(policy, priority); nbrec_logical_router_policy_set_match(policy, ctx->argv[3]); nbrec_logical_router_policy_set_action(policy, action); - if (ctx->argc == 6) { + if (reroute) { nbrec_logical_router_policy_set_nexthop(policy, next_hop); } + + /* Parse the options. */ + struct smap options = SMAP_INITIALIZER(&options); + for (size_t i = reroute ? 6 : 5; i < ctx->argc; i++) { + char *key, *value; + value = xstrdup(ctx->argv[i]); + key = strsep(&value, "="); + if (value && value[0]) { + smap_add(&options, key, value); + } else { + ctl_error(ctx, "No value specified for the option : %s", key); + free(key); + return; + } + free(key); + } + nbrec_logical_router_policy_set_options(policy, &options); + smap_destroy(&options); + nbrec_logical_router_verify_policies(lr); struct nbrec_logical_router_policy **new_policies = xmalloc(sizeof *new_policies * (lr->n_policies + 1)); @@ -3716,6 +3741,16 @@ print_routing_policy(const struct nbrec_logical_router_policy *policy, ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, policy->match, policy->action); } + + if (!smap_is_empty(&policy->options)) { + ds_put_format(s, "%15s", ""); + struct smap_node *node; + SMAP_FOR_EACH (node, &policy->options) { + ds_put_format(s, "%s=%s,", node->key, node->value); + } + ds_chomp(s, ','); + } + ds_put_char(s, '\n'); } @@ -3731,7 +3766,7 @@ nbctl_lr_policy_list(struct ctl_context *ctx) return; } policies = xmalloc(sizeof *policies * lr->n_policies); - for (int i = 0; i < lr->n_policies; i++) { + for (int i = 0; i < lr->n_policies; i++) { const struct nbrec_logical_router_policy *policy = lr->policies[i]; policies[n_policies].priority = policy->priority; @@ -6273,8 +6308,9 @@ static const struct ctl_command_syntax nbctl_commands[] = { "", RO }, /* Policy commands */ - { "lr-policy-add", 4, 5, "ROUTER PRIORITY MATCH ACTION [NEXTHOP]", NULL, - nbctl_lr_policy_add, NULL, "", RW }, + { "lr-policy-add", 4, INT_MAX, + "ROUTER PRIORITY MATCH ACTION [NEXTHOP] [OPTIONS - KEY=VALUE ...]", + NULL, nbctl_lr_policy_add, NULL, "", RW }, { "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]", NULL, nbctl_lr_policy_del, NULL, "", RW }, { "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL,