[ovs-dev] [PATCH 0/5] ovn: Add HA chassis group and 'external' port support
Numan Siddique
nusiddiq at redhat.com
Thu Feb 21 04:12:25 UTC 2019
Thanks for the review and comments.
Please see below for few comments.
Thanks
Numan
On Thu, Feb 21, 2019 at 3:29 AM Mark Michelson <mmichels at redhat.com> wrote:
> Hi Numan,
>
> This is quite a large patchset, but I believe I understand it. It
> essentially does two things:
>
> 1) Deprecates Gateway_Chassis in favor of HA_Chassis_Groups and
> HA_Chassis. The benefit of HA_Chassis_Groups is that it allows for
> automatic failover of chassis using BFD.
>
>
The existing Gateway_Chassis also supports automatic failover using BFD.
(1) actully doesn't add any new extra functionality to support HA for
gateway router ports.
It only provides a different way to do the same HA. (1) maintains full
backward compatibility so
that CMS can still use Gateway_Chassis in NB DB and ovn-northd will create
HA_Chassis_Group
in SB DB instead of creating Gateway_Chassis in SB DB.
> 2) Adds external logical switch port type that makes use of
> HA_Chassis_Groups to allow for services (such as DHCP) to be offered on
> ports outside of the OVN environment. This uses the HA_Chassis_Groups in
> order to "bind" the external port to a specific instance of ovn-controller.
>
> The problem I'm having is that I don't really understand what value (1)
> is adding. It seems like it's just changing the syntax behind the
> existing Gateway_Chassis. You can already specify multiple
> Gateway_Chassis for a logical router port, and you can associate a
> priority with a Gateway_Chassis, allowing for controlled failover. And
> it seems like you could do the same thing for (2) as well. Am I missing
> something?
>
>
The present Gateway_Chassis approach is very much tied to the logical
router ports.
And I felt odd to associate a set of Gateway_Chassis to 'external' logical
ports.
Right now the association of the logical_router_port to Gateway_Chassis in
NB DB
is a strong reference.
If we want to use the existing Gateway_Chassis , we need to add a column
'gateway_chassis' in
Logical_switch_port. If suppose there are like 10 'external' ports in a
logical switch, CMS has to create 10 sets
of Gateway_Chassis rows (with each set representing few chassis which
provides gateway
functionality) and associate each set to the logical_switch_port.
Right now Gateway_Chassis table in the schema doesn't have 'is_Root' set to
True.
So I think using the present Gateway_Chassis table for the HA of 'external'
ports would be more complicated
for CMS and also semantically it feels odd for a logical port to have
Gatway_Chassis associated to it.
That's the reason I thought I would add a generic table which could be used
by both distributed logical router ports
and 'external' ports.
This patch series also simplifies the code to establish BFD tunnels. I felt
the present
code [1] which does this is quite complicated.
Please let me know your thoughts and and if you have any more questions.
[1] -
https://github.com/openvswitch/ovs/blob/master/ovn/controller/bfd.c#L210
https://github.com/openvswitch/ovs/blob/master/ovn/controller/bfd.c#L91
> As far as individual critiques of the code, I would recommend
> double-checking the spelling of "chassis" throughout the code. Sometimes
> it's "chassi" and other times it's "chasss". Also, in
> ovn/controller/physical.c, I noticed that the "Gateway_Chassis" is still
> referenced in an error message instead of HA_Chassis. Other than that, I
> didn't see anything that was jumped out as wrong.
>
>
Thanks I will address that.
Numan
> On 2/18/19 2:26 PM, nusiddiq at redhat.com wrote:
> > From: Numan Siddique <nusiddiq at redhat.com>
> >
> > This patch series adds a generic HA chassis group support in OVN
> > deprecating the existing Gateway chassis support. The final patch
> > of the series adds the 'external' port support in OVN.
> > The 'external' port patch addresses the review comments from Han Zhou
> > which he provided when 'external' port patch was submitted without
> > the HA support.
> >
> > A generic HA chassis group support is added so that both the distributed
> > logical router ports (providing gateway functionality) and 'external'
> > ports can use it for HA and also to simplify the existing HA code
> > (which seems to be a bit complicated).
> >
> > To support HA, BFD is configured on tunnel ports. And even though
> > 'external' ports are expected to be used with the logical
> > switches having localnet ports (representing physical networks),
> > BFD is used for now since each chassis uses geneve tunnels with
> > all other chassis in the OVN cluster.
> >
> > Numan Siddique (5):
> > ovn-northd: Reuse the hmaps - datapaths and ports in ovnsb_db_run()
> > ovn: Add generic HA chassis group
> > ovn-controller: Make use of ha_chassis_group table to bind the
> > chassisredirect ports
> > ovn-northd: Delete the references to gateway_chasss in SB DB
> > ovn: Support a new Logical_Switch_Port.type - 'external'
> >
> > NEWS | 3 +
> > ovn/controller/automake.mk | 4 +-
> > ovn/controller/bfd.c | 229 +++----
> > ovn/controller/bfd.h | 11 +-
> > ovn/controller/binding.c | 31 +-
> > ovn/controller/binding.h | 1 -
> > ovn/controller/gchassis.c | 222 -------
> > ovn/controller/gchassis.h | 71 ---
> > ovn/controller/ha-chassis.c | 203 ++++++
> > ovn/controller/ha-chassis.h | 50 ++
> > ovn/controller/lflow.c | 29 +-
> > ovn/controller/lflow.h | 3 +-
> > ovn/controller/ovn-controller.c | 14 +-
> > ovn/controller/physical.c | 109 ++--
> > ovn/controller/physical.h | 3 +-
> > ovn/controller/pinctrl.c | 38 +-
> > ovn/controller/pinctrl.h | 1 -
> > ovn/lib/chassis-index.c | 26 +
> > ovn/lib/chassis-index.h | 4 +
> > ovn/lib/ovn-util.c | 1 +
> > ovn/northd/ovn-northd.8.xml | 37 +-
> > ovn/northd/ovn-northd.c | 779 ++++++++++++++++-------
> > ovn/ovn-architecture.7.xml | 71 +++
> > ovn/ovn-nb.ovsschema | 42 +-
> > ovn/ovn-nb.xml | 132 ++++
> > ovn/ovn-sb.ovsschema | 43 +-
> > ovn/ovn-sb.xml | 63 ++
> > ovn/utilities/ovn-nbctl.8.xml | 41 ++
> > ovn/utilities/ovn-nbctl.c | 221 +++++++
> > ovn/utilities/ovn-sbctl.c | 6 +
> > tests/ovn-northd.at | 396 +++++++++++-
> > tests/ovn.at | 1031 ++++++++++++++++++++++++++++++-
> > 32 files changed, 3061 insertions(+), 854 deletions(-)
> > delete mode 100644 ovn/controller/gchassis.c
> > delete mode 100644 ovn/controller/gchassis.h
> > create mode 100644 ovn/controller/ha-chassis.c
> > create mode 100644 ovn/controller/ha-chassis.h
> >
>
>
More information about the dev
mailing list