Mickey Spiegel/San Jose/IBM wrote on 20/07/2016 08:53:42 AM:
> From: Mickey Spiegel/San Jose/IBM
> To: Liran Schour/Haifa/IBM@IBMIL
> Cc: Ben Pfaff <b...@ovn.org>, dev@openvswitch.org
> Date: 20/07/2016 08:53 AM
> Subject: Re: [ovs-dev] [PATCH monitor_cond V10] RFC OVN:
> Implementation of conditional monitoring usage
>
> Comments inline as <Mickey>
>
> -----"dev" <dev-boun...@openvswitch.org> wrote: -----
> To: Ben Pfaff <b...@ovn.org>
> From: Liran Schour
> Sent by: "dev"
> Date: 07/19/2016 01:45AM
> Cc: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH monitor_cond V10] RFC OVN: Implementation
> of conditional monitoring usage
> Conditional monitor of: Port_Binding, Logical_Flow, Multicast_Group
> MAC_Binding tables. As a result ovn-controller will be notified only
about
> records belongs to a datapath that is being served by this hypervisor.
>
> Performance evaluation:
> OVN is the main candidate for conditional monitoring usage. It is clear
that
> conditional monitoring reduces computation on the ovn-controller
(client) side
> due to the reduced size of flow tables and update messages. Performance
> evaluation shows up to 75% computation reduction.
> However, performance evaluation shows also a reduction in
> computation on the SB
> ovsdb-server side proportional to the degree that each logical network
is
> spread over physical hosts in the DC. Evaluation shows that in a
realistic
> scenarios there is a computation reduction also in the server side.
>
> <Mickey> Before getting into details, please confirm whether I have
> a proper understanding how this works:
> When a hypervisor first comes up, it gets no port bindings.
> When a VIF comes up on the hypervisor, this will trigger filter_port
> on that VIF, which causes the port binding for that VIF to come down
> to the controller.
> Upon receipt of that port binding, the controller will add the
> datapath for the VIF's logical switch to local datapaths, which will
> trigger filter_datapath.
> This will cause all of the port bindings, MAC bindings, logical
> flows, and multicast groups for the VIF's logical switch to come
> down to the controller.
> Following the patch ports from the VIF's logical switch, this will
> trigger filter_port for all peer patch ports.
> The port bindings for those peer patch ports will trigger
> filter_datapath for all logical router datapaths connected to the
> logical switch.
> This will cause all of the port bindings, MAC bindings, logical
> flows, and multicast groups for those datapaths to come down to the
> controller.
> If those datapaths have patch ports to additional datapaths,
> following the patch ports, eventually all of the SB info for all
> connected datapaths will come down to the controller.
> Is this correct?
>
Yes. One small addition: to minimize the number of filters, the code will
unfilter_lport when the lport is already included in a flittered datapath.
> <Mickey> Do you have any information about the latency involved in
> this process?
>
No, I did not test it. I will write it in my TODO list.
> <Mickey> Further comments on the details inline.
>
> Evaluation on simulated environment of 50 hosts and 1000 logical ports
shows
> the following results (cycles #):
>
> LN spread over # hosts| master | patch | change
> -------------------------------------------------------------
> 1 | 24597200127 | 24339235374 | 1.0%
> 6 | 23788521572 | 19145229352 | 19.5%
> 12 | 23886405758 | 17913143176 | 25.0%
> 18 | 25812686279 | 23675094540 | 8.2%
> 24 | 28414671499 | 24770202308 | 12.8%
> 30 | 31487218890 | 28397543436 | 9.8%
> 36 | 36116993930 | 34105388739 | 5.5%
> 42 | 37898342465 | 38647139083 | -1.9%
> 48 | 41637996229 | 41846616306 | -0.5%
> 50 | 41679995357 | 43455565977 | -4.2%
>
> Signed-off-by: Liran Schour <lir...@il.ibm.com>
> ---
> ovn/controller/automake.mk | 4 +-
> ovn/controller/binding.c | 48 +++++++---
> ovn/controller/binding.h | 4 +-
> ovn/controller/filter.c | 207 +++++++++++++++++++++++++++++
> +++++++++++
> ovn/controller/filter.h | 36 +++++++
> ovn/controller/ovn-controller.c | 14 ++-
> ovn/controller/ovn-controller.h | 1 +
> ovn/controller/patch.c | 7 +-
> tests/ovn-controller.at | 3 +
> tests/ovn.at | 1 +
> 10 files changed, 305 insertions(+), 20 deletions(-)
> create mode 100644 ovn/controller/filter.c
> create mode 100644 ovn/controller/filter.h
>
> diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
> index cf57bbd..0318654 100644
> --- a/ovn/controller/automake.mk
> +++ b/ovn/controller/automake.mk
> @@ -19,7 +19,9 @@ ovn_controller_ovn_controller_SOURCES = \
> ovn/controller/ovn-controller.c \
> ovn/controller/ovn-controller.h \
> ovn/controller/physical.c \
> - ovn/controller/physical.h
> + ovn/controller/physical.h \
> + ovn/controller/filter.c \
> + ovn/controller/filter.h
> ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la
lib/libopenvswitch.la
> man_MANS += ovn/controller/ovn-controller.8
> EXTRA_DIST += ovn/controller/ovn-controller.8.xml
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index bc6df32..dcc2a2f 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -25,6 +25,8 @@
> #include "openvswitch/vlog.h"
> #include "ovn/lib/ovn-sb-idl.h"
> #include "ovn-controller.h"
> +#include "lport.h"
> +#include "filter.h"
>
> VLOG_DEFINE_THIS_MODULE(binding);
>
> @@ -64,7 +66,9 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>
> static bool
> get_local_iface_ids(const struct ovsrec_bridge *br_int,
> - struct shash *lport_to_iface)
> + struct shash *lport_to_iface,
> + struct lport_index *lports_index,
> + struct controller_ctx *ctx)
> {
> int i;
> bool changed = false;
> @@ -90,6 +94,9 @@ get_local_iface_ids(const struct ovsrec_bridge
*br_int,
> continue;
> }
> shash_add(lport_to_iface, iface_id, iface_rec);
> + if (!lport_lookup_by_name(lports_index, iface_id)) {
> + filter_lport(ctx, iface_id);
> + }
> if (!sset_find_and_delete(&old_local_ids, iface_id)) {
> sset_add(&local_ids, iface_id);
> changed = true;
> @@ -121,8 +128,11 @@ local_datapath_lookup_by_uuid(struct hmap
> *hmap_p, const struct uuid *uuid)
> }
>
> static void
> -remove_local_datapath(struct hmap *local_datapaths, struct
> local_datapath *ld)
> +remove_local_datapath(struct controller_ctx *ctx, struct hmap
> *local_datapaths,
> + struct local_datapath *ld)
> {
> + unfilter_datapath(ctx, ld->tunnel_key);
> +
> <Mickey> You have two different triggers for filter_datapath: One
> from add_local_datapath, another from add_patched_datapath in
> patch.c. If you call unfilter_datapath here, you might be blowing
> away the trigger from add_patched_datapath as well. It will not
> correct itself until another VIF, L2 gateway, L3 gateway on that
> datapath is added, or a patch port to that datapath is added.
>
You are right. I will fix this.
> if (ld->logical_port) {
> free(ld->logical_port);
> ld->logical_port = NULL;
> @@ -133,7 +143,8 @@ remove_local_datapath(struct hmap
> *local_datapaths, struct local_datapath *ld)
>
> static void
> add_local_datapath(struct hmap *local_datapaths,
> - const struct sbrec_port_binding *binding_rec)
> + const struct sbrec_port_binding *binding_rec,
> + struct controller_ctx *ctx)
> {
> if (get_local_datapath(local_datapaths,
> binding_rec->datapath->tunnel_key)) {
> @@ -143,8 +154,10 @@ add_local_datapath(struct hmap *local_datapaths,
> struct local_datapath *ld = xzalloc(sizeof *ld);
> ld->logical_port = xstrdup(binding_rec->logical_port);
> memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
> + ld->tunnel_key = binding_rec->datapath->tunnel_key;
> hmap_insert(local_datapaths, &ld->hmap_node,
> binding_rec->datapath->tunnel_key);
> + filter_datapath(ctx, binding_rec);
> }
> <Mickey> Just noting that this should take care of L3 gateway as
> well, once Chandra's "ovn: Add datapth of gateway port to
local_datapaths
> " patch lands. The addition of "ctx" to add_local_datapaths will be
> necessary, in whichever of the two patches lands later.
>
Right.
> static void
> @@ -160,6 +173,7 @@ update_qos(const struct ovsrec_interface *iface_rec,
>
> static void
> consider_local_datapath(struct controller_ctx *ctx,
> + struct lport_index *lports_index,
> const struct sbrec_chassis *chassis_rec,
> const struct sbrec_port_binding *binding_rec,
> struct hmap *local_datapaths,
> @@ -171,7 +185,7 @@ consider_local_datapath(struct controller_ctx *ctx,
> if (iface_rec
> || (binding_rec->parent_port && binding_rec->parent_port[0] &&
> sset_contains(&local_ids, binding_rec->parent_port))) {
> - add_local_datapath(local_datapaths, binding_rec);
> + add_local_datapath(local_datapaths, binding_rec, ctx);
> if (iface_rec && ctx->ovs_idl_txn) {
> update_qos(iface_rec, binding_rec);
> }
> @@ -210,7 +224,7 @@ consider_local_datapath(struct controller_ctx *ctx,
> VLOG_INFO("Claiming l2gateway port %s for this chassis.",
> binding_rec->logical_port);
> sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> - add_local_datapath(local_datapaths, binding_rec);
> + add_local_datapath(local_datapaths, binding_rec, ctx);
> }
> } else if (chassis_rec && binding_rec->chassis == chassis_rec
> && strcmp(binding_rec->type, "gateway")) {
> @@ -220,11 +234,18 @@ consider_local_datapath(struct controller_ctx
*ctx,
> sbrec_port_binding_set_chassis(binding_rec, NULL);
> }
> }
> + const char *peer = smap_get(&binding_rec->options, "peer");
> + if (peer) {
> + if (!lport_lookup_by_name(lports_index, peer)) {
> + filter_lport(ctx, peer);
> + }
> + }
> }
> <Mickey> If you follow patch ports here, where it applies for every
> port binding that is received, you might be following a patch port
> that does not apply to this chassis, such as a "gateway" patch port
> to an L3 gateway router that is not local to this chassis. It would
> be better to add this logic in patch.c, perhaps in
> add_logical_patch_ports around where create_patch_port is called?
>
Thanks, you are right.
> void
> binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge
*br_int,
> - const char *chassis_id, struct hmap *local_datapaths)
> + const char *chassis_id, struct lport_index *lports_index,
> + struct hmap *local_datapaths)
> {
> const struct sbrec_chassis *chassis_rec;
> const struct sbrec_port_binding *binding_rec;
> @@ -236,7 +257,8 @@ binding_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> }
>
> if (br_int) {
> - if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int,
> &lport_to_iface)) {
> + if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int,
> &lport_to_iface,
> + lports_index,
ctx)) {
> process_full_binding = true;
> }
> } else {
> @@ -252,8 +274,9 @@ binding_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> struct hmap keep_local_datapath_by_uuid =
> HMAP_INITIALIZER(&keep_local_datapath_by_uuid);
> SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> - consider_local_datapath(ctx, chassis_rec, binding_rec,
> - local_datapaths, &lport_to_iface);
> + consider_local_datapath(ctx, lports_index, chassis_rec,
> + binding_rec, local_datapaths,
> + &lport_to_iface);
> struct local_datapath *ld = xzalloc(sizeof *ld);
> memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof
ld->uuid);
> hmap_insert(&keep_local_datapath_by_uuid,
&ld->uuid_hmap_node,
> @@ -263,7 +286,7 @@ binding_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> HMAP_FOR_EACH_SAFE (old_ld, next, hmap_node, local_datapaths) {
> if
(!local_datapath_lookup_by_uuid(&keep_local_datapath_by_uuid,
> &old_ld->uuid)) {
> - remove_local_datapath(local_datapaths, old_ld);
> + remove_local_datapath(ctx, local_datapaths, old_ld);
> }
> }
> hmap_destroy(&keep_local_datapath_by_uuid);
> @@ -280,8 +303,9 @@ binding_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> poll_immediate_wake();
> }
> } else {
> - consider_local_datapath(ctx, chassis_rec, binding_rec,
> - local_datapaths,
&lport_to_iface);
> + consider_local_datapath(ctx, lports_index, chassis_rec,
> + binding_rec, local_datapaths,
> + &lport_to_iface);
> }
> }
> }
> diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
> index 8753d44..11e0c0f 100644
> --- a/ovn/controller/binding.h
> +++ b/ovn/controller/binding.h
> @@ -25,11 +25,13 @@ struct ovsdb_idl;
> struct ovsrec_bridge;
> struct simap;
> struct sset;
> +struct lport_index;
>
> void binding_register_ovs_idl(struct ovsdb_idl *);
> void binding_reset_processing(void);
> void binding_run(struct controller_ctx *, const struct ovsrec_bridge
*br_int,
> - const char *chassis_id, struct hmap *local_datapaths);
> + const char *chassis_id, struct lport_index
*lports_index,
> + struct hmap *local_datapaths);
> bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
>
> #endif /* ovn/binding.h */
> diff --git a/ovn/controller/filter.c b/ovn/controller/filter.c
> new file mode 100644
> index 0000000..cc25737
> --- /dev/null
> +++ b/ovn/controller/filter.c
> @@ -0,0 +1,207 @@
> +/* Copyright (c) 2015, 2016 Nicira, 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 "filter.h"
> +
> +#include "openvswitch/vlog.h"
> +#include "ovn/lib/ovn-sb-idl.h"
> +#include "ovn-controller.h"
> +#include "lport.h"
> +
> +VLOG_DEFINE_THIS_MODULE(filter);
> +
> +static struct hmap filtered_dps = HMAP_INITIALIZER(&filtered_dps);
> +static struct hmap filtered_lps = HMAP_INITIALIZER(&filtered_lps);
> +
> +struct filtered_dp {
> + struct hmap_node hmap_node;
> + int64_t tunnel_key;
> + struct sbrec_datapath_binding *datapath;
> +};
> +
> +struct filtered_lp {
> + struct hmap_node hmap_node;
> + const char *lport_name;
> + bool used;
> +};
> +
> +void
> +filter_init(struct ovsdb_idl *idl)
> +{
> + sbrec_port_binding_add_clause_false(idl);
> + sbrec_mac_binding_add_clause_false(idl);
> + sbrec_logical_flow_add_clause_false(idl);
> + sbrec_multicast_group_add_clause_false(idl);
> +}
> +
> +void
> +filter_mark_unused(void)
> +{
> + struct filtered_lp *lp;
> +
> + HMAP_FOR_EACH(lp, hmap_node, &filtered_lps) {
> + lp->used = false;
> + }
> +}
> +
> +void
> +filter_clear(struct ovsdb_idl *idl)
> +{
> + struct filtered_lp *lp, *next;
> + struct filtered_lp *dp, *next_dp;
> +
> + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) {
> + hmap_remove(&filtered_lps, &lp->hmap_node);
> + free(lp);
> + }
> + HMAP_FOR_EACH_SAFE(dp, next_dp, hmap_node, &filtered_dps) {
> + hmap_remove(&filtered_dps, &dp->hmap_node);
> + free(dp);
> + }
> +
> + ovsdb_idl_condition_reset(idl,
> + &sbrec_table_port_binding);
> + ovsdb_idl_condition_reset(idl,
> + &sbrec_table_logical_flow);
> + ovsdb_idl_condition_reset(idl,
> + &sbrec_table_mac_binding);
> + ovsdb_idl_condition_reset(idl,
> + &sbrec_table_multicast_group);
> +}
> +
> +static struct filtered_dp *
> +lookup_dp_by_key(int64_t tunnel_key)
> +{
> + struct filtered_dp *dp;
> +
> + HMAP_FOR_EACH_WITH_HASH(dp, hmap_node, tunnel_key,
> + &filtered_dps) {
> + if (dp->tunnel_key == tunnel_key) {
> + return dp;
> + }
> + }
> + return NULL;
> +}
> +
> +void
> +filter_remove_unused_lports(struct controller_ctx *ctx,
> + const struct lport_index *lports_index)
> +{
> + struct filtered_lp *lp, *next;
> +
> + HMAP_FOR_EACH_SAFE(lp, next, hmap_node, &filtered_lps) {
> + if (!lp->used) {
> + const struct sbrec_port_binding *pb =
> + lport_lookup_by_name(lports_index, lp->lport_name);
> + if (!pb) {
> + continue;
> + }
> + if (lookup_dp_by_key(pb->datapath->tunnel_key)) {
> + VLOG_INFO("Unfilter Port %s", lp->lport_name);
> + sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> +
> lp->lport_name);
> + hmap_remove(&filtered_lps, &lp->hmap_node);
> + free(lp);
> + }
> + }
> + }
> +}
> +
> +void
> +filter_lport(struct controller_ctx *ctx, const char *lport_name)
> +{
> + struct filtered_lp *lp;
> + size_t hash = hash_bytes(lport_name, strlen(lport_name), 0);
> +
> + HMAP_FOR_EACH_WITH_HASH(lp, hmap_node, hash, &filtered_lps) {
> + if (!strcmp(lp->lport_name, lport_name)) {
> + lp->used = true;
> + return;
> + }
> + }
> +
> + VLOG_INFO("Filter Port %s", lport_name);
> +
> + sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + lport_name);
> +
> + char *name = xmalloc(strlen(lport_name));
> + lp = xmalloc(sizeof *lp);
> +
> + strcpy(name, lport_name);
> + lp->lport_name = name;
> + lp->used = true;
> + hmap_insert(&filtered_lps, &lp->hmap_node,
> + hash);
> +}
> +
> +void
> +filter_datapath(struct controller_ctx *ctx,
> + const struct sbrec_port_binding *pb)
> <Mickey> Since "pb" is only used as "pb->datapath", wondering if it
> would be cleaner to change the second argument to
> sbrec_datapath_binding instead?
>
Will change it to sbrec_datapath_binding *.
> +{
> + struct filtered_dp *dp;
> +
> + dp = lookup_dp_by_key(pb->datapath->tunnel_key);
> + if (dp) {
> + return;
> + }
> +
> + VLOG_INFO("Filter DP "UUID_FMT,
UUID_ARGS(&pb->datapath->header_.uuid));
> + sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + pb->datapath);
> + sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + pb->datapath);
> + sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + pb->datapath);
> + sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + pb->datapath);
> +
> + dp = xmalloc(sizeof *dp);
> + dp->tunnel_key = pb->datapath->tunnel_key;
> + dp->datapath = pb->datapath;
> + hmap_insert(&filtered_dps, &dp->hmap_node,
pb->datapath->tunnel_key);
> +}
> +
> +void
> +unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key)
> +{
> + struct filtered_dp *dp = lookup_dp_by_key(tunnel_key);
> +
> + if (dp) {
> + VLOG_INFO("Unfilter DP "UUID_FMT,
> + UUID_ARGS(&dp->datapath->header_.uuid));
> + sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + dp->datapath);
> + sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + dp->datapath);
> + sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + dp->datapath);
> + sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl,
> + OVSDB_F_EQ,
> + dp->datapath);
> + hmap_remove(&filtered_dps, &dp->hmap_node);
> + free(dp);
> + }
> +}
> diff --git a/ovn/controller/filter.h b/ovn/controller/filter.h
> new file mode 100644
> index 0000000..203689a
> --- /dev/null
> +++ b/ovn/controller/filter.h
> @@ -0,0 +1,36 @@
> +/* Copyright (c) 2015 Nicira, 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.
> + */
> +
> +#ifndef OVN_FILTER_H
> +#define OVN_FILTER_H 1
> +
> +#include <stdint.h>
> +
> +struct controller_ctx;
> +struct ovsdb_idl;
> +struct sbrec_port_binding;
> +struct lport_index;
> +
> +void filter_init(struct ovsdb_idl *idl);
> +void filter_clear(struct ovsdb_idl *idl);
> +void filter_mark_unused(void);
> +void filter_remove_unused_lports(struct controller_ctx *ctx,
> + const struct lport_index *lports);
> +void filter_lport(struct controller_ctx *ctx, const char *lport_name);
> +void filter_datapath(struct controller_ctx *ctx,
> + const struct sbrec_port_binding *pb);
> +void unfilter_datapath(struct controller_ctx *ctx, int64_t tunnel_key);
> +
> +#endif /* ovn/controller/filter.h */
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index 04684b2..2ad8523 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -54,6 +54,7 @@
> #include "stream.h"
> #include "unixctl.h"
> #include "util.h"
> +#include "filter.h"
>
> VLOG_DEFINE_THIS_MODULE(main);
>
> @@ -379,6 +380,8 @@ main(int argc, char *argv[])
> struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
>
> + filter_init(ovnsb_idl_loop.idl);
> +
> /* Track the southbound idl. */
> ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
>
> @@ -404,6 +407,7 @@ main(int argc, char *argv[])
> binding_reset_processing();
> lport_index_clear(&lports);
> mcgroup_index_clear(&mcgroups);
> + filter_clear(ovnsb_idl_loop.idl);
> } else {
> free(new_ovnsb_remote);
> }
> @@ -422,19 +426,20 @@ main(int argc, char *argv[])
> const struct ovsrec_bridge *br_int = get_br_int(&ctx);
> const char *chassis_id = get_chassis_id(ctx.ovs_idl);
>
> + lport_index_fill(&lports, ctx.ovnsb_idl);
> + mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl);
> + filter_mark_unused();
> +
> if (chassis_id) {
> chassis_run(&ctx, chassis_id);
> encaps_run(&ctx, br_int, chassis_id);
> - binding_run(&ctx, br_int, chassis_id, &local_datapaths);
> + binding_run(&ctx, br_int, chassis_id, &lports,
&local_datapaths);
> }
>
> if (br_int && chassis_id) {
> patch_run(&ctx, br_int, chassis_id, &local_datapaths,
> &patched_datapaths);
>
> - lport_index_fill(&lports, ctx.ovnsb_idl);
> - mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl);
> -
> enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
>
> pinctrl_run(&ctx, &lports, br_int, chassis_id,
&local_datapaths);
> @@ -452,6 +457,7 @@ main(int argc, char *argv[])
> ofctrl_put(&group_table);
> }
>
> + filter_remove_unused_lports(&ctx, &lports);
> sset_destroy(&all_lports);
>
> unixctl_server_run(unixctl);
> diff --git a/ovn/controller/ovn-controller.h
b/ovn/controller/ovn-controller.h
> index 470b1f5..3469fe5 100644
> --- a/ovn/controller/ovn-controller.h
> +++ b/ovn/controller/ovn-controller.h
> @@ -40,6 +40,7 @@ struct local_datapath {
> struct hmap_node hmap_node;
> struct hmap_node uuid_hmap_node;
> struct uuid uuid;
> + int64_t tunnel_key;
> char *logical_port;
> const struct sbrec_port_binding *localnet_port;
> };
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index 52d9e8d..438dff5 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -22,6 +22,7 @@
> #include "lib/vswitch-idl.h"
> #include "openvswitch/vlog.h"
> #include "ovn-controller.h"
> +#include "filter.h"
>
> VLOG_DEFINE_THIS_MODULE(patch);
>
> @@ -250,7 +251,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
>
> static void
> add_patched_datapath(struct hmap *patched_datapaths,
> - const struct sbrec_port_binding *binding_rec,
> bool local)
> + const struct sbrec_port_binding *binding_rec,
> bool local,
> + struct controller_ctx *ctx)
> {
> struct patched_datapath *pd =
get_patched_datapath(patched_datapaths,
> binding_rec->datapath->tunnel_key);
> @@ -268,6 +270,7 @@ add_patched_datapath(struct hmap *patched_datapaths,
> /* stale is set to false. */
> hmap_insert(patched_datapaths, &pd->hmap_node,
> binding_rec->datapath->tunnel_key);
> + filter_datapath(ctx, binding_rec);
> }
> <Mickey> Should unfilter_datapath be called somewhere around line
> 297? With a check against local_datapaths as well?
>
Right, will fix that.
Thanks for the review,
- Liran
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
