On Mon, Jul 10, 2017 at 5:38 AM, Miguel Angel Ajo Pelayo
<majop...@redhat.com> wrote:
>
>
> On Fri, Jul 7, 2017 at 9:09 PM, Russell Bryant <russ...@ovn.org> wrote:
>>
>> review part 2 of this one ...
>>
>> On Wed, Jul 5, 2017 at 9:45 AM, Miguel Angel Ajo <majop...@redhat.com>
>> wrote:
>> > This patch handles multiple gateway_chassis with in chassisredirect
>> > ports, any gateway with a chassis redirect port will implement the
>> > rules to de-encapsulate incomming packets for such port.
>> >
>> > And hosts targetting a remote chassisredirect port will setup a
>> > bundle(active_backup, ..) action to each tunnel port, in the given
>> > priority order.
>> >
>> > Signed-off-by: Miguel Angel Ajo <majop...@redhat.com>
>> > Signed-off-by: Venkata Anil Kommaddi <vkomm...@redhat.com>
>> > ---
>> > ovn/controller/automake.mk | 2 +
>> > ovn/controller/binding.c | 13 +-
>> > ovn/controller/binding.h | 1 +
>> > ovn/controller/gchassis.c | 176 +++++++++++++++++++++
>> > ovn/controller/gchassis.h | 63 ++++++++
>> > ovn/controller/lflow.c | 7 +-
>> > ovn/controller/lport.h | 4 +
>> > ovn/controller/ovn-controller.c | 15 +-
>> > ovn/controller/physical.c | 124 ++++++++++++---
>> > ovn/controller/physical.h | 4 +-
>> > tests/ovn.at | 330
>> > +++++++++++++++++++++++++++++++++++++++-
>> > 11 files changed, 710 insertions(+), 29 deletions(-)
>> > create mode 100644 ovn/controller/gchassis.c
>> > create mode 100644 ovn/controller/gchassis.h
>> >
>> > diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
>> > index 8c6a787..d3828f5 100644
>> > --- a/ovn/controller/automake.mk
>> > +++ b/ovn/controller/automake.mk
>> > @@ -6,6 +6,8 @@ ovn_controller_ovn_controller_SOURCES = \
>> > ovn/controller/chassis.h \
>> > ovn/controller/encaps.c \
>> > ovn/controller/encaps.h \
>> > + ovn/controller/gchassis.c \
>> > + ovn/controller/gchassis.h \
>> > ovn/controller/lflow.c \
>> > ovn/controller/lflow.h \
>> > ovn/controller/lport.c \
>> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
>> > index bb76608..f5526fd 100644
>> > --- a/ovn/controller/binding.c
>> > +++ b/ovn/controller/binding.c
>> > @@ -15,6 +15,7 @@
>> >
>> > #include <config.h>
>> > #include "binding.h"
>> > +#include "gchassis.h"
>> > #include "lflow.h"
>> > #include "lport.h"
>> >
>> > @@ -26,6 +27,7 @@
>> > #include "lib/vswitch-idl.h"
>> > #include "openvswitch/hmap.h"
>> > #include "openvswitch/vlog.h"
>> > +#include "ovn/lib/chassis-index.h"
>> > #include "ovn/lib/ovn-sb-idl.h"
>> > #include "ovn-controller.h"
>> >
>> > @@ -394,12 +396,15 @@ consider_local_datapath(struct controller_ctx
>> > *ctx,
>> > false, local_datapaths);
>> > }
>> > } else if (!strcmp(binding_rec->type, "chassisredirect")) {
>> > - const char *chassis_id = smap_get(&binding_rec->options,
>> > - "redirect-chassis");
>> > - our_chassis = chassis_id && !strcmp(chassis_id,
>> > chassis_rec->name);
>> > - if (our_chassis) {
>> > + if (gateway_chassis_in_pb_contains(binding_rec, chassis_rec)) {
>> > add_local_datapath(ldatapaths, lports,
>> > binding_rec->datapath,
>> > false, local_datapaths);
>> > + /* XXX this should only be set to true if our chassis
>> > + * (chassis_rec) is the master for this chassisredirect
>> > port
>> > + * but for now we'll bind it only when not bound, this is
>> > + * handled in subsequent patches */
>> > + our_chassis = !binding_rec->chassis ||
>> > + chassis_rec == binding_rec->chassis;
>> > }
>> > } else if (!strcmp(binding_rec->type, "l3gateway")) {
>> > const char *chassis_id = smap_get(&binding_rec->options,
>> > diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
>> > index 3bfa7d1..136b3a7 100644
>> > --- a/ovn/controller/binding.h
>> > +++ b/ovn/controller/binding.h
>> > @@ -20,6 +20,7 @@
>> > #include <stdbool.h>
>> >
>> > struct controller_ctx;
>> > +struct chassis_index;
>> > struct hmap;
>> > struct ldatapath_index;
>> > struct lport_index;
>> > diff --git a/ovn/controller/gchassis.c b/ovn/controller/gchassis.c
>> > new file mode 100644
>> > index 0000000..27d8f66
>> > --- /dev/null
>> > +++ b/ovn/controller/gchassis.c
>> > @@ -0,0 +1,176 @@
>> > +/* Copyright (c) 2017, Red Hat, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + * http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> > implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +#include <config.h>
>> > +
>> > +#include "gchassis.h"
>> > +#include "lport.h"
>> > +#include "openvswitch/vlog.h"
>> > +#include "ovn/lib/chassis-index.h"
>> > +#include "ovn/lib/ovn-sb-idl.h"
>> > +
>> > +VLOG_DEFINE_THIS_MODULE(gchassis);
>> > +
>> > +/* gateway_chassis ordering
>> > + */
>> > +static int
>> > +compare_chassis_prio_(const void *a_, const void *b_)
>> > +{
>> > + const struct gateway_chassis *gc_a = a_;
>> > + const struct gateway_chassis *gc_b = b_;
>> > + int prio_diff = gc_b->db->priority - gc_a->db->priority;
>> > + if (!prio_diff) {
>> > + return strcmp(gc_a->db->name, gc_b->db->name);
>> > + }
>> > + return prio_diff;
>> > +}
>> > +
>> > +struct ovs_list*
>> > +gateway_chassis_get_ordered(const struct sbrec_port_binding *binding,
>> > + const struct chassis_index *chassis_index)
>> > +{
>> > + const char *redir_chassis_str;
>> > + const struct sbrec_chassis *redirect_chassis = NULL;
>> > +
>> > + /* XXX: redirect-chassis SBDB option handling is supported for
>> > backwards
>> > + * compatibility with N-1 version of ovn-northd, this support can
>> > + * be removed in OVS 2.9 where Gateway_Chassis list on the port
>> > binding
>> > + * will allways be populated by northd */
>> > + redir_chassis_str = smap_get(&binding->options,
>> > "redirect-chassis");
>> > +
>> > + if (redir_chassis_str) {
>> > + redirect_chassis = chassis_lookup_by_name(chassis_index,
>> > + redir_chassis_str);
>> > + if (!redirect_chassis) {
>> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
>> > 1);
>> > + VLOG_WARN_RL(&rl, "chassis name (%s) in redirect-chassis
>> > option "
>> > + "of logical port %s not known",
>> > + redir_chassis_str,
>> > binding->logical_port);
>> > + }
>> > + }
>> > +
>> > + if (!redirect_chassis && binding->n_gateway_chassis == 0) {
>> > + return NULL;
>> > + }
>> > +
>> > + struct gateway_chassis *gateway_chassis = NULL;
>> > + int n=0;
>> > +
>> > + if (binding->n_gateway_chassis) {
>> > + gateway_chassis = xmalloc(sizeof *gateway_chassis *
>> > + binding->n_gateway_chassis);
>> > + for (n=0; n < binding->n_gateway_chassis; n++) {
>> > + gateway_chassis[n].db = binding->gateway_chassis[n];
>> > + gateway_chassis[n].virtual_gwc = false;
>> > + }
>> > + qsort(gateway_chassis, n, sizeof *gateway_chassis,
>> > + compare_chassis_prio_);
>> > + } else if (redirect_chassis) {
>> > + /* When only redirect_chassis is available, return a single
>> > + * virtual entry that it's not on OVSDB, this way the code
>> > + * handling the returned list will be uniform, regardless
>> > + * of gateway_chassis being populated or redirect-chassis
>> > option
>> > + * being used */
>> > + gateway_chassis = xmalloc(sizeof *gateway_chassis);
>> > + struct sbrec_gateway_chassis *gwc =
>> > + xzalloc(sizeof *gateway_chassis->db);
>> > + sbrec_gateway_chassis_init(gwc);
>> > + gwc->name = xasprintf("%s_%s", binding->logical_port,
>> > + redirect_chassis->name);
>> > + gwc->chassis = (struct sbrec_chassis *)redirect_chassis;
>> > + gateway_chassis->db = gwc;
>> > + gateway_chassis->virtual_gwc = true;
>> > + n++;
>> > + }
>> > +
>> > + struct ovs_list *list = NULL;
>> > + if (n) {
>> > + list = xmalloc(sizeof *list);
>> > + ovs_list_init(list);
>> > +
>> > + int i;
>> > + for (i=0; i<n; i++) {
>> > + ovs_list_push_back(list, &gateway_chassis[i].node);
>> > + }
>> > + }
>> > +
>> > + return list;
>> > +}
>> > +
>> > +bool
>> > +gateway_chassis_contains(struct ovs_list *gateway_chassis,
>> > + const struct sbrec_chassis *chassis) {
>> > + struct gateway_chassis *chassis_item;
>> > + if (gateway_chassis) {
>> > + LIST_FOR_EACH (chassis_item, node, gateway_chassis) {
>> > + if (chassis_item->db->chassis &&
>> > + !strcmp(chassis_item->db->chassis->name,
>> > chassis->name)) {
>> > + return true;
>> > + }
>> > + }
>> > + }
>> > + return false;
>> > +}
>> > +
>> > +void
>> > +gateway_chassis_destroy(struct ovs_list *list)
>> > +{
>> > + if (!list) {
>> > + return;
>> > + }
>> > +
>> > + /* XXX: This loop is for backwards compatibility with
>> > redirect-chassis
>> > + * which we insert as a single virtual Gateway_Chassis on the
>> > ordered
>> > + * list */
>> > + struct gateway_chassis *chassis_item;
>> > + LIST_FOR_EACH (chassis_item, node, list) {
>> > + if (chassis_item->virtual_gwc) {
>> > + free(chassis_item->db->name);
>> > + free((void *)chassis_item->db);
>> > + }
>> > + }
>> > +
>> > + free(ovs_list_front(list));
>> > + free(list);
>> > +}
>> > +
>> > +bool
>> > +gateway_chassis_in_pb_contains(const struct sbrec_port_binding
>> > *binding,
>> > + const struct sbrec_chassis *chassis)
>> > +{
>> > + if (!binding || !chassis) {
>> > + return false;
>> > + }
>> > +
>> > + /* XXX: redirect-chassis handling for backwards compatibility,
>> > + * with older ovs-northd during upgrade phase, can be removed
>>
>> s/ovs-northd/ovn-northd/
>>
>> > + * for OVS 2.9 */
>>
>> I think we can remove this, because I'm not aware of anyone using the
>> southbound db directly. It's possible that you could use
>> ovn-controller and the southbound db only though, so we'll we have to
>> watch out for people starting to use it that way. If that starts
>> happening, we'll have to be more strict about removing things from the
>> southbound db. Just FYI. :-)
>
>
> I believe we can't remove it yet. This is there because ovn-controller uses
> the SBDB directly, and since the order we recommend for upgrades is:
>
> 1st) update ovn-controller
> 2nd) update ovn-northd
>
> For during the upgrade process ovn-controller still depends on understanding
> the old semantics of the SBDB.
>
> Then on next version we can drop that support from ovn-controller.
>
>
> Does that make sense, or am I missing something?
You are right. Thanks. :-)
--
Russell Bryant
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
