From patchwork Mon Sep 19 20:22:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gurucharan Shetty X-Patchwork-Id: 671938 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3sdHPJ6lS8z9s9x for ; Tue, 20 Sep 2016 06:22:28 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 4F35810573; Mon, 19 Sep 2016 13:22:27 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 115D010572 for ; Mon, 19 Sep 2016 13:22:26 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id 447D14201F6 for ; Mon, 19 Sep 2016 14:22:25 -0600 (MDT) X-ASG-Debug-ID: 1474316544-09eadd35316883c0001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar5.cudamail.com with ESMTP id kBaNFmzRIr4OgMNq (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 19 Sep 2016 14:22:24 -0600 (MDT) X-Barracuda-Envelope-From: guru@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO relay9-d.mail.gandi.net) (217.70.183.199) by mx3-pf1.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 19 Sep 2016 20:22:23 -0000 Received-SPF: pass (mx3-pf1.cudamail.com: SPF record at ovn.org designates 217.70.183.199 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.199 X-Barracuda-RBL-IP: 217.70.183.199 Received: from mfilter30-d.gandi.net (mfilter30-d.gandi.net [217.70.178.161]) by relay9-d.mail.gandi.net (Postfix) with ESMTP id 1952E406E0 for ; Mon, 19 Sep 2016 22:22:22 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter30-d.gandi.net Received: from relay9-d.mail.gandi.net ([IPv6:::ffff:217.70.183.199]) by mfilter30-d.gandi.net (mfilter30-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id l8_xkYT9Q1sa for ; Mon, 19 Sep 2016 22:22:19 +0200 (CEST) X-Originating-IP: 209.85.215.52 Received: from mail-lf0-f52.google.com (mail-lf0-f52.google.com [209.85.215.52]) (Authenticated sender: guru@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 8809740367 for ; Mon, 19 Sep 2016 22:22:18 +0200 (CEST) Received: by mail-lf0-f52.google.com with SMTP id h127so123452201lfh.0 for ; Mon, 19 Sep 2016 13:22:18 -0700 (PDT) X-Gm-Message-State: AE9vXwMWGp7WIQgnwXEUoTa5GzWOGS5Y+l3oheXfAm7b70UPEjAfVUwI/8fAwlj9HLwLwj3HnI3+1D80hYXeJA== X-Received: by 10.25.38.146 with SMTP id m140mr12426606lfm.110.1474316538294; Mon, 19 Sep 2016 13:22:18 -0700 (PDT) MIME-Version: 1.0 Received: by 10.114.172.144 with HTTP; Mon, 19 Sep 2016 13:22:17 -0700 (PDT) In-Reply-To: <1474303131-128355-1-git-send-email-nickcooper-zhangtonghao@opencloud.tech> References: <1474303131-128355-1-git-send-email-nickcooper-zhangtonghao@opencloud.tech> X-CudaMail-Envelope-Sender: guru@ovn.org From: Guru Shetty Date: Mon, 19 Sep 2016 13:22:17 -0700 X-Gmail-Original-Message-ID: Message-ID: X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V1-918041765 X-CudaMail-DTE: 091916 X-CudaMail-Originating-IP: 217.70.183.199 To: nickcooper-zhangtonghao X-ASG-Orig-Subj: [##CM-V1-918041765##]Re: [PATCH v7] ovn-nbctl: Add LB commands. X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1474316544 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Content-Filtered-By: Mailman/MimeDel 2.1.16 Cc: ovs dev Subject: Re: [ovs-dev] [PATCH v7] ovn-nbctl: Add LB commands. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" On 19 September 2016 at 09:38, nickcooper-zhangtonghao < nickcooper-zhangtonghao@opencloud.tech> wrote: > This patch provides the command line to create a load balancer. > You can create a load balancer independently and add it to multiple > switches or routers. A single load balancer can have multiple vips. > Add a name column for the load balancer. With --add-duplicate, > the command really creates a new load balancer with a duplicate name. > This name has no special meaning or purpose other than to provide > convenience for human interaction with the ovn-nb database. > This patch also provides the unit tests and the documentation. > > Signed-off-by: nickcooper-zhangtonghao opencloud.tech> > You should run your ./configure with ---enable-Werror option and you will not miss any warnings as they are treated as errors. For the case wherein the LB VIP is just a IP address, you should not automatically give it a protocol. The code does not look at protocol when there are no L4 ports (as explained in ovn-nb documentation). There are a few lines in ovn-nbctl that exceed 78 characters. You can add the following incremental to fix them (along with the compilation error). The incremental does not fix the problem of automatic protocol allocation when no port is given. lr-lb-del ROUTER [LB] remove load-balancers from ROUTER\n\ @@ -1381,7 +1383,8 @@ nbctl_lb_add(struct ctl_context *ctx) lb_proto = ctx->argv[4]; is_update_proto = true; if (strcmp(lb_proto, "tcp") && strcmp(lb_proto, "udp")) { - ctl_fatal("%s: protocol must be one of \"tcp\", \"udp\".", lb_proto); + ctl_fatal("%s: protocol must be one of \"tcp\", \"udp\".", + lb_proto); } } @@ -1391,13 +1394,15 @@ nbctl_lb_add(struct ctl_context *ctx) if (lb) { if (smap_get(&lb->vips, lb_vip)) { if (!may_exist) { - ctl_fatal("%s: a load balancer with this vip (%s) already exists", lb_name, lb_vip); + ctl_fatal("%s: a load balancer with this vip (%s) already " + "exists", lb_name, lb_vip); } /* Update the vips. */ - smap_replace(CONST_CAST(struct smap * ,&lb->vips), lb_vip, lb_ips); + smap_replace(CONST_CAST(struct smap *, &lb->vips), lb_vip, + lb_ips); } else { /* Add the new vips. */ - smap_add(CONST_CAST(struct smap * ,&lb->vips), lb_vip, lb_ips); + smap_add(CONST_CAST(struct smap *, &lb->vips), lb_vip, lb_ips); } /* Update the load balancer. */ @@ -1481,7 +1486,7 @@ nbctl_lb_list_router(struct ctl_context *ctx, struct smap *lbs, node->key, node->value); } else { - ds_put_format(&val,"\n%49s %-8s %-20s %s", + ds_put_format(&val, "\n%49s %-8s %-20s %s", "", lb->protocol, node->key, @@ -1490,7 +1495,7 @@ nbctl_lb_list_router(struct ctl_context *ctx, struct smap *lbs, } ds_put_format(&key, "%-10.8s", lb->name); - smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val)); + smap_add(lbs, ds_cstr(&key), ds_cstr(&val)); ds_destroy(&key); ds_destroy(&val); @@ -1537,7 +1542,7 @@ nbctl_lb_list_switch(struct ctl_context *ctx, struct smap *lbs, } ds_put_format(&key, "%-10.8s", lb->name); - smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val)); + smap_add(lbs, ds_cstr(&key), ds_cstr(&val)); ds_destroy(&key); ds_destroy(&val); @@ -1581,7 +1586,7 @@ nbctl_lb_list_all(struct ctl_context *ctx, struct smap *lbs, } ds_put_format(&key, "%-10.8s", lb->name); - smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val)); + smap_add(lbs, ds_cstr(&key), ds_cstr(&val)); ds_destroy(&key); ds_destroy(&val); @@ -1635,8 +1640,8 @@ nbctl_lr_lb_add(struct ctl_context *ctx) if (may_exist) { return; } - ctl_fatal(UUID_FMT " : a load balancer with this UUID already exists", - UUID_ARGS(&lb->header_.uuid)); + ctl_fatal(UUID_FMT " : a load balancer with this UUID already " + "exists", UUID_ARGS(&lb->header_.uuid)); } } @@ -1646,8 +1651,10 @@ nbctl_lr_lb_add(struct ctl_context *ctx) = xmalloc(sizeof *new_lbs * (lr->n_load_balancer + 1)); memcpy(new_lbs, lr->load_balancer, sizeof *new_lbs * lr->n_load_balancer); - new_lbs[lr->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *, new_lb); - nbrec_logical_router_set_load_balancer(lr, new_lbs, lr->n_load_balancer + 1); + new_lbs[lr->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *, + new_lb); + nbrec_logical_router_set_load_balancer(lr, new_lbs, + lr->n_load_balancer + 1); free(new_lbs); } @@ -1676,7 +1683,8 @@ nbctl_lr_lb_del(struct ctl_context *ctx) nbrec_logical_router_verify_load_balancer(lr); struct nbrec_load_balancer **new_lbs - = xmemdup(lr->load_balancer, sizeof *new_lbs * lr->n_load_balancer); + = xmemdup(lr->load_balancer, + sizeof *new_lbs * lr->n_load_balancer); new_lbs[i] = lr->load_balancer[lr->n_load_balancer - 1]; nbrec_logical_router_set_load_balancer(lr, new_lbs, lr->n_load_balancer - 1); @@ -1701,7 +1709,8 @@ nbctl_lr_lb_list(struct ctl_context *ctx) if (ctx->argc == 2) { nodes = nbctl_lb_list_router(ctx, &lbs, ctx->argv[1], NULL, false); } else { - nodes = nbctl_lb_list_router(ctx, &lbs, ctx->argv[1], ctx->argv[2], true); + nodes = nbctl_lb_list_router(ctx, &lbs, ctx->argv[1], ctx->argv[2], + true); } if (nodes) { @@ -1735,8 +1744,8 @@ nbctl_ls_lb_add(struct ctl_context *ctx) if (may_exist) { return; } - ctl_fatal(UUID_FMT " : a load balancer with this UUID already exists", - UUID_ARGS(&lb->header_.uuid)); + ctl_fatal(UUID_FMT " : a load balancer with this UUID already " + "exists", UUID_ARGS(&lb->header_.uuid)); } } @@ -1746,8 +1755,10 @@ nbctl_ls_lb_add(struct ctl_context *ctx) = xmalloc(sizeof *new_lbs * (ls->n_load_balancer + 1)); memcpy(new_lbs, ls->load_balancer, sizeof *new_lbs * ls->n_load_balancer); - new_lbs[ls->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *, new_lb); - nbrec_logical_switch_set_load_balancer(ls, new_lbs, ls->n_load_balancer + 1); + new_lbs[ls->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *, + new_lb); + nbrec_logical_switch_set_load_balancer(ls, new_lbs, + ls->n_load_balancer + 1); free(new_lbs); } @@ -1776,7 +1787,8 @@ nbctl_ls_lb_del(struct ctl_context *ctx) nbrec_logical_switch_verify_load_balancer(ls); struct nbrec_load_balancer **new_lbs - = xmemdup(ls->load_balancer, sizeof *new_lbs * ls->n_load_balancer); + = xmemdup(ls->load_balancer, + sizeof *new_lbs * ls->n_load_balancer); new_lbs[i] = ls->load_balancer[ls->n_load_balancer - 1]; nbrec_logical_switch_set_load_balancer(ls, new_lbs, ls->n_load_balancer - 1); @@ -1801,7 +1813,8 @@ nbctl_ls_lb_list(struct ctl_context *ctx) if (ctx->argc == 2) { nodes = nbctl_lb_list_switch(ctx, &lbs, ctx->argv[1], NULL, false); } else { - nodes = nbctl_lb_list_switch(ctx, &lbs, ctx->argv[1], ctx->argv[2], true); + nodes = nbctl_lb_list_switch(ctx, &lbs, ctx->argv[1], ctx->argv[2], + true); } if (nodes) { diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml index bdccc23..991ecea 100644 --- a/ovn/utilities/ovn-nbctl.8.xml +++ b/ovn/utilities/ovn-nbctl.8.xml @@ -478,9 +478,9 @@
ls-lb-list switch [lb]
- Lists the LBs. If switch is supplied, all the LBs from - the logical switch are listed. If lb is also specified, - only the lb will be listed from the logical switch. + Lists the LBs for the given switch. If lb is + also specified, only the lb will be listed from the logical + switch.
[--may-exist] lr-lb-add router lb
@@ -502,9 +502,9 @@
lr-lb-list router [lb]
- Lists the LBs. If router is supplied, all the LBs from - the logical router are listed. If lb is also specified, - then only the lb will be listed from the logical router. + Lists the LBs for the given router. If lb is + also specified, then only the lb will be listed from the + logical router.
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index f47070f..2541ac5 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -386,8 +386,10 @@ Route commands:\n\ \n\ LB commands:\n\ lb-add LB VIP[:PORT] IP[:PORT]... [PROTOCOL]\n\ - add a load-balancer or VIP to load balancer\n\ - lb-del LB [VIP] remove a load-balancer or VIP from load balancer\n\ + create a load-balancer or add a VIP to an\n\ + existing load balancer\n\ + lb-del LB [VIP] remove a load-balancer or just the VIP from\n\ + the load balancer\n\ lb-list [LB] print load-balancers\n\ lr-lb-add ROUTER LB add a load-balancer to ROUTER\n\