[ovs-dev] [PATCH monitor_cond V10] RFC OVN: Implementation of conditional monitoring usage
Mickey Spiegel
emspiege at us.ibm.com
Wed Jul 20 05:53:42 UTC 2016
Comments inline as <Mickey>
-----"dev" <dev-bounces at openvswitch.org> wrote: -----
To: Ben Pfaff <blp at ovn.org>
From: Liran Schour
Sent by: "dev"
Date: 07/19/2016 01:45AM
Cc: dev at 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?
<Mickey> Do you have any information about the latency involved in this process?
<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 <lirans at 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.
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.
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?
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?
+{
+ 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?
static void
@@ -360,7 +363,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
existing_ports);
free(dst_name);
free(src_name);
- add_patched_datapath(patched_datapaths, binding, local_port);
+ add_patched_datapath(patched_datapaths, binding, local_port, ctx);
if (local_port) {
if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) {
sbrec_port_binding_set_chassis(binding, chassis_rec);
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index a2349a4..f2962eb 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -92,11 +92,14 @@ AT_CHECK([ovn-sbctl \
-- --id=@dp2 create Datapath_Binding tunnel_key=2 \
-- create Port_Binding datapath=@dp1 logical_port=foo tunnel_key=1 type=patch options:peer=bar \
-- create Port_Binding datapath=@dp2 logical_port=bar tunnel_key=2 type=patch options:peer=foo \
+ -- create Port_Binding datapath=@dp1 logical_port=localvif3 tunnel_key=3 \
| ${PERL} $srcdir/uuidfilt.pl], [0], [<0>
<1>
<2>
<3>
+<4>
])
+ovs-vsctl add-port br-int localvif3 -- set Interface localvif3 external_ids:iface-id=localvif3
check_patches \
'br-int patch-br-int-to-localnet2 patch-localnet2-to-br-int' \
'br-eth0 patch-localnet2-to-br-int patch-br-int-to-localnet2' \
diff --git a/tests/ovn.at b/tests/ovn.at
index 95aa822..17512b5 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1356,6 +1356,7 @@ sim_add hv_gw
as hv_gw
ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.3
+ovs-vsctl add-port br-int viflp-gw -- set Interface viflp-gw external-ids:iface-id=lp-gw
ovs-vsctl add-br br-phys2
net_attach n2 br-phys2
ovs-vsctl set open . external_ids:ovn-bridge-mappings="physnet1:br-phys2"
--
2.1.4
_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list