[ovs-dev] [PATCH ovn v2 00/13] OVN Interconnection
Han Zhou
hzhou at ovn.org
Thu Nov 14 18:13:59 UTC 2019
On Wed, Nov 13, 2019 at 10:27 AM Numan Siddique <numans at ovn.org> wrote:
>
> On Thu, Oct 31, 2019 at 2:43 AM Han Zhou <hzhou at ovn.org> wrote:
> >
> > The series supports interconnecting multiple OVN deployments (e.g.
located at
> > multiple data centers) through logical routers connected with tansit
logical
> > switches with overlay tunnels, managed through OVN control plane. See
the
> > ovn-architecture.rst document updates for more details, and find the
> > instructions in Documentation/tutorials/ovn-interconnection.rst.
> >
> > Han Zhou (13):
> > ovn-architecture: Add documentation for OVN interconnection feature.
> > ovn-inb: Interconnection northbound DB schema and CLI.
> > ovn-isb: Interconnection southbound DB schema and CLI.
> > ovn-ic: Interconnection controller with AZ registeration.
> > ovn-northd.c: Refactor allocate_tnlid.
> > ovn-ic: Transit switch controller.
> > ovn-sb: Add columns is_interconn and is_remote to Chassis.
> > ovn-ic: Interconnection gateway controller.
> > ovn-ic: Interconnection port controller.
> > ovn.at: e2e test for OVN interconnection.
> > ovn-ctl: Refactor to reduce redundant code.
> > ovn-ctl: Support commands for interconnection.
> > tutorial: Add tutorial for OVN Interconnection.
> >
>
> Hi Han,
>
> Thanks for working on this feature. It's an interesting use case.
>
> I had a quick look at all the patches.
>
Numan, thanks a lot for the thorough review! Please see my response inlined.
> I have few comments
>
> 1. I would suggest to rename the DBs as ovn-ic-nb.ovsschema (and the
> same for ovn-isb).
> The DB name is - OVN_IC_Northbound. So it would make sense to have
> - ovn-ic-nb.ovsschema
> I would also suggest to rename the utilities to ovn-ic-nbctl and
> ovn-ic-sbctl.
> With ovn-inbctl/ovn-isbctl, it is really confusing.
>
Sure, I felt not quite convenient with two dashes in a command name. I
agree that ovn-ic-nbctl and ovn-ic-sbctl are more clear. I can change it.
> 2. ovn-ic service writes to interconnect south db, ovn north db and
> ovn south db. Writing to ic south db and
> ovn north db is fine. But it seems a little odd for ovn-ic to
> write to south db. From what I understand it writes
> to south db for 3 purposes
> a. Updating the tunnel_key column of datapath_binding
> representing the transit switch
> b. Updating the key column of port_binding representing the
> logical router port connecting to the transit switch.
> c. Creating chassis rows for remote gateway chassis.
>
> I think it's better if ovn-ic can delegate all these to ovn-northd.
> For (a) and (b), ovn-ic can set the generated tunnel key
> in the other_config/options column of Logical switch/Logical switch
> port. ovn-northd can internally set this value to
> the south db.
>
> For (c), I think its better we add another table - Remote_Chassis
> (or some other appropriate name) . ovn-ic will create rows
> in this table for each remote chassis and ovn-northd will create
> chassis row in south db.
> We already have Gateway_Chassis table in North db. So I think it's
> fine if we add Remote_Chassis table. The only odd thing
> would be is to store the encap details in North db.
>
> With this, ovn-ic, doesn't need to worry about syncing between
> interconnect south db and ovn south db. In my opinion ovn-ic
> should act more like CMS to each availability zone and hence should
> not do any write transactions to the south db.
>
> Any concerns with this proposed approach ?
>
We could avoid ovn-ic writing directly to SB with some extra logic in
northd, but I don't see any problem for ovn-ic to update SB directly. First
of all, we have hypervisors creating and updating SB directly for Chassis
and Encap records. The design here is that ovn-ic updates Chassis and Encap
on behalf of remote gateway nodes, which I think is straightforward and
reasonable. Similarly, port-binding's chassis column is updated the same
way as how hypervisors are updating it. Secondly, for tunnel keys updating,
it may seem graceful to update from northd, since northd is the only client
that updates tunnel keys today, but since ovn-ic is responsible for
calculating these keys, and it already has connection to SB, I think it is
just more natural and efficient to update it directly, to avoid the extra
logic and unnecessary latency from northd with another round of
computation. The scope of the ovn-ic is only for the interconnection
objects, so I don't see any conflict or ownership ambiguity here. What do
you think?
> 3. In patch 7,its better to rename the ovs configuration option -
> "external_ids:is-interconn" to "external_ids:ovn-is-interconn".
> You also need to document it in ovn-controller-8.xml.
>
Agree!
> Or maybe we can remove this option - external_ids:is-interconn. We
> probably don't need this if we do like below
>
> 2 (c) can also be done this way
> - User creates transit switch.
> - ovn-ic creates transit switch in north db.
> - then the user adds the logical router port - logical switch
> port to the transit switch in availability zone - az1 (for example)
> - then the user creates gw chassis - for example
> ovn-nbctl lrp-set-gateway-chassis lrp-lr1-ts1 <gateway
> name> [priority]
> - ovn-ic on az1 ,learns this and creates a Gateway_Chassis
> row in the interconnect south db.
> - ovn-ic on other az's then create Remote_Chassis rows in the
North db.
> - ovn-northd on other az's then create chassis row in south db
> with "is_remote" set to true.
>
> I am not sure if this complicates further and hence its better
> that ovn-ic writes to the south dbs. But we can discuss further on
> this.
>
I think this is a good idea and I am incline to it, because it avoids one
configuration on the gateway node, which is good.
The only concern is that it makes the remote gateway sync more dynamic - it
changes when LRP's gateway-chassis/ha-chassis settings change, which may be
less efficient. The code of ovn-ic will be a little more complex. I think
it should be ok, but I'd like to hear from you, too.
>
> 4. Can you please add a section in ovn-architecture about the "Journey
> of a packet in inter-az scenario" ? This would really
> help in understanding the feature.
>
Sure, I can add it.
> Thanks
> Numan
>
>
> > .gitignore | 6 +
> > Documentation/automake.mk | 1 +
> > Documentation/tutorials/index.rst | 1 +
> > Documentation/tutorials/ovn-interconnection.rst | 188 ++++
> > Makefile.am | 1 +
> > NEWS | 5 +
> > TODO.rst | 10 +
> > automake.mk | 75 ++
> > controller/binding.c | 6 +-
> > controller/chassis.c | 14 +
> > debian/ovn-common.install | 2 +
> > debian/ovn-common.manpages | 4 +
> > ic/.gitignore | 2 +
> > ic/automake.mk | 10 +
> > ic/ovn-ic.8.xml | 111 +++
> > ic/ovn-ic.c | 1050
+++++++++++++++++++++++
> > lib/.gitignore | 6 +
> > lib/automake.mk | 32 +-
> > lib/ovn-inb-idl.ann | 9 +
> > lib/ovn-isb-idl.ann | 9 +
> > lib/ovn-util.c | 92 ++
> > lib/ovn-util.h | 15 +
> > northd/ovn-northd.c | 108 +--
> > ovn-architecture.7.xml | 107 ++-
> > ovn-inb.ovsschema | 75 ++
> > ovn-inb.xml | 371 ++++++++
> > ovn-isb.ovsschema | 129 +++
> > ovn-isb.xml | 582 +++++++++++++
> > ovn-nb.ovsschema | 5 +-
> > ovn-nb.xml | 28 +-
> > ovn-sb.ovsschema | 8 +-
> > ovn-sb.xml | 24 +
> > tests/automake.mk | 8 +-
> > tests/ovn-ic.at | 192 +++++
> > tests/ovn-inbctl.at | 65 ++
> > tests/ovn-isbctl.at | 112 +++
> > tests/ovn-macros.at | 161 +++-
> > tests/ovn.at | 149 ++++
> > tests/testsuite.at | 3 +
> > tutorial/ovs-sandbox | 78 +-
> > utilities/.gitignore | 4 +
> > utilities/automake.mk | 16 +
> > utilities/ovn-ctl | 423 ++++++++-
> > utilities/ovn-ctl.8.xml | 91 ++
> > utilities/ovn-inbctl.8.xml | 174 ++++
> > utilities/ovn-inbctl.c | 948
++++++++++++++++++++
> > utilities/ovn-isbctl.8.xml | 148 ++++
> > utilities/ovn-isbctl.c | 1015
++++++++++++++++++++++
> > 48 files changed, 6528 insertions(+), 145 deletions(-)
> > create mode 100644 Documentation/tutorials/ovn-interconnection.rst
> > create mode 100644 ic/.gitignore
> > create mode 100644 ic/automake.mk
> > create mode 100644 ic/ovn-ic.8.xml
> > create mode 100644 ic/ovn-ic.c
> > create mode 100644 lib/ovn-inb-idl.ann
> > create mode 100644 lib/ovn-isb-idl.ann
> > create mode 100644 ovn-inb.ovsschema
> > create mode 100644 ovn-inb.xml
> > create mode 100644 ovn-isb.ovsschema
> > create mode 100644 ovn-isb.xml
> > create mode 100644 tests/ovn-ic.at
> > create mode 100644 tests/ovn-inbctl.at
> > create mode 100644 tests/ovn-isbctl.at
> > create mode 100644 utilities/ovn-inbctl.8.xml
> > create mode 100644 utilities/ovn-inbctl.c
> > create mode 100644 utilities/ovn-isbctl.8.xml
> > create mode 100644 utilities/ovn-isbctl.c
> >
> > --
> > 2.1.0
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list