Blob Blame History Raw
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