[ovs-dev] [PATCH v3] ovn-controller: Persist desired conntrack groups.
Ryan Moats
rmoats at us.ibm.com
Thu Jul 28 22:17:41 UTC 2016
With incremental processing of logical flows desired conntrack groups
are not being persisted. This patch adds this capability, with the
side effect of adding a ds_clone method that this capability leverages.
Signed-off-by: Ryan Moats <rmoats at us.ibm.com>
Reported-by: Guru Shetty <guru at ovn.org>
Reported-at: http://openvswitch.org/pipermail/dev/2016-July/076320.html
Fixes: 70c7cfe ("ovn-controller: Add incremental processing to lflow_run and physical_run")
---
include/openvswitch/dynamic-string.h | 1 +
include/ovn/actions.h | 6 +++++
lib/dynamic-string.c | 9 ++++++++
ovn/controller/lflow.c | 2 ++
ovn/controller/ofctrl.c | 43 ++++++++++++++++++++++++------------
ovn/controller/ofctrl.h | 5 ++++-
ovn/controller/ovn-controller.c | 2 +-
ovn/lib/actions.c | 1 +
8 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/include/openvswitch/dynamic-string.h b/include/openvswitch/dynamic-string.h
index dfe2688..bf1f64a 100644
--- a/include/openvswitch/dynamic-string.h
+++ b/include/openvswitch/dynamic-string.h
@@ -73,6 +73,7 @@ void ds_swap(struct ds *, struct ds *);
int ds_last(const struct ds *);
bool ds_chomp(struct ds *, int c);
+void ds_clone(struct ds *, struct ds *);
/* Inline functions. */
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 114c71e..55720ce 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -22,7 +22,9 @@
#include "compiler.h"
#include "openvswitch/hmap.h"
#include "openvswitch/dynamic-string.h"
+#include "openvswitch/uuid.h"
#include "util.h"
+#include "uuid.h"
struct expr;
struct lexer;
@@ -43,6 +45,7 @@ struct group_table {
struct group_info {
struct hmap_node hmap_node;
struct ds group;
+ struct uuid lflow_uuid;
uint32_t group_id;
};
@@ -107,6 +110,9 @@ struct action_params {
/* A struct to figure out the group_id for group actions. */
struct group_table *group_table;
+ /* The logical flow uuid that drove this action. */
+ struct uuid lflow_uuid;
+
/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
* OpenFlow flow table (ptable). A number of parameters describe this
* mapping and data related to flow tables:
diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
index 1f17a9f..6f7b610 100644
--- a/lib/dynamic-string.c
+++ b/lib/dynamic-string.c
@@ -456,3 +456,12 @@ ds_chomp(struct ds *ds, int c)
return false;
}
}
+
+void
+ds_clone(struct ds *dst, struct ds *source)
+{
+ dst->length = source->length;
+ dst->allocated = dst->length;
+ dst->string = xmalloc(dst->allocated + 1);
+ memcpy(dst->string, source->string, dst->allocated + 1);
+}
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index a4f3322..e38c32a 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -383,6 +383,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
if (full_flow_processing) {
ovn_flow_table_clear();
+ ovn_group_table_clear(group_table, false);
full_logical_flow_processing = true;
full_neighbor_flow_processing = true;
full_flow_processing = false;
@@ -515,6 +516,7 @@ consider_logical_flow(const struct lport_index *lports,
.aux = &aux,
.ct_zones = ct_zones,
.group_table = group_table,
+ .lflow_uuid = lflow->header_.uuid,
.n_tables = LOG_PIPELINE_LEN,
.first_ptable = first_ptable,
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index dd9f5ec..54bea99 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -140,8 +140,6 @@ static ovs_be32 queue_msg(struct ofpbuf *);
static void ovn_flow_table_destroy(void);
static struct ofpbuf *encode_flow_mod(struct ofputil_flow_mod *);
-static void ovn_group_table_clear(struct group_table *group_table,
- bool existing);
static struct ofpbuf *encode_group_mod(const struct ofputil_group_mod *);
static void ofctrl_recv(const struct ofp_header *, enum ofptype);
@@ -150,12 +148,15 @@ static struct hmap match_flow_table = HMAP_INITIALIZER(&match_flow_table);
static struct hindex uuid_flow_table = HINDEX_INITIALIZER(&uuid_flow_table);
void
-ofctrl_init(void)
+ofctrl_init(struct group_table *group_table)
{
swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP13_VERSION);
tx_counter = rconn_packet_counter_create();
hmap_init(&installed_flows);
ovs_list_init(&flow_updates);
+ if (!groups) {
+ groups = group_table;
+ }
}
/* S_NEW, for a new connection.
@@ -680,6 +681,16 @@ ofctrl_remove_flows(const struct uuid *uuid)
ovn_flow_destroy(f);
}
}
+
+ /* Remove any group_info information created by this logical flow. */
+ struct group_info *g, *next_g;
+ HMAP_FOR_EACH_SAFE (g, next_g, hmap_node, &groups->desired_groups) {
+ if (uuid_equals(&g->lflow_uuid, uuid)) {
+ hmap_remove(&groups->desired_groups, &g->hmap_node);
+ ds_destroy(&g->group);
+ free(g);
+ }
+ }
}
/* Shortcut to remove all flows matching the supplied UUID and add this
@@ -833,6 +844,17 @@ add_flow_mod(struct ofputil_flow_mod *fm, struct ovs_list *msgs)
/* group_table. */
+static struct group_info *
+group_info_clone(struct group_info *source)
+{
+ struct group_info *clone = xmalloc(sizeof *clone);
+ ds_clone(&clone->group, &source->group);
+ clone->group_id = source->group_id;
+ clone->lflow_uuid = source->lflow_uuid;
+ clone->hmap_node.hash = source->hmap_node.hash;
+ return clone;
+}
+
/* Finds and returns a group_info in 'existing_groups' whose key is identical
* to 'target''s key, or NULL if there is none. */
static struct group_info *
@@ -851,7 +873,7 @@ ovn_group_lookup(struct hmap *exisiting_groups,
}
/* Clear either desired_groups or existing_groups in group_table. */
-static void
+void
ovn_group_table_clear(struct group_table *group_table, bool existing)
{
struct group_info *g, *next;
@@ -894,10 +916,6 @@ add_group_mod(const struct ofputil_group_mod *gm, struct ovs_list *msgs)
void
ofctrl_put(struct group_table *group_table, int64_t nb_cfg)
{
- if (!groups) {
- groups = group_table;
- }
-
/* The flow table can be updated if the connection to the switch is up and
* in the correct state and not backlogged with existing flow_mods. (Our
* criteria for being backlogged appear very conservative, but the socket
@@ -1066,13 +1084,10 @@ ofctrl_put(struct group_table *group_table, int64_t nb_cfg)
/* Move the contents of desired_groups to existing_groups. */
HMAP_FOR_EACH_SAFE(desired, next_group, hmap_node,
&group_table->desired_groups) {
- hmap_remove(&group_table->desired_groups, &desired->hmap_node);
if (!ovn_group_lookup(&group_table->existing_groups, desired)) {
- hmap_insert(&group_table->existing_groups, &desired->hmap_node,
- desired->hmap_node.hash);
- } else {
- ds_destroy(&desired->group);
- free(desired);
+ struct group_info *clone = group_info_clone(desired);
+ hmap_insert(&group_table->existing_groups, &clone->hmap_node,
+ clone->hmap_node.hash);
}
}
diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
index befae01..da4ebb4 100644
--- a/ovn/controller/ofctrl.h
+++ b/ovn/controller/ofctrl.h
@@ -30,7 +30,7 @@ struct ovsrec_bridge;
struct group_table;
/* Interface for OVN main loop. */
-void ofctrl_init(void);
+void ofctrl_init(struct group_table *group_table);
enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int);
void ofctrl_put(struct group_table *group_table, int64_t nb_cfg);
void ofctrl_wait(void);
@@ -54,4 +54,7 @@ void ofctrl_flow_table_clear(void);
void ovn_flow_table_clear(void);
+void ovn_group_table_clear(struct group_table *group_table,
+ bool existing);
+
#endif /* ovn/ofctrl.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 5c74186..3cdfcdf 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -347,7 +347,7 @@ main(int argc, char *argv[])
ovsrec_init();
sbrec_init();
- ofctrl_init();
+ ofctrl_init(&group_table);
pinctrl_init();
lflow_init();
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index fd5a867..aef5c75 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -761,6 +761,7 @@ parse_ct_lb_action(struct action_context *ctx)
group_info = xmalloc(sizeof *group_info);
group_info->group = ds;
group_info->group_id = group_id;
+ group_info->lflow_uuid = ctx->ap->lflow_uuid;
group_info->hmap_node.hash = hash;
hmap_insert(&ctx->ap->group_table->desired_groups,
--
1.9.1
More information about the dev
mailing list