From d811e1027f74de0f1eee1af9af8dd3338eadb61d Mon Sep 17 00:00:00 2001
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Fri, 25 Sep 2020 13:21:58 +0200
Subject: [PATCH] ovn-nbctl: add --may-exist/--if-exists options for policy
routing
Introduce the following options to avoid error reporting for policy
routing:
1) --may-exist: the lr-policy-add does not result in an error if a policy
with the same priority and match string is already present
2) --if-exists: the lr-policy-del does not result in an error if a policy
with the specified uuid is not present in the db
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Han Zhou <hzhou@ovn.org>
---
tests/ovn-nbctl.at | 7 ++++++-
utilities/ovn-nbctl.8.xml | 20 +++++++++++++++-----
utilities/ovn-nbctl.c | 16 ++++++++++------
3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index baf7a87f5..3dbedc843 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1651,6 +1651,8 @@ 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.
])
+AT_CHECK([ovn-nbctl --may-exist lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop])
+
dnl Add duplicated policy
AT_CHECK([ovn-nbctl lr-policy-add lr0 103 "ip4.src == 1.1.1.0/24" deny], [1], [],
[ovn-nbctl: deny: action must be one of "allow", "drop", and "reroute"
@@ -1675,10 +1677,13 @@ Routing Policies
dnl Delete policy by specified uuid
-AT_CHECK([ovn-nbctl lr-policy-del lr0 $(ovn-nbctl --bare --column _uuid list logical_router_policy)])
+uuid=$(ovn-nbctl --bare --column _uuid list logical_router_policy)
+AT_CHECK([ovn-nbctl lr-policy-del lr0 $uuid])
AT_CHECK([ovn-nbctl list logical-router-policy], [0], [dnl
])
+AT_CHECK([ovn-nbctl --if-exists lr-policy-del lr0 $uuid])
+
dnl Add policy with reroute action
AT_CHECK([ovn-nbctl lr-policy-add lr0 102 "ip4.src == 3.1.2.0/24" reroute 3.3.3.3])
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index fcc4312dd..59302296b 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -737,8 +737,9 @@
<h1>Logical Router Policy Commands</h1>
<dl>
- <dt><code>lr-policy-add</code> <var>router</var> <var>priority</var>
- <var>match</var> <var>action</var> [<var>nexthop</var>]
+ <dt>[<code>--may-exist</code>]<code>lr-policy-add</code>
+ <var>router</var> <var>priority</var> <var>match</var>
+ <var>action</var> [<var>nexthop</var>]
[<var>options key=value]</var>] </dt>
<dd>
<p>
@@ -754,6 +755,13 @@
The supported option is : <code>pkt_mark</code>.
</p>
+ <p>
+ If <code>--may-exist</code> is specified, adding a duplicated
+ routing policy with the same priority and match string is not
+ really created. Without <code>--may-exist</code>, adding a
+ duplicated routing policy results in error.
+ </p>
+
<p>
The following example shows a policy to lr1, which will drop packets
from<code>192.168.100.0/24</code>.
@@ -771,8 +779,8 @@
</p>
</dd>
- <dt><code>lr-policy-del</code> <var>router</var> [<var>{priority | uuid}
- [match]</var>]</dt>
+ <dt>[<code>--if-exists</code>] <code>lr-policy-del</code>
+ <var>router</var> [<var>{priority | uuid} [match]</var>]</dt>
<dd>
<p>
Deletes polices from <var>router</var>. If only <var>router</var>
@@ -784,7 +792,9 @@
<p>
If <var>router</var> and <var>uuid</var> are supplied, then the
- policy with sepcified uuid is deleted.
+ policy with sepcified uuid is deleted. It is an error if
+ <var>uuid</var> does not exist, unless <code>--if-exists</code>
+ is specified.
</p>
</dd>
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index c54e63937..caf99dfeb 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -3648,12 +3648,15 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
/* Check if same routing policy already exists.
* A policy is uniquely identified by priority and match */
+ bool may_exist = !!shash_find(&ctx->options, "--may-exist");
for (int i = 0; i < lr->n_policies; i++) {
const struct nbrec_logical_router_policy *policy = lr->policies[i];
if (policy->priority == priority &&
!strcmp(policy->match, ctx->argv[3])) {
- ctl_error(ctx, "Same routing policy already existed on the "
- "logical router %s.", ctx->argv[1]);
+ if (!may_exist) {
+ ctl_error(ctx, "Same routing policy already existed on the "
+ "logical router %s.", ctx->argv[1]);
+ }
return;
}
}
@@ -3733,7 +3736,6 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
ctx->error = error;
return;
}
-
}
/* If uuid was specified, delete routing policy with the
* specified uuid. */
@@ -3751,7 +3753,9 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
}
}
if (n_policies == lr->n_policies) {
- ctl_error(ctx, "Logical router policy uuid is not found.");
+ if (!shash_find(&ctx->options, "--if-exists")) {
+ ctl_error(ctx, "Logical router policy uuid is not found.");
+ }
return;
}
@@ -6529,9 +6533,9 @@ static const struct ctl_command_syntax nbctl_commands[] = {
/* Policy commands */
{ "lr-policy-add", 4, INT_MAX,
"ROUTER PRIORITY MATCH ACTION [NEXTHOP] [OPTIONS - KEY=VALUE ...]",
- NULL, nbctl_lr_policy_add, NULL, "", RW },
+ NULL, nbctl_lr_policy_add, NULL, "--may-exist", RW },
{ "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]", NULL,
- nbctl_lr_policy_del, NULL, "", RW },
+ nbctl_lr_policy_del, NULL, "--if-exists", RW },
{ "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL,
"", RO },
--
2.26.2