[ovs-dev,v2,2/3] Routing policies, add routing-policy commands in ovn-nbctl

Message ID 1542004180-202310-2-git-send-email-mary.manohar@nutanix.com
State Deferred
Headers show
Series
  • [ovs-dev,v2,1/3] Routing policies, add routing-policy commands in ovn-nbctl
Related show

Commit Message

Mary Manohar Nov. 12, 2018, 6:27 a.m.
Policy-based routing (PBR) provides a mechanism to configure permit/deny and reroute policies on the router.
Permit/deny policies are similar to OVN ACLs, but exist on the logical-router.
Reroute policies are needed for service-insertion and service-chaining.
Currently, we support only stateless policies.

To achieve this, a new table is introduced in the ingress pipeline of the Logical-router.
The new table is between the ‘IP Routing’ and the ‘ARP/ND resolution’ table.
This way, PBR can override routing decisions and provide a different next-hop.

This Series:
a. Changes in OVN NB Schema to introduce a new table in the Logical router.
b. Add commands to ovn-nbctl to add/delete/list routing policies.
c. Changes in ovn-northd to process routing-policy configurations.

This Patch:
Add commands to ovn-nbctl to add/delete/list routing policies.

Routing-policy commands:

a. Add a new ovn-nbctl command to add a routing policy.
   lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]

   Nexthop is an optional parameter.
   It needs to be provided only when 'action' is 'reroute'
   A policy is uniquely identified by priority and match.
   Multiple policies can have the same priority.

b. Add a new ovn-nbctl command to delete a routing policy.
   lr-policy-del ROUTER [PRIORITY [MATCH]]

   Takes priority and match as optional parameters.
   If priority and match are specified, the policy with the given priority and match is deleted.
   If priority is specified and match is not specified, all rules with that priority are deleted.
   If priority is not specified, all the rules would be deleted.

c. Add a new ovn-nbctl command to list routing-policies in the logical router.
   lr-policy-list ROUTER

d. Sample CLI:
   ovn-nbctl lr-policy-add lr1 10 "ip4.src == 1.1.1.0/24" drop
   ovn-nbctl lr-policy-del lr1 10
   ovn-nbctl lr-policy-list lr1

   Sample output of the list command:
   611 ip4.dst==12.2.1.0/24 && ip4.src==11.2.1.0/24 && inport=="lrp-ac44a00b-26c9-45e2-851c-62463750c3a2"           allow
   610       ip4.dst==12.2.1.0/24 && ip4.src==11.2.1.0/24         reroute                 13.2.1.12
   600           ip4.dst==0.0.0.0/0 && ip4.src==0.0.0.0/0           drop

e. Unit tests to validate these commands

Signed-off-by: Mary Manohar <mary.manohar@nutanix.com>
---
 ovn/utilities/ovn-nbctl.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/ovn-nbctl.at        |  47 +++++++++++
 2 files changed, 245 insertions(+)

Comments

0-day Robot Nov. 12, 2018, 6:59 a.m. | #1
Bleep bloop.  Greetings Mary Manohar, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#71 FILE: ovn/utilities/ovn-nbctl.c:644:
  lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\

WARNING: Line lacks whitespace around operator
#73 FILE: ovn/utilities/ovn-nbctl.c:646:
  lr-policy-del ROUTER [PRIORITY [MATCH]]\n\

WARNING: Line lacks whitespace around operator
#75 FILE: ovn/utilities/ovn-nbctl.c:648:
  lr-policy-list ROUTER   print policies for ROUTER\n\

ERROR: Inappropriate bracing around statement
#146 FILE: ovn/utilities/ovn-nbctl.c:3465:
    if (next_hop != NULL)

ERROR: Inappropriate bracing around statement
#232 FILE: ovn/utilities/ovn-nbctl.c:3551:
    } else

Lines checked: 346, Warnings: 3, Errors: 2


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

Patch

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 9d1b220..5f4cb2e 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -640,6 +640,13 @@  Route commands:\n\
                             remove routes from ROUTER\n\
   lr-route-list ROUTER      print routes for ROUTER\n\
 \n\
