On Thu, Jul 28, 2016 at 10:17:41PM +0000, Ryan Moats wrote:
> 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 <rmo...@us.ibm.com>
> Reported-by: Guru Shetty <g...@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")
I find the new treatment of the group table confusingly wishy-washy.
It's not clear why it has to be passed in to multiple functions if ofctrl
retains a pointer anyway. So I folded in the following (which includes
Flavio's suggestion), and applied this to master:
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index a588fef..fb2d6a9 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -24,7 +24,6 @@
#include "openvswitch/dynamic-string.h"
#include "openvswitch/uuid.h"
#include "util.h"
-#include "uuid.h"
struct expr;
struct lexer;
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index a90aada..9388cb8 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -154,9 +154,7 @@ ofctrl_init(struct group_table *group_table)
tx_counter = rconn_packet_counter_create();
hmap_init(&installed_flows);
ovs_list_init(&flow_updates);
- if (!groups) {
- groups = group_table;
- }
+ groups = group_table;
}
/* S_NEW, for a new connection.
@@ -903,18 +901,16 @@ add_group_mod(const struct ofputil_group_mod *gm, struct
ovs_list *msgs)
}
-/* Replaces the flow table on the switch, if possible, by the flows in added
+/* Replaces the flow table on the switch, if possible, by the flows added
* with ofctrl_add_flow().
*
- * Replaces the group table on the switch, if possible, by the groups in
- * 'group_table->desired_groups'. Regardless of whether the group table
- * is updated, this deletes all the groups from the
- * 'group_table->desired_groups' and frees them. (The hmap itself isn't
- * destroyed.)
+ * Replaces the group table on the switch, if possible, by the groups added to
+ * the group table. Regardless of whether the group table is updated, clears
+ * the gruop table.
*
* This should be called after ofctrl_run() within the main loop. */
void
-ofctrl_put(struct group_table *group_table, int64_t nb_cfg)
+ofctrl_put(int64_t nb_cfg)
{
/* 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
@@ -922,7 +918,7 @@ ofctrl_put(struct group_table *group_table, int64_t nb_cfg)
* between ovn-controller and OVS provides some buffering.) */
if (state != S_UPDATE_FLOWS
|| rconn_packet_counter_n_packets(tx_counter)) {
- ovn_group_table_clear(group_table, false);
+ ovn_group_table_clear(groups, false);
return;
}
@@ -932,8 +928,8 @@ ofctrl_put(struct group_table *group_table, int64_t nb_cfg)
/* Iterate through all the desired groups. If there are new ones,
* add them to the switch. */
struct group_info *desired;
- HMAP_FOR_EACH(desired, hmap_node, &group_table->desired_groups) {
- if (!ovn_group_lookup(&group_table->existing_groups, desired)) {
+ HMAP_FOR_EACH(desired, hmap_node, &groups->desired_groups) {
+ if (!ovn_group_lookup(&groups->existing_groups, desired)) {
/* Create and install new group. */
struct ofputil_group_mod gm;
enum ofputil_protocol usable_protocols;
@@ -1048,8 +1044,8 @@ ofctrl_put(struct group_table *group_table, int64_t
nb_cfg)
* are not needed delete them. */
struct group_info *installed, *next_group;
HMAP_FOR_EACH_SAFE(installed, next_group, hmap_node,
- &group_table->existing_groups) {
- if (!ovn_group_lookup(&group_table->desired_groups, installed)) {
+ &groups->existing_groups) {
+ if (!ovn_group_lookup(&groups->desired_groups, installed)) {
/* Delete the group. */
struct ofputil_group_mod gm;
enum ofputil_protocol usable_protocols;
@@ -1071,22 +1067,22 @@ ofctrl_put(struct group_table *group_table, int64_t
nb_cfg)
ds_destroy(&group_string);
ofputil_uninit_group_mod(&gm);
- /* Remove 'installed' from 'group_table->existing_groups' */
- hmap_remove(&group_table->existing_groups, &installed->hmap_node);
+ /* Remove 'installed' from 'groups->existing_groups' */
+ hmap_remove(&groups->existing_groups, &installed->hmap_node);
ds_destroy(&installed->group);
/* Dealloc group_id. */
- bitmap_set0(group_table->group_ids, installed->group_id);
+ bitmap_set0(groups->group_ids, installed->group_id);
free(installed);
}
}
/* Move the contents of desired_groups to existing_groups. */
HMAP_FOR_EACH_SAFE(desired, next_group, hmap_node,
- &group_table->desired_groups) {
- if (!ovn_group_lookup(&group_table->existing_groups, desired)) {
+ &groups->desired_groups) {
+ if (!ovn_group_lookup(&groups->existing_groups, desired)) {
struct group_info *clone = group_info_clone(desired);
- hmap_insert(&group_table->existing_groups, &clone->hmap_node,
+ hmap_insert(&groups->existing_groups, &clone->hmap_node,
clone->hmap_node.hash);
}
}
diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
index da4ebb4..d21a7fe 100644
--- a/ovn/controller/ofctrl.h
+++ b/ovn/controller/ofctrl.h
@@ -32,7 +32,7 @@ struct group_table;
/* Interface for OVN main loop. */
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_put(int64_t nb_cfg);
void ofctrl_wait(void);
void ofctrl_destroy(void);
int64_t ofctrl_get_cur_cfg(void);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index b2a602c..a2530a7 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -459,7 +459,7 @@ main(int argc, char *argv[])
br_int, chassis_id, &ct_zones,
&local_datapaths, &patched_datapaths);
- ofctrl_put(&group_table, get_nb_cfg(ctx.ovnsb_idl));
+ ofctrl_put(get_nb_cfg(ctx.ovnsb_idl));
if (ctx.ovnsb_idl_txn) {
int64_t cur_cfg = ofctrl_get_cur_cfg();
if (cur_cfg && cur_cfg != chassis->nb_cfg) {
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
