Re: [ovs-dev] [PATCH v7] ovn-nbctl: Add LB commands.

2016-09-21 Thread nickcooper-zhangtonghao 

> On Sep 20, 2016, at 4:22 AM, Guru Shetty  wrote:
> 
> 
> On 19 September 2016 at 09:38, nickcooper-zhangtonghao 
>  > 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 
> 
> 
> 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.


The 'ovn-nbctl lb-add’ will check the vip and ips. The protocol will be 
unnecessary when there are no L4 ports.
The ips should not contain port number if the vip does not contains a port 
number. each ip of ips should contain a
port number if the vip contains a port number. There are 16 characters for LB 
name. The v8 patch updates the test case.

Unfortunately， I can’t get the compilation error, ./configure —enable-Werror
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v7] ovn-nbctl: Add LB commands.

2016-09-19 Thread Guru Shetty 
On 19 September 2016 at 09:38, nickcooper-zhangtonghao <
nickcooper-zhangtong...@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.



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\
   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(>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 * ,>vips), lb_vip,
lb_ips);
+smap_replace(CONST_CAST(struct smap *, >vips), lb_vip,
+ lb_ips);
 } else {
 /* Add the new vips. */
-smap_add(CONST_CAST(struct smap * ,>vips), lb_vip,
lb_ips);
+smap_add(CONST_CAST(struct smap *, >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(,"\n%49s %-8s %-20s %s",
+ds_put_format(, "\n%49s %-8s %-20s %s",
 "",
 lb->protocol,
 node->key,
@@ -1490,7

Re: [ovs-dev] [PATCH v7] ovn-nbctl: Add LB commands.

2016-09-19 Thread Guru Shetty 
On 19 September 2016 at 09:38, nickcooper-zhangtonghao <
nickcooper-zhangtong...@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>
>

I also forgot to mention that only 8 characters for LB name when you do a
"lb-list" looks aggressive. I suggest increasing it to atleast 16
characters.


> ---
>  ovn/ovn-nb.ovsschema  |   5 +-
>  ovn/ovn-nb.xml|   6 +
>  ovn/utilities/ovn-nbctl.8.xml | 109 +
>  ovn/utilities/ovn-nbctl.c | 514 ++
> +++-
>  tests/ovn-nbctl.at| 167 ++
>  5 files changed, 798 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index b7e70aa..5f2f2bf 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "5.3.3",
> -"cksum": "2442952958 9945",
> +"version": "5.3.4",
> +"cksum": "1155817817 9975",
>  "tables": {
>  "NB_Global": {
>  "columns": {
> @@ -97,6 +97,7 @@
>  "isRoot": true},
>  "Load_Balancer": {
>  "columns": {
> +   "name": {"type": "string"},
>  "vips": {
>  "type": {"key": "string", "value": "string",
>   "min": 0, "max": "unlimited"}},
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index c45a444..b7690d0 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -676,6 +676,12 @@
>Each row represents one load balancer.
>  
>
> +
> +  A name for the load balancer.  This name has no special meaning or
> +  purpose other than to provide convenience for human interaction with
> +  the ovn-nb database.
> +
> +
>  
>
>  A map of virtual IPv4 addresses (and an optional port number with
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 76cf97e..bdccc23 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -400,6 +400,115 @@
>
>  
>
> +Load Balancer Commands
> +
> +  [--may-exist | --add-duplicate]
> lb-add lb vip ips
> [protocol]
> +  
> +
> + Creates a new load balancer named lb with the provided
> + vip and ips or adds the vip to
> + an existing lb.  vip should be a
> + virtual IPv4 address (or an IPv4 address and a port number with
> + : as a separator).  Examples for vip are
> + 192.168.1.4 and 192.168.1.5:8080.
> + ips should be comma separated IPv4 endpoints (or comma
> + separated IPv4 addresses and port numbers with : as
> a
> + separator).  Examples for ips are
> 10.0.0.1,10.0.0.2
> + or 20.0.0.10:8800,20.0.0.11:8800.
> +
> +
> +
> + The optional argument protocol must be either
> + tcp or udp.  This argument is useful
> when
> + a port number is provided as part of the vip.  If the
> + protocol is unspecified and a port number is provided
> as
> + part of the vip, OVN assumes the protocol
> to
> + be tcp.
> +
> +
> +
> + It is an error if the vip already exists in the load
> + balancer named lb, unless --may-exist is
> + specified.  With --add-duplicate, the command really
> + creates a new load balancer with a duplicate name.
> +
> +
> +
> + The following example adds a load balancer.
> +
> +
> +
> + lb-add lb0 30.0.0.10:80
> + 192.168.10.10:80,192.168.10.20:80,192.168.10.30:80 udp
> +
> +  
> +
> +  [--if-exists] lb-del lb
> [vip]
> +  
> +Deletes lb or the vip from lb.
> +If vip is supplied, only the vip will be
> +deleted from the lb.  If only the lb is
> supplied,
> +the lb will be deleted.  It is an error if
> vip
> +does not already exist in lb, unless
> +--if-exists is specified.
> +  
> +
> +  lb-list [lb]
> +  
> +Lists the LBs.  If lb is also specified, then only the
> +specified lb will be listed.
> +  
> +
> +  [--may-exist] ls-lb-add
> switch lb
> +  
> +Adds the specified lb to switch.
> +It is an error if a load balancer named lb already
> exists
> +in the switch, unless --may-exist is
>

[ovs-dev] [PATCH v7] ovn-nbctl: Add LB commands.

2016-09-19 Thread nickcooper-zhangtonghao 
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 
---
 ovn/ovn-nb.ovsschema  |   5 +-
 ovn/ovn-nb.xml|   6 +
 ovn/utilities/ovn-nbctl.8.xml | 109 +
 ovn/utilities/ovn-nbctl.c | 514 +-
 tests/ovn-nbctl.at| 167 ++
 5 files changed, 798 insertions(+), 3 deletions(-)

diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index b7e70aa..5f2f2bf 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
-"version": "5.3.3",
-"cksum": "2442952958 9945",
+"version": "5.3.4",
+"cksum": "1155817817 9975",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -97,6 +97,7 @@
 "isRoot": true},
 "Load_Balancer": {
 "columns": {
+   "name": {"type": "string"},
 "vips": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index c45a444..b7690d0 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -676,6 +676,12 @@
   Each row represents one load balancer.
 
 
+
+  A name for the load balancer.  This name has no special meaning or
+  purpose other than to provide convenience for human interaction with
+  the ovn-nb database.
+
+
 
   
 A map of virtual IPv4 addresses (and an optional port number with
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 76cf97e..bdccc23 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -400,6 +400,115 @@
   
 
 
+Load Balancer Commands
+
+  [--may-exist | --add-duplicate] 
lb-add lb vip ips 
[protocol]
+  
+
+ Creates a new load balancer named lb with the provided
+ vip and ips or adds the vip to
+ an existing lb.  vip should be a
+ virtual IPv4 address (or an IPv4 address and a port number with
+ : as a separator).  Examples for vip are
+ 192.168.1.4 and 192.168.1.5:8080.
+ ips should be comma separated IPv4 endpoints (or comma
+ separated IPv4 addresses and port numbers with : as a
+ separator).  Examples for ips are 10.0.0.1,10.0.0.2
+ or 20.0.0.10:8800,20.0.0.11:8800.
+
+
+
+ The optional argument protocol must be either
+ tcp or udp.  This argument is useful when
+ a port number is provided as part of the vip.  If the
+ protocol is unspecified and a port number is provided as
+ part of the vip, OVN assumes the protocol to
+ be tcp.
+
+
+
+ It is an error if the vip already exists in the load
+ balancer named lb, unless --may-exist is
+ specified.  With --add-duplicate, the command really
+ creates a new load balancer with a duplicate name.
+
+
+
+ The following example adds a load balancer.
+
+
+
+ lb-add lb0 30.0.0.10:80
+ 192.168.10.10:80,192.168.10.20:80,192.168.10.30:80 udp
+
+  
+
+  [--if-exists] lb-del lb 
[vip]
+  
+Deletes lb or the vip from lb.
+If vip is supplied, only the vip will be
+deleted from the lb.  If only the lb is supplied,
+the lb will be deleted.  It is an error if vip
+does not already exist in lb, unless
+--if-exists is specified.
+  
+
+  lb-list [lb]
+  
+Lists the LBs.  If lb is also specified, then only the
+specified lb will be listed.
+  
+
+  [--may-exist] ls-lb-add switch 
lb
+  
+Adds the specified lb to switch.
+It is an error if a load balancer named lb already exists
+in the switch, unless --may-exist is specified.
+  
+
+  [--if-exists] ls-lb-del switch 
[lb]
+  
+Removes lb from switch.  If only
+switch is supplied, all the LBs from the logical switch are
+removed.  If lb is also specified, then only the
+lb will be removed from the logical switch.
+It is an error if lb does not exist in the
+switch, unless --if-exists is specified.
+  
+
+  ls-lb-list switch [lb]
+  
+Lists the LBs.  If switch is supplied, all the LBs from
+the logical switch are