+Policy commands:\n\
+  lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\
+                            add a policy to router\n\
+  lr-policy-del ROUTER [PRIORITY [MATCH]]\n\
+                            remove policies from ROUTER\n\
+  lr-policy-list ROUTER   print policies for ROUTER\n\
+\n\
 NAT commands:\n\
   lr-nat-add ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]\n\
                             add a NAT to ROUTER\n\
@@ -3393,6 +3400,189 @@  normalize_prefix_str(const char *orig_prefix)
         return normalize_ipv6_prefix(ipv6, plen);
     }
 }
+
+static void
+nbctl_lr_policy_add(struct ctl_context *ctx)
+{
+    const struct nbrec_logical_router *lr;
+    int64_t priority = 0;
+    char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+    error = parse_priority(ctx->argv[2], &priority);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+    const char *action = ctx->argv[4];
+    char *next_hop = NULL;
+    /* 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);
+    }
+    if (!strcmp(action, "reroute")) {
+        if (ctx->argc < 6) {
+            ctl_error(ctx, "Nexthop is not specified when action is reroute.");
+        }
+    }
+    /* Check if same routing policy already exists.
+     * A policy is uniquely identified by priority and match */
+    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 (ctx->argc == 6) {
+        next_hop = normalize_prefix_str(ctx->argv[5]);
+        if (!next_hop) {
+            ctl_error(ctx, "bad next hop argument: %s", ctx->argv[5]);
+        }
+    }
+    struct nbrec_logical_router_policy *policy;
+    policy = nbrec_logical_router_policy_insert(ctx->txn);
+    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) {
+        nbrec_logical_router_policy_set_nexthop(policy, next_hop);
+    }
+    nbrec_logical_router_verify_policies(lr);
+    struct nbrec_logical_router_policy **new_policies
+        = xmalloc(sizeof *new_policies * (lr->n_policies + 1));
+    memcpy(new_policies, lr->policies,
+               sizeof *new_policies * lr->n_policies);
+    new_policies[lr->n_policies] = policy;
+    nbrec_logical_router_set_policies(lr, new_policies,
+                                           lr->n_policies + 1);
+    free(new_policies);
+    if (next_hop != NULL)
+        free(next_hop);
+}
+
+static void
+nbctl_lr_policy_del(struct ctl_context *ctx)
+{
+    const struct nbrec_logical_router *lr;
+    int64_t priority = 0;
+    char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+     if (ctx->argc == 2) {
+        /* If a priority is not specified, delete all policies. */
+        nbrec_logical_router_set_policies(lr, NULL, 0);
+        return;
+    }
+    error = parse_priority(ctx->argv[2], &priority);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+    /* If match is not specified, delete all routing policies with the
+     * specified priority. */
+    if (ctx->argc == 3) {
+        struct nbrec_logical_router_policy **new_policies
+                = xmemdup(lr->policies,
+                          sizeof *new_policies * lr->n_policies);
+    int n_policies = 0;
+        for (int i = 0; i < lr->n_policies; i++) {
+            if (priority != lr->policies[i]->priority) {
+                new_policies[n_policies++] = lr->policies[i];
+            }
+        }
+        nbrec_logical_router_verify_policies(lr);
+        nbrec_logical_router_set_policies(lr, new_policies, n_policies);
+        free(new_policies);
+        return;
+    }
+    /* Delete policy that has the same priority and match string */
+    for (int i = 0; i < lr->n_policies; i++) {
+        struct nbrec_logical_router_policy *routing_policy = lr->policies[i];
+         if (priority == routing_policy->priority &&
+            !strcmp(ctx->argv[3], routing_policy->match)) {
+            struct nbrec_logical_router_policy **new_policies
+                = xmemdup(lr->policies,
+                          sizeof *new_policies * lr->n_policies);
+             new_policies[i] = lr->policies[lr->n_policies - 1];
+            nbrec_logical_router_verify_policies(lr);
+            nbrec_logical_router_set_policies(lr, new_policies,
+                                      lr->n_policies - 1);
+            free(new_policies);
+            return;
+        }
+    }
+}
+
+ struct routing_policy {
+    int priority;
+    char *match;
+    const struct nbrec_logical_router_policy *policy;
+};
+
+static int
+routing_policy_cmp(const void *policy1_, const void *policy2_)
+{
+    const struct routing_policy *policy1p = policy1_;
+    const struct routing_policy *policy2p = policy2_;
+    if (policy1p->priority != policy2p->priority) {
+        return policy1p->priority > policy2p->priority ? -1 : 1;
+    } else {
+        return strcmp(policy1p->match, policy2p->match);
+    }
+}
+
+static void
+print_routing_policy(const struct nbrec_logical_router_policy *policy,
+                     struct ds *s)
+{
+    if (policy->nexthop != NULL) {
+        char *next_hop = normalize_prefix_str(policy->nexthop);
+        ds_put_format(s, "%10ld %50s %15s %25s", policy->priority,
+                      policy->match, policy->action, next_hop);
+        free(next_hop);
+    } else
+        ds_put_format(s, "%10ld %50s %15s", policy->priority,
+                      policy->match, policy->action);
+     ds_put_char(s, '\n');
+}
+
+static void
+nbctl_lr_policy_list(struct ctl_context *ctx)
+{
+    const struct nbrec_logical_router *lr;
+    struct routing_policy *policies;
+    size_t n_policies = 0;
+    char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
+    if (error) {
+        ctx->error = error;
+        return;
+    }
+    policies = xmalloc(sizeof *policies * lr->n_policies);
+     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;
+        policies[n_policies].match = policy->match;
+        policies[n_policies].policy = policy;
+        n_policies++;
+    }
+    qsort(policies, n_policies, sizeof *policies, routing_policy_cmp);
+    if (n_policies) {
+        ds_put_cstr(&ctx->output, "Routing Policies\n");
+    }
+    for (int i = 0; i < n_policies; i++) {
+        print_routing_policy(policies[i].policy, &ctx->output);
+    }
+    free(policies);
+}
 
 static void
 nbctl_lr_route_add(struct ctl_context *ctx)
