Re: [ovs-dev] [PATCH v6 13/17] ovn-controller: Incremental processing for port-group changes.

2019-05-17 Thread 0-day Robot
Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Comment with 'xxx' marker
#123 FILE: ovn/controller/ovn-controller.c:1561:
/* XXX: The change_tracked check may be added to inc-proc

Lines checked: 211, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v6 13/17] ovn-controller: Incremental processing for port-group changes.

2019-05-17 Thread Han Zhou
From: Han Zhou 

Signed-off-by: Han Zhou 
---
 ovn/controller/lflow.h  |   3 +-
 ovn/controller/ovn-controller.c | 112 ++--
 2 files changed, 99 insertions(+), 16 deletions(-)

diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index 2a75ac5..2b5016e 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -70,7 +70,8 @@ struct uuid;
 #define LOG_PIPELINE_LEN 24
 
 enum ref_type {
-REF_TYPE_ADDRSET
+REF_TYPE_ADDRSET,
+REF_TYPE_PORTGROUP
 };
 
 /* Maintains the relationship for a pair of named resource and
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 5ec9a5e..fa94879 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -357,6 +357,29 @@ port_groups_init(const struct sbrec_port_group_table 
*port_group_table,
 }
 
 static void
+port_groups_update(const struct sbrec_port_group_table *port_group_table,
+   struct shash *port_groups, struct sset *new,
+   struct sset *deleted, struct sset *updated)
+{
+const struct sbrec_port_group *pg;
+SBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (pg, port_group_table) {
+if (sbrec_port_group_is_deleted(pg)) {
+expr_const_sets_remove(port_groups, pg->name);
+sset_add(deleted, pg->name);
+} else {
+expr_const_sets_add(port_groups, pg->name,
+(const char *const *) pg->ports,
+pg->n_ports, false);
+if (sbrec_port_group_is_new(pg)) {
+sset_add(new, pg->name);
+} else {
+sset_add(updated, pg->name);
+}
+}
+}
+}
+
+static void
 update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
 {
 const struct ovsrec_ssl *ssl = ovsrec_ssl_table_first(ssl_table);
@@ -793,6 +816,30 @@ en_port_groups_run(struct engine_node *node)
 node->changed = true;
 }
 
+static bool
+port_groups_sb_port_group_handler(struct engine_node *node)
+{
+struct ed_type_port_groups *pg = (struct ed_type_port_groups *)node->data;
+
+sset_clear(>new);
+sset_clear(>deleted);
+sset_clear(>updated);
+
+struct sbrec_port_group_table *pg_table =
+(struct sbrec_port_group_table *)EN_OVSDB_GET(
+engine_get_input("SB_port_group", node));
+
+port_groups_update(pg_table, >port_groups, >new,
+ >deleted, >updated);
+
+node->changed = !sset_is_empty(>new) || !sset_is_empty(>deleted)
+|| !sset_is_empty(>updated);
+
+pg->change_tracked = true;
+node->changed = true;
+return true;
+}
+
 struct ed_type_runtime_data {
 /* Contains "struct local_datapath" nodes. */
 struct hmap local_datapaths;
@@ -1436,7 +1483,8 @@ flow_output_sb_multicast_group_handler(struct engine_node 
*node)
 }
 
 static bool
-flow_output_addr_sets_handler(struct engine_node *node)
+_flow_output_resource_ref_handler(struct engine_node *node,
+ enum ref_type ref_type)
 {
 struct ed_type_runtime_data *data =
 (struct ed_type_runtime_data *)engine_get_input(
@@ -1447,11 +1495,6 @@ flow_output_addr_sets_handler(struct engine_node *node)
 
 struct ed_type_addr_sets *as_data =
 (struct ed_type_addr_sets *)engine_get_input("addr_sets", node)->data;
-
-/* XXX: The change_tracked check may be added to inc-proc framework. */
-if (!as_data->change_tracked) {
-return false;
-}
 struct shash *addr_sets = _data->addr_sets;
 
 struct ed_type_port_groups *pg_data =
@@ -1510,10 +1553,35 @@ flow_output_addr_sets_handler(struct engine_node *node)
 engine_get_input("SB_logical_flow", node));
 
 bool changed;
-const char *as;
+const char *ref_name;
+struct sset *new, *updated, *deleted;
+
+switch (ref_type) {
+case REF_TYPE_ADDRSET:
+/* XXX: The change_tracked check may be added to inc-proc
+ * framework. */
+if (!as_data->change_tracked) {
+return false;
+}
+new = _data->new;
+updated = _data->updated;
+deleted = _data->deleted;
+break;
+case REF_TYPE_PORTGROUP:
+if (!pg_data->change_tracked) {
+return false;
+}
+new = _data->new;
+updated = _data->updated;
+deleted = _data->deleted;
+break;
+default:
+OVS_NOT_REACHED();
+}
+
 
-SSET_FOR_EACH (as, _data->deleted) {
-if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, as,
+SSET_FOR_EACH (ref_name, deleted) {
+if (!lflow_handle_changed_ref(ref_type, ref_name,
 sbrec_multicast_group_by_name_datapath,
 sbrec_port_binding_by_name,dhcp_table,
 dhcpv6_table, logical_flow_table,
@@ -1525,8 +1593,8 @@