> On Jun 22, 2016, at 10:28 AM, Zong Kai LI <zealo...@gmail.com> wrote:
> 
> This patch renames Logical_Router_Port 'network' column to 'networks'
> and aligns it to Logical_Switch_Port 'addresses' column for peer port
> in Logical_Switch_Port which type is 'router'.
> 
> Currently, a lsp port whos type is 'router', may have multiple addresses
> for IPv6 address case, but it's peer lrp port has only 1 address in
> 'network' column. It's inconsistent, this patch tries to fix that.
> 
> Signed-off-by: Zong Kai LI <zealo...@gmail.com>
> ---
> ovn/northd/ovn-northd.c   |  5 +++--
> ovn/ovn-nb.ovsschema      |  6 ++++--
> ovn/ovn-nb.xml            |  7 +++---
> ovn/utilities/ovn-nbctl.c | 54 +++++++++++++++++++++++++++++++++++------------
> tests/ovn-nbctl.at        |  9 ++++++++
> 5 files changed, 61 insertions(+), 20 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 17713ec..5851fca 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -612,11 +612,12 @@ join_logical_ports(struct northd_context *ctx,
>                 }
> 
>                 ovs_be32 ip, mask;
> -                char *error = ip_parse_masked(nbr->network, &ip, &mask);
> +                /* It should have only one IPv4 address. */
> +                char *error = ip_parse_masked(nbr->networks[0], &ip, &mask);

You changed the schema to allow not specifying a network, but don't check if 
one is not defined.  I think this will cause a crash in that instance.

> 
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 58f04b2..4a345c8 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",
> +    "cksum": "2770494524 6251",

You need to change the version number when you modify the schema.

>     "tables": {
>         "Logical_Switch": {
>             "columns": {
> @@ -95,7 +95,9 @@
>         "Logical_Router_Port": {
>             "columns": {
>                 "name": {"type": "string"},
> -                "network": {"type": "string"},
> +                "networks": {"type": {"key": "string",
> +                                      "min": 0,
> +                                      "max": "unlimited"}},
...
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -689,11 +689,12 @@
>       </p>
>     </column>
> 
> -    <column name="network">
> -      The IP address of the router and the netmask.  For example,
> +    <column name="networks">
> +      The IP addresses of the router and the netmask.  For example,
>       <code>192.168.0.1/24</code> indicates that the router's IP address is
>       192.168.0.1 and that packets destined to 192.168.0.<var>x</var> should 
> be
> -      routed to this port.
> +      routed to this port. A router port should have only one IPv4 address, 
> or
> +      multiple IPv6 addresses.

Is there a reason to limit it to one IPv4 address, but allow multiple IPv6 
addresses?  Also, this gives the impression that IPv6 routing is implemented, 
but it's not at this point.

I have a patch in my tree that adds support for multiple IPv4 addresses in 
addition to multiple IPv6 addresses.  My preference would be to add multiple 
lrp network support to the ovn-nbctl command at the same time that ovn-northd 
can make use of them.  I'm planning to send out my series within the next 
couple of weeks.

Thanks,

--Justin


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

Reply via email to