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 @@

     
Logical Router Policy Commands

-      
lr-policy-add router priority

-          match action [nexthop]

+      
[--may-exist]lr-policy-add

+          router priority match

+          action [nexthop]

           [options key=value]] 

@@ -754,6 +755,13 @@

           The supported option is : pkt_mark.

+        

+          If --may-exist is specified, adding a duplicated

+          routing policy with the same priority and match string is not

+          really created. Without --may-exist, adding a

+          duplicated routing policy results in error.

+        

+

           The following example shows a policy to lr1, which will drop packets

           from192.168.100.0/24.

@@ -771,8 +779,8 @@

-      
lr-policy-del router [{priority | uuid}

-          [match]]

+      
[--if-exists] lr-policy-del

+          router [{priority | uuid} [match]]

           Deletes polices from router. If only router

@@ -784,7 +792,9 @@

           If router and uuid are supplied, then the

-          policy with sepcified uuid is deleted.

+          policy with sepcified uuid is deleted. It is an error if

+          uuid does not exist, unless --if-exists

+          is specified.

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