diff mbox

[ovs-dev,5/7] ovn-nb: Add Load-balancer table to schema.

Message ID 1467188231-10194-6-git-send-email-guru@ovn.org
State Accepted
Headers show

Commit Message

Gurucharan Shetty June 29, 2016, 8:17 a.m. UTC
Also add the ability to run database commands on the
new schema using ovn-nbctl.

Signed-off-by: Gurucharan Shetty <guru@ovn.org>
---
 ovn/ovn-nb.ovsschema      | 22 ++++++++++++++++++++--
 ovn/ovn-nb.xml            | 41 +++++++++++++++++++++++++++++++++++++++++
 ovn/utilities/ovn-nbctl.c |  4 ++++
 3 files changed, 65 insertions(+), 2 deletions(-)

Comments

Zong Kai LI July 1, 2016, 8:21 a.m. UTC | #1
Hi, Guru.


> +  <table name="Load_Balancer" title="load balancer">
> +    <p>
> +      Each row represents one load balancer.
> +    </p>
> +
> +    <column name="vips">
> +      <p>
> +        A map of virtual IPv4 addresses (and an optional port number
> separated
> +        by <code>:</code>) associated with this load balancer and
> +        their corresponding endpoint IPv4 addresses (and optional port
> numbers
> +        separated by <code>:</code>) separated by commas.  If
>

I'm not sure whether will this confuse others, but it confused me.
At first, I thougth it would be:
ovn-nbctl create load-balancer vips:'"30.0.0.1:8000,40.0.0.1:5000"'='"
10.0.0.1:80,10.0.0.2:80,10.0.0.3:80"'
it inserts successfully into DB, but fails in ovn-northd for "bad ip port
for load balancer key".
Later, when I read your patch 7/7, I recognize it should be:
ovn-nbctl create load-balancer vips:'"30.0.0.1:8000"'='"10.0.0.1:80,
10.0.0.2:80,10.0.0.3:80"' vips:'"30.0.0.1:5000"'='"10.0.0.1:80,10.0.0.2:80,
10.0.0.3:80"', this time it get correctly parsed in ovn-northd.

Woud you mind to update the description here to notice it's a list of maps,
or/and give some examples?

Thanks.
Zong Kai, LI
Gurucharan Shetty July 1, 2016, 2:31 p.m. UTC | #2
On 1 July 2016 at 01:21, Zong Kai LI <zealokii@gmail.com> wrote:

> Hi, Guru.
>
>
> > +  <table name="Load_Balancer" title="load balancer">
> > +    <p>
> > +      Each row represents one load balancer.
> > +    </p>
> > +
> > +    <column name="vips">
> > +      <p>
> > +        A map of virtual IPv4 addresses (and an optional port number
> > separated
> > +        by <code>:</code>) associated with this load balancer and
> > +        their corresponding endpoint IPv4 addresses (and optional port
> > numbers
> > +        separated by <code>:</code>) separated by commas.  If
> >
>
> I'm not sure whether will this confuse others, but it confused me.
> At first, I thougth it would be:
> ovn-nbctl create load-balancer vips:'"30.0.0.1:8000,40.0.0.1:5000"'='"
> 10.0.0.1:80,10.0.0.2:80,10.0.0.3:80"'
> it inserts successfully into DB, but fails in ovn-northd for "bad ip port
> for load balancer key".
> Later, when I read your patch 7/7, I recognize it should be:
> ovn-nbctl create load-balancer vips:'"30.0.0.1:8000"'='"10.0.0.1:80,
> 10.0.0.2:80,10.0.0.3:80"' vips:'"30.0.0.1:5000"'='"10.0.0.1:80,10.0.0.2:80
> ,
> 10.0.0.3:80"', this time it get correctly parsed in ovn-northd.
>
> Woud you mind to update the description here to notice it's a list of maps,
> or/and give some examples?
>
The patch0 of the series did give a couple of examples. But I see your
point. I will add examples in the man page as part of v2.


>
> Thanks.
> Zong Kai, LI
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Ben Pfaff July 3, 2016, 4:45 a.m. UTC | #3
On Wed, Jun 29, 2016 at 01:17:09AM -0700, Gurucharan Shetty wrote:
> Also add the ability to run database commands on the
> new schema using ovn-nbctl.
> 
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

