Cleanup ovn-nbctl lr-route-add IP normalization logic.

Use newer IP normalization routines to make things much cleaner. Signed-off-by: Mark Michelson <mmichels@redhat.com> Acked-by: Numan Siddique <numans@ovn.org>
ovn-org · Jun 26, 2020 · b160e68 · b160e68
1 parent eb9c552
commit b160e68
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 31 deletions.
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
@@ -1423,7 +1423,7 @@ AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24a 11.0.0.1], [1], [],
   [ovn-nbctl: bad prefix argument: 10.0.0.111/24a
 ])
 AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1a], [1], [],
-  [ovn-nbctl: bad next hop argument: 11.0.0.1a
+  [ovn-nbctl: bad IPv4 nexthop argument: 11.0.0.1a
 ])
 AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1/24], [1], [],
   [ovn-nbctl: bad IPv4 nexthop argument: 11.0.0.1/24

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
@@ -3850,7 +3850,7 @@ nbctl_lr_route_add(struct ctl_context *ctx)
         ctx->error = error;
         return;
     }
-    char *prefix, *next_hop;
+    char *prefix = NULL, *next_hop = NULL;
 
     const char *policy = shash_find_data(&ctx->options, "--policy");
     bool is_src_route = false;
@@ -3863,35 +3863,24 @@ nbctl_lr_route_add(struct ctl_context *ctx)
         }
     }
 
-    prefix = normalize_prefix_str(ctx->argv[2]);
+    bool v6_prefix = false;
+    prefix = normalize_ipv4_prefix_str(ctx->argv[2]);
+    if (!prefix) {
+        prefix = normalize_ipv6_prefix_str(ctx->argv[2]);
+        v6_prefix = true;
+    }
     if (!prefix) {
         ctl_error(ctx, "bad prefix argument: %s", ctx->argv[2]);
         return;
     }
 
-    next_hop = normalize_prefix_str(ctx->argv[3]);
+    next_hop = v6_prefix
+        ? normalize_ipv6_addr_str(ctx->argv[3])
+        : normalize_ipv4_addr_str(ctx->argv[3]);
     if (!next_hop) {
-        free(prefix);
-        ctl_error(ctx, "bad next hop argument: %s", ctx->argv[3]);
-        return;
-    }
-
-    if (strchr(prefix, '.')) {
-        ovs_be32 hop_ipv4;
-        if (!ip_parse(ctx->argv[3], &hop_ipv4)) {
-            free(prefix);
-            free(next_hop);
-            ctl_error(ctx, "bad IPv4 nexthop argument: %s", ctx->argv[3]);
-            return;
-        }
-    } else {
-        struct in6_addr hop_ipv6;
-        if (!ipv6_parse(ctx->argv[3], &hop_ipv6)) {
-            free(prefix);
-            free(next_hop);
-            ctl_error(ctx, "bad IPv6 nexthop argument: %s", ctx->argv[3]);
-            return;
-        }
+        ctl_error(ctx, "bad %s nexthop argument: %s",
+                  v6_prefix ? "IPv6" : "IPv4", ctx->argv[3]);
+        goto cleanup;
     }
 
     bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
@@ -3927,10 +3916,8 @@ nbctl_lr_route_add(struct ctl_context *ctx)
             if (!may_exist) {
                 ctl_error(ctx, "duplicate prefix: %s (policy: %s)",
                           prefix, is_src_route ? "src-ip" : "dst-ip");
-                free(next_hop);
                 free(rt_prefix);
-                free(prefix);
-                return;
+                goto cleanup;
             }
 
             /* Update the next hop for an existing route. */
@@ -3947,9 +3934,7 @@ nbctl_lr_route_add(struct ctl_context *ctx)
                  nbrec_logical_router_static_route_set_policy(route, policy);
             }
             free(rt_prefix);
-            free(next_hop);
-            free(prefix);
-            return;
+            goto cleanup;
         }
     }
 
@@ -3973,6 +3958,8 @@ nbctl_lr_route_add(struct ctl_context *ctx)
     nbrec_logical_router_set_static_routes(lr, new_routes,
                                            lr->n_static_routes + 1);
     free(new_routes);
+
+cleanup:
     free(next_hop);
     free(prefix);
 }