Re: [ovs-dev] [PATCH v6 05/17] ovn-controller: runtime_data change handler for SB port-binding

[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
#149 FILE: ovn/controller/binding.c:706:
/* XXX: currently OVSDB change tracking doesn't support getting old

Lines checked: 250, 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 05/17] ovn-controller: runtime_data change handler for SB port-binding

[Han Zhou]

From: Han Zhou 

Evaluates change for SB port-binding in runtime_data node.
If the port-binding change has no impact for the runtime_data it will
not trigger runtime_data change.

Signed-off-by: Han Zhou 
---
 ovn/controller/binding.c| 100 
 ovn/controller/binding.h|   6 +++
 ovn/controller/ovn-controller.c |  41 +++-
 3 files changed, 126 insertions(+), 21 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index c8e737a..b62b3da 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -427,6 +427,41 @@ sbrec_get_port_encap(const struct sbrec_chassis 
*chassis_rec,
 return best_encap;
 }
 
+static bool
+is_our_chassis(const struct sbrec_chassis *chassis_rec,
+   const struct sbrec_port_binding *binding_rec,
+   const struct sset *active_tunnels,
+   const struct shash *lport_to_iface,
+   const struct sset *local_lports)
+{
+const struct ovsrec_interface *iface_rec
+= shash_find_data(lport_to_iface, binding_rec->logical_port);
+
+bool our_chassis = false;
+if (iface_rec
+|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
+sset_contains(local_lports, binding_rec->parent_port))) {
+/* This port is in our chassis unless it is a localport. */
+our_chassis = strcmp(binding_rec->type, "localport");
+} else if (!strcmp(binding_rec->type, "l2gateway")) {
+const char *chassis_id = smap_get(_rec->options,
+  "l2gateway-chassis");
+our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
+} else if (!strcmp(binding_rec->type, "chassisredirect") ||
+   !strcmp(binding_rec->type, "external")) {
+our_chassis = ha_chassis_group_contains(binding_rec->ha_chassis_group,
+chassis_rec) &&
+  ha_chassis_group_is_active(binding_rec->ha_chassis_group,
+ active_tunnels, chassis_rec);
+} else if (!strcmp(binding_rec->type, "l3gateway")) {
+const char *chassis_id = smap_get(_rec->options,
+  "l3gateway-chassis");
+our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
+}
+
+return our_chassis;
+}
+
 static void
 consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
 struct ovsdb_idl_txn *ovs_idl_txn,
@@ -445,7 +480,8 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const struct ovsrec_interface *iface_rec
 = shash_find_data(lport_to_iface, binding_rec->logical_port);
 
-bool our_chassis = false;
+bool our_chassis = is_our_chassis(chassis_rec, binding_rec, active_tunnels,
+  lport_to_iface, local_lports);
 if (iface_rec
 || (binding_rec->parent_port && binding_rec->parent_port[0] &&
 sset_contains(local_lports, binding_rec->parent_port))) {
@@ -460,14 +496,7 @@ consider_local_datapath(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
 if (iface_rec && qos_map && ovs_idl_txn) {
 get_qos_params(binding_rec, qos_map);
 }
-/* This port is in our chassis unless it is a localport. */
-if (strcmp(binding_rec->type, "localport")) {
-our_chassis = true;
-}
 } else if (!strcmp(binding_rec->type, "l2gateway")) {
-const char *chassis_id = smap_get(_rec->options,
-  "l2gateway-chassis");
-our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
 if (our_chassis) {
 sset_add(local_lports, binding_rec->logical_port);
 add_local_datapath(sbrec_datapath_binding_by_key,
@@ -478,19 +507,12 @@ consider_local_datapath(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
 } else if (!strcmp(binding_rec->type, "chassisredirect")) {
 if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
   chassis_rec)) {
-our_chassis = ha_chassis_group_is_active(
-binding_rec->ha_chassis_group,
-active_tunnels, chassis_rec);
-
 add_local_datapath(sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
sbrec_port_binding_by_name,
binding_rec->datapath, false, local_datapaths);
 }
 } else if (!strcmp(binding_rec->type, "l3gateway")) {
-const char *chassis_id = smap_get(_rec->options,
-  "l3gateway-chassis");
-our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
 if (our_chassis) {
 add_local_datapath(sbrec_datapath_binding_by_key,