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

[nickcooper-zhangtonghao]

> On Sep 19, 2016, at 10:40 PM, Guru Shetty  wrote:
> 
> On 19 September 2016 at 05:16, 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 
> 
> 
> I get the following compilation error:
> 
> ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_router’:
> ovn/utilities/ovn-nbctl.c:1493:13: error: format not a string literal and no 
> format arguments [-Werror=format-security]
>  smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
>  ^
> ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_switch’:
> ovn/utilities/ovn-nbctl.c:1540:13: error: format not a string literal and no 
> format arguments [-Werror=format-security]
>  smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
>  ^
> ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_all’:
> ovn/utilities/ovn-nbctl.c:1584:13: error: format not a string literal and no 
> format arguments [-Werror=format-security]
>  smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
>  ^
> cc1: all warnings being treated as errors
> make[2]: *** [ovn/utilities/ovn-nbctl.o] Error 1
> make[2]: *** Waiting for unfinished jobs
> make[2]: Leaving directory `/root/git/openvswitch'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/root/git/openvswitch'
> make: *** [all] Error 2
>  
> Can you please add to the unit test the scenario where we have multiple vips 
> in a single load balancer and lb-list it.. Also add the case wherein the vip 
> is just an IP address. Also, please update the version number in schema to 
> 5.3.4 (the rationale is explained in 'man ovs-vswitchd.conf.db'. Search for 
> 'db_version'.) 

I do not get the compilation error above. I submitted the v7 patch which 
updates the test case. Thanks for your review.




___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Guru Shetty]

On 19 September 2016 at 05:16, 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 get the following compilation error:

ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_router’:
ovn/utilities/ovn-nbctl.c:1493:13: error: format not a string literal and
no format arguments [-Werror=format-security]
 smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
 ^
ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_switch’:
ovn/utilities/ovn-nbctl.c:1540:13: error: format not a string literal and
no format arguments [-Werror=format-security]
 smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
 ^
ovn/utilities/ovn-nbctl.c: In function ‘nbctl_lb_list_all’:
ovn/utilities/ovn-nbctl.c:1584:13: error: format not a string literal and
no format arguments [-Werror=format-security]
 smap_add_format(lbs, ds_cstr(&key), ds_cstr(&val));
 ^
cc1: all warnings being treated as errors
make[2]: *** [ovn/utilities/ovn-nbctl.o] Error 1
make[2]: *** Waiting for unfinished jobs
make[2]: Leaving directory `/root/git/openvswitch'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/root/git/openvswitch'
make: *** [all] Error 2

Can you please add to the unit test the scenario where we have multiple
vips in a single load balancer and lb-list it.. Also add the case wherein
the vip is just an IP address. Also, please update the version number in
schema to 5.3.4 (the rationale is explained in 'man ovs-vswitchd.conf.db'.
Search for 'db_version'.)


---
>  ovn/ovn-nb.ovsschema  |   3 +-
>  ovn/ovn-nb.xml|   6 +
>  ovn/utilities/ovn-nbctl.8.xml | 109 +
>  ovn/utilities/ovn-nbctl.c | 514 ++
> +++-
>  tests/ovn-nbctl.at| 147 
>  5 files changed, 777 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index b7e70aa..7772ad2 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",
> +"cksum": "1191768021 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 9a8bdbd..2ebae29 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.
> +
> +
> +