@@ -5141,6 +5331,14 @@  static const struct ctl_command_syntax nbctl_commands[] = {
     { "lr-route-list", 1, 1, "ROUTER", NULL, nbctl_lr_route_list, NULL,
       "", RO },
 
+    /* Policy commands */
+    { "lr-policy-add", 4, 5, "ROUTER PRIORITY MATCH ACTION [NEXTHOP]", NULL,
+        nbctl_lr_policy_add, NULL, "", RW },
+    { "lr-policy-del", 1, 3, "ROUTER [PRIORITY [MATCH]]", NULL,
+        nbctl_lr_policy_del, NULL, "", RW },
+    { "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL,
+       "", RO },
+
     /* NAT commands. */
     { "lr-nat-add", 4, 6,
       "ROUTER TYPE EXTERNAL_IP LOGICAL_IP [LOGICAL_PORT EXTERNAL_MAC]", NULL,
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 25414b8..70aaf81 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1331,6 +1331,53 @@  IPv6 Routes
 
 dnl ---------------------------------------------------------------------
 
+OVN_NBCTL_TEST([ovn_nbctl_policies], [policies], [
+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 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])
+
+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.
+])
+
+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"
+])
+
+dnl Delete by priority and match string
+AT_CHECK([ovn-nbctl lr-policy-del lr0 100 "ip4.src == 1.1.1.0/24"])
+AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
+Routing Policies
+       101                              ip4.src == 2.1.1.0/24           allow
+       101                              ip4.src == 2.1.2.0/24            drop
+       100                              ip4.src == 1.1.2.0/24           allow
+])
+
+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
+])
+
+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])
+
+dnl Add policy with invalid reroute ip
+AT_CHECK([ovn-nbctl lr-policy-add lr0 103 "ip4.src == 3.1.2.0/24" reroute 3.3.3.x], [1], [],
+  [ovn-nbctl: bad next hop argument: 3.3.3.x
+])
+
+])
+
+dnl ---------------------------------------------------------------------
+
 OVN_NBCTL_TEST([ovn_nbctl_lsp_types], [lsp types], [
 AT_CHECK([ovn-nbctl ls-add ls0])
 AT_CHECK([ovn-nbctl lsp-add ls0 lp0])