[ovs-dev] [PATCH v5 1/1] ovn: l3ha, CLI for logical router port gateway chassis

Miguel Angel Ajo Pelayo majopela at redhat.com
Fri Jul 14 12:54:55 UTC 2017


Hey Anil!, thanks for the cli commands :D that's great

Comments inline

Other than those comments it looks fine to my unexperienced eyes. I'm going
to grab & try your patch :)


On Fri, Jul 14, 2017 at 2:14 PM, Venkata Anil Kommaddi <vkommadi at redhat.com>
wrote:

> This change adds commands to set, get and delete gateway chassis
> for logical router port.
>
> Signed-off-by: Venkata Anil <vkommadi at redhat.com>
> ---
>  ovn/utilities/ovn-nbctl.c | 172 ++++++++++++++++++++++++++++++
> ++++++++++++++++
>  tests/ovn-nbctl.at        |  52 ++++++++++++++
>  2 files changed, 224 insertions(+)
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 0488318..7cf826f 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -376,6 +376,13 @@ Logical router commands:\n\
>  Logical router port commands:\n\
>    lrp-add ROUTER PORT MAC NETWORK... [peer=PEER]\n\
>                              add logical port PORT on ROUTER\n\
> +  lrp-set-gateway-chassis PORT CHASSIS [PRIORITY]\n\
> +                            set gateway chassis for port PORT\n\
> +  lrp-del-gateway-chassis PORT CHASSIS\n\
> +                            delete gateway chassis from port PORT\n\
> +  lrp-get-gateway-chassis PORT\n\
> +                            print the names of all gateway chassis on
> PORT\n\
> +                            with PRIORITY\n\
>    lrp-del PORT              delete PORT from its attached router\n\
>    lrp-list ROUTER           print the names of all ports on ROUTER\n\
>    lrp-set-enabled PORT STATE\n\
> @@ -2528,6 +2535,164 @@ lr_get_name(const struct nbrec_logical_router *lr,
> char uuid_s[UUID_LEN + 1],
>      return uuid_s;
>  }
>
> +static const struct nbrec_gateway_chassis*
> +gc_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> must_exist)
> +{
> +    const struct nbrec_gateway_chassis *gc = NULL;
> +
> +    struct uuid gc_uuid;
> +    bool is_uuid = uuid_from_string(&gc_uuid, id);
> +    if (is_uuid) {
> +        gc = nbrec_gateway_chassis_get_for_uuid(ctx->idl, &gc_uuid);
> +    }
> +
> +    if (!gc) {
> +        NBREC_GATEWAY_CHASSIS_FOR_EACH (gc, ctx->idl) {
> +            if (!strcmp(gc->name, id)) {
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (!gc && must_exist) {
> +        ctl_fatal("%s: gateway chassis %s not found", id,
> +                  is_uuid ? "UUID" : "name");
> +    }
> +
> +    return gc;
> +}
> +
> +static void
> +nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
> +{
> +    const char *gc_name;
> +    int64_t priority = 0;
> +    const char *lrp_name = ctx->argv[1];
> +    const struct nbrec_logical_router_port *lrp;
> +    lrp = lrp_by_name_or_uuid(ctx, lrp_name, true);
> +    if (!lrp) {
> +        ctl_fatal("router port %s is required", lrp_name);
> +        return;
> +    }
> +
> +    const char *chassis_name = ctx->argv[2];
> +    if (ctx->argv[3]) {
> +        priority = parse_priority(ctx->argv[3]);
> +    }
> +
> +    gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
> +    const struct nbrec_gateway_chassis *gc;
> +    gc = gc_by_name_or_uuid(ctx, gc_name, false);
> +    if (gc) {
> +        nbrec_gateway_chassis_set_priority(gc, priority);
> +        return;
> +    }
> +
> +    /* Create the logical gateway chassis. */
> +    gc = nbrec_gateway_chassis_insert(ctx->txn);
> +    nbrec_gateway_chassis_set_name(gc, gc_name);
> +    nbrec_gateway_chassis_set_chassis_name(gc, chassis_name);
> +    nbrec_gateway_chassis_set_priority(gc, priority);
> +
> +    /* Insert the logical gateway chassis into the logical router port. */
> +    nbrec_logical_router_port_verify_gateway_chassis(lrp);
> +    struct nbrec_gateway_chassis **new_gc = xmalloc(
> +        sizeof *new_gc * (lrp->n_gateway_chassis + 1));
> +    nullable_memcpy(new_gc, lrp->gateway_chassis,
> +                    sizeof *new_gc * lrp->n_gateway_chassis);
> +    new_gc[lrp->n_gateway_chassis] = CONST_CAST(
> +        struct nbrec_gateway_chassis *, gc);
> +    nbrec_logical_router_port_set_gateway_chassis(
> +        lrp, new_gc, lrp->n_gateway_chassis + 1);
> +    free(new_gc);
> +}
> +
> +/* Removes logical router port 'lrp->gateway_chassis[idx]'. */
> +static void
> +remove_gc(const struct nbrec_logical_router_port *lrp, size_t idx)
> +{
> +    const struct nbrec_gateway_chassis *gc = lrp->gateway_chassis[idx];
> +    /* First remove 'gc' from the array of gateway_chassis.  This is what
> will
> +     * actually cause the gateway chassis to be deleted when the
> transaction is
> +     * sent to the database server (due to garbage collection). */
> +    struct nbrec_gateway_chassis **new_gc
> +        = xmemdup(lrp->gateway_chassis,
> +                  sizeof *new_gc * lrp->n_gateway_chassis);
> +    new_gc[idx] = new_gc[lrp->n_gateway_chassis - 1];
> +    nbrec_logical_router_port_verify_gateway_chassis(lrp);
> +    nbrec_logical_router_port_set_gateway_chassis(
> +        lrp, new_gc, lrp->n_gateway_chassis - 1);
> +    free(new_gc);
> +
> +    /* Delete 'gc' from the IDL.  This won't have a real effect on
> +     * the database server (the IDL will suppress it in fact) but it
> +     * means that it won't show up when we iterate with
> +     * NBREC_GATEWAY_CHASSIS_FOR_EACH later. */
> +    nbrec_gateway_chassis_delete(gc);
> +}
> +
> +static void
> +nbctl_lrp_del_gateway_chassis(struct ctl_context *ctx)
> +{
> +    const struct nbrec_logical_router_port *lrp;
> +    lrp = lrp_by_name_or_uuid(ctx, ctx->argv[1], true);
> +    if (!lrp) {
> +        return;
> +    }
> +    /* Find the lrp that contains 'gc', then delete it. */
> +    const char *chassis_name = ctx->argv[2];
> +    for (size_t i = 0; i < lrp->n_gateway_chassis; i++) {
> +        if (!strncmp(lrp->gateway_chassis[i]->chassis_name,
> +                    chassis_name,
> +                    strlen(lrp->gateway_chassis[i]->chassis_name))) {
> +            remove_gc(lrp, i);
> +            return;
> +        }
> +    }
> +
> +    /* Can't happen because of the database schema. */
> +    ctl_fatal("chassis %s is not added to logical port %s",
> +              chassis_name, ctx->argv[1]);
> +}
> +
> +static int
> +gateway_chassis_cmp(const void *gc1_, const void *gc2_)
> +{
> +    const struct nbrec_gateway_chassis *const *gc1p = gc1_;
> +    const struct nbrec_gateway_chassis *const *gc2p = gc2_;
> +    const struct nbrec_gateway_chassis *gc1 = *gc1p;
> +    const struct nbrec_gateway_chassis *gc2 = *gc2p;
> +
> +    return strcmp(gc1->name, gc2->name);
> +}
>


Can we also compare the priority first, as we do in the controller?

That way the admin sees the same interpretation of the list that
ovn-controller
will do.


> +
> +/* Print a list of gateway chassis. */
> +static void
> +nbctl_lrp_get_gateway_chassis(struct ctl_context *ctx)
> +{
> +    const char *id = ctx->argv[1];
> +    const struct nbrec_logical_router_port *lrp;
> +    const struct nbrec_gateway_chassis **gcs;
> +    size_t i;
> +
> +    lrp = lrp_by_name_or_uuid(ctx, id, true);
> +
> +    gcs = xmalloc(sizeof *gcs * lrp->n_gateway_chassis);
> +    for (i = 0; i < lrp->n_gateway_chassis; i++) {
> +        gcs[i] = lrp->gateway_chassis[i];
> +    }
> +
> +    qsort(gcs, lrp->n_gateway_chassis, sizeof *gcs, gateway_chassis_cmp);
> +
> +    for (i = 0; i < lrp->n_gateway_chassis; i++) {
> +        const struct nbrec_gateway_chassis *gc = gcs[i];
> +        ds_put_format(&ctx->output, "%s %5"PRId64"\n",
> +                      gc->name, gc->priority);
> +    }
> +
> +    free(gcs);
> +}
> +
>  static void
>  nbctl_lrp_add(struct ctl_context *ctx)
>  {
> @@ -3413,6 +3578,13 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>      { "lrp-add", 4, INT_MAX,
>        "ROUTER PORT MAC NETWORK... [COLUMN[:KEY]=VALUE]...",
>        NULL, nbctl_lrp_add, NULL, "--may-exist", RW },
> +    { "lrp-set-gateway-chassis", 2, 3,
> +      "PORT CHASSIS [PRIORITY]",
> +      NULL, nbctl_lrp_set_gateway_chassis, NULL, "--may-exist", RW },
> +    { "lrp-del-gateway-chassis", 2, 2, "PORT CHASSIS", NULL,
> +      nbctl_lrp_del_gateway_chassis, NULL, "", RW },
> +    { "lrp-get-gateway-chassis", 1, 1, "PORT", NULL,
> +      nbctl_lrp_get_gateway_chassis, NULL, "", RO },
>      { "lrp-del", 1, 1, "PORT", NULL, nbctl_lrp_del, NULL, "--if-exists",
> RW },
>      { "lrp-list", 1, 1, "ROUTER", NULL, nbctl_lrp_list, NULL, "", RO },
>      { "lrp-set-enabled", 2, 2, "PORT STATE", NULL, nbctl_lrp_set_enabled,
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 975d702..ded21fc 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -756,6 +756,58 @@ AT_CLEANUP
>
>  dnl ---------------------------------------------------------------------
>
> +AT_SETUP([ovn-nbctl - logical router port gateway chassis])
> +OVN_NBCTL_TEST_START
> +AT_CHECK([ovn-nbctl lr-add lr0])
> +AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24])
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [])
> +
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lp0 chassis1], [1], [],
> +[ovn-nbctl: lp0: port name not found
> +])
> +
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lp0], [1], [],
> +[ovn-nbctl: lp0: port name not found
> +])
> +
> +AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lp0 chassis1], [1], [],
> +[ovn-nbctl: lp0: port name not found
> +])
> +
> +AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lrp0 chassis1], [1], [],
> +[ovn-nbctl: chassis chassis1 is not added to logical port lrp0
> +])
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1])
> +
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
> +lrp0-chassis1     0
> +])
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1 10])
> +
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
> +lrp0-chassis1    10
> +])
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1 20])
> +
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
> +lrp0-chassis1    20
> +])
> +AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis2 5])
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
> +lrp0-chassis1    20
> +lrp0-chassis2     5
> +])
> +
> +AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lrp0 chassis1])
> +AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
> +lrp0-chassis2     5
> +])
> +
> +OVN_NBCTL_TEST_STOP
> +AT_CLEANUP
> +
> +dnl ---------------------------------------------------------------------
> +
>  AT_SETUP([ovn-nbctl - logical router port enable and disable])
>  OVN_NBCTL_TEST_START
>
> --
> 2.9.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list