[ovs-dev] [PATCH ovn] ovn-nbctl: Enhance lr-policy-add to set the options.

numans at ovn.org numans at ovn.org
Tue Jun 23 11:43:09 UTC 2020


From: Numan Siddique <numans at ovn.org>

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.

[1] - a123ef0fb8fd("Support packet metadata marking for logical router policies.")

Signed-off-by: Numan Siddique <numans at ovn.org>
---
 tests/ovn-nbctl.at        |  6 +++---
 tests/ovn.at              |  8 ++------
 utilities/ovn-nbctl.8.xml | 11 ++++++++++-
 utilities/ovn-nbctl.c     | 38 ++++++++++++++++++++++++++++++++------
 4 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 14de1a8bf..8180ccf9c 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1590,7 +1590,7 @@ 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])
@@ -1612,14 +1612,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 @@
 
     <dl>
       <dt><code>lr-policy-add</code> <var>router</var> <var>priority</var>
-          <var>match</var> <var>action</var> [<var>nexthop</var>]</dt>
+          <var>match</var> <var>action</var> [<var>nexthop</var>]
+          [<var>options key=value]</var>] </dt>
       <dd>
         <p>
           Add Policy to <var>router</var> which provides a way to configure
@@ -732,6 +733,8 @@
           only when <var>action</var> is <var>reroute</var>. A policy is
           uniquely identified by <var>priority</var> and <var>match</var>.
           Multiple policies can have the same <var>priority</var>.
+          <var>options</var> sets the router policy options as key-value pair.
+          The supported option is : <code>pkt_mark</code>.
         </p>
 
           <p>
@@ -743,6 +746,12 @@
           <code>lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 drop</code>.
           </p>
 
+          <p>
+          <code>
+            lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 allow
+            pkt_mark=100
+          </code>.
+          </p>
       </dd>
 
       <dt><code>lr-policy-del</code> <var>router</var> [<var>{priority | uuid}
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 6ccc7025d..ec269f252 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -694,7 +694,7 @@ 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,6 +3552,7 @@ 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")) {
@@ -3562,6 +3563,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
         if (ctx->argc < 6) {
             ctl_error(ctx, "Nexthop is required when action is reroute.");
         }
+        reroute = true;
     }
 
     /* Check if same routing policy already exists.
@@ -3574,7 +3576,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
                       "logical router %s.", ctx->argv[1]);
         }
     }
-    if (ctx->argc == 6) {
+    if (ctx->argc >= 6 && reroute) {
         next_hop = normalize_prefix_str(ctx->argv[5]);
         if (!next_hop) {
             ctl_error(ctx, "bad next hop argument: %s", ctx->argv[5]);
@@ -3586,9 +3588,23 @@ 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 (ctx->argc >= 6 && 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) {
+            smap_add(&options, key, value);
+        }
+        free(key);
+    }
+    nbrec_logical_router_policy_set_options(policy, &options);
+
     nbrec_logical_router_verify_policies(lr);
     struct nbrec_logical_router_policy **new_policies
         = xmalloc(sizeof *new_policies * (lr->n_policies + 1));
@@ -3716,6 +3732,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 +3757,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 +6299,8 @@ 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]",
+     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,
-- 
2.26.2



More information about the dev mailing list