[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openvswitch-dev
Subject:    [ovs-dev] [RFC 2/2] ovn: Add minimal software l2 gateway.
From:       russell () ovn ! org (Russell Bryant)
Date:       2016-03-31 20:58:51
Message-ID: CA+0q_PhVxE+D0PskmFqQh2ewHgD7kNhQx8tF6SSWDRQkNcUybg () mail ! gmail ! com
[Download RAW message or body]

On Thu, Mar 31, 2016 at 4:14 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Tue, Mar 29, 2016 at 06:47:43PM -0700, Russell Bryant wrote:
> > This patch implements one approach to using ovn-controller to implement
> > a software l2 gateway between logical and physical networks.
> >
> > A new logical port type called "gateway" is introduced here.  It is very
> > close to how localnet ports work, with the following exception:
> > A localnet port makes OVN use the physical network as the
> > transport between hypervisors instead of tunnels. A gateway port still
> > uses tunnels between all hypervisors, and packets only go to/from the
> > specified physical network as needed via the chassis the gateway port
> > is bound to.
> >
> > Signed-off-by: Russell Bryant <russell at ovn.org>
>
> This isn't a full review yet but I have some comments.
>
> Below, I don't think it's guaranteed that 'chassis_id != NULL'.  There's
> a check for that just above the patch context, and I don't think that
> br_int != NULL implies that chassis_id != NULL.  Chasing the usage of
> chassis_id down a few levels, it appears to be used without a null test.


OK, I can fix that up.

It'd be a broken setup for it to be NULL, but we definitely shouldn't crash
in that case.  :-)


>
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -293,7 +293,7 @@ main(int argc, char *argv[])
> >          }
> >
> >          if (br_int) {
> > -            patch_run(&ctx, br_int, &local_datapaths);
> > +            patch_run(&ctx, br_int, &local_datapaths, chassis_id);
> >
> >              struct lport_index lports;
> >              struct mcgroup_index mcgroups;
>
> When I apply the hunk below I get two <dt>s in a row about "localnet",
> which seems like a mistake.  Is the second one supposed to be about
> "gateway"?


It's a typo.  "localnet" should be "gateway" in the hunk of the patch
quoted below.  copy/paste error.

The two port types are *very* similar.  It's just that some external entity
will bind a gateway port to a single chassis.  OpenStack would do this
binding in that case.  Packets to/from the physical network use this
gateway port.  Otherwise, the overlay is used as usual.

> --- a/ovn/ovn-sb.xml
> > +++ b/ovn/ovn-sb.xml
> > @@ -1299,6 +1299,14 @@ tcp.flags = RST;
> >              to model direct connectivity to an existing network.
> >            </dd>
> >
> > +          <dt><code>localnet</code></dt>
> > +          <dd>
> > +            A connection to a physical network.  The chassis this
> > +            <ref table="Port_Binding"/> is bound to will serve as
> > +            an L2 gateway to the network named by
> > +            <ref column="options"
> table="Port_Binding"/>:<code>network_name</code>.
> > +          </dd>
> > +
> >            <dt><code>vtep</code></dt>
> >            <dd>
> >              A port to a logical switch on a VTEP gateway chassis.  In
> order to
>



-- 
Russell Bryant

[prev in list] [next in list] [prev in thread] [next in thread]