I agree with Zong's suggestion to add examples.

It is a little unusual, for OVS, to add documentation for a new feature
in a different commit from the implementation for the new feature.  It
might make sense to squash this with the implementation patch.

I haven't read ahead to the implementation patches, but I am pleased
with the docoumentation and schema for the moment, especially if you add
an example, so:
Acked-by: Ben Pfaff <blp@ovn.org>
diff mbox

Patch

diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 58f04b2..f73ec7d 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
-    "version": "3.1.0",
-    "cksum": "1426508118 6135",
+    "version": "3.2.0",
+    "cksum": "3068657893 7054",
     "tables": {
         "Logical_Switch": {
             "columns": {
@@ -16,6 +16,11 @@ 
                                           "refType": "strong"},
                                   "min": 0,
                                   "max": "unlimited"}},
+                "load_balancer": {"type": {"key": {"type": "uuid",
+                                                  "refTable": "Load_Balancer",
+                                                  "refType": "strong"},
+                                           "min": 0,
+                                           "max": 1}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
@@ -48,6 +53,19 @@ 
                              "min": 0, "max": "unlimited"}}},
             "indexes": [["name"]],
             "isRoot": false},
+        "Load_Balancer": {
+            "columns": {
+                "vips": {
+                    "type": {"key": "string", "value": "string",
+                             "min": 0, "max": "unlimited"}},
+                "protocol": {
+                    "type": {"key": {"type": "string",
+                             "enum": ["set", ["tcp", "udp"]]},
+                             "min": 0, "max": 1}},
+                "external_ids": {
+                    "type": {"key": "string", "value": "string",
+                             "min": 0, "max": "unlimited"}}},
+            "isRoot": true},
         "ACL": {
             "columns": {
                 "priority": {"type": {"key": {"type": "integer",
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 6355c44..0dd6b44 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -69,6 +69,11 @@ 
       </p>
     </column>
 
+    <column name="load_balancer">
+      Load balance a virtual ipv4 address to a set of logical port endpoint
+      ipv4 addresses.
+    </column>
+
     <column name="acls">
       Access control rules that apply to packets within the logical switch.
     </column>
@@ -484,6 +489,42 @@ 
     </group>
   </table>
 
+  <table name="Load_Balancer" title="load balancer">
+    <p>
+      Each row represents one load balancer.
+    </p>
+
+    <column name="vips">
+      <p>
+        A map of virtual IPv4 addresses (and an optional port number separated
+        by <code>:</code>) associated with this load balancer and
+        their corresponding endpoint IPv4 addresses (and optional port numbers
+        separated by <code>:</code>) separated by commas.  If
+        the destination IP address (and port number) of a packet leaving a
+        container or a VM matches the virtual IPv4 address (and port number)
+        provided here as a key, then OVN will statefully replace the
+        destination IP address by one of the provided IPv4 address (and port
+        number) in this map as a value.
+      </p>
+    </column>
+
+    <column name="protocol">
+      <p>
+        Valid protocols are <code>tcp</code> or <code>udp</code>.  This column
+        is useful when a port number is provided as part of the
+        <code>vips</code> column.  If this column is empty and a port number
+        is provided as part of <code>vips</code> column, OVN assumes the
+        protocol to be <code>tcp</code>.
+      </p>
+    </column>
+
+    <group title="Common Columns">
+      <column name="external_ids">
+        See <em>External IDs</em> at the beginning of this document.
+      </column>
+    </group>
+  </table>
+
   <table name="ACL" title="Access Control List (ACL) rule">
     <p>
       Each row in this table represents one ACL rule for a logical switch
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 345647a..18b72ae 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1847,6 +1847,10 @@  static const struct ctl_table_class tables[] = {
      {{NULL, NULL, NULL},
       {NULL, NULL, NULL}}},
 
+    {&nbrec_table_load_balancer,
+     {{NULL, NULL, NULL},
+      {NULL, NULL, NULL}}},
+
     {&nbrec_table_logical_router,
      {{&nbrec_table_logical_router, &nbrec_logical_router_col_name, NULL},
       {NULL, NULL, NULL}}},