On Fri, Jun 24, 2016 at 4:43 AM, Justin Pettit <jpet...@ovn.org> wrote:

>
> > 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.
>
> Sure, I will try to fix this, and add a test for the case you mentioned.


> >
> > 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.
>
> Oops, thanks for mention that.


> > -    <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
>
>
>
To be honest, I start to think about it just from IPv6 side, not IPv4. So I
may make some mistake, and afer I checked ovn-nb.xml for
Logical_Switch_Port table addresses column, I'm sure I've made a mistake.
Maybe multiple IPv4 addresses should be supported also. Good catch.
For IPv6, multiple IPv6 addresses in Logical_Router_Port network column
will benefit the implementation for RA/SLAAC, cause they're prefixes
natively. For IPv4, I'm not sure yet.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to