From 7a4814d4a50a78a9434df7f55ca0491f4736c658 Mon Sep 17 00:00:00 2001

From: Numan Siddique <numans@ovn.org>

Date: Wed, 22 Jan 2020 19:02:18 +0530

Subject: [PATCH 4/4] ovn-controller: Remove ports from struct local_datapaths.

struct local_datapaths stores the array of port bindings for each datapath.

These ports are used only in the pinctrl module to check if a mac binding

has been learnt for the buffered packets.

MAC bindings are always learnt in the router pipeline and so

logical_port column of MAC_Binding table will always refer to a

logical router port. run_buffered_binding() of pinctrl module can use

the peer ports stored in the struct local_datapaths instead.

This would save many calls to mac_binding_lookup().

This patch doesn't store the array of port bindings for each local

datapath as it is not required at all.

Earlier, the peer ports were stored only for patch port bindings. But we can have

peer ports even for l3gateway port bindings. This patch now considers l3gateway

ports also for storing the peer ports in struct local_datapaths.

Acked-by: Han Zhou <hzhou@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry-picked from upstream master commit 6c7d9f1ff6a50f695bfd13be3912f4e82c5045f5)

Change-Id: I30e984df51c1d8f81b74b863aaf37778fffc6b96

---

 ovn/controller/binding.c        | 26 +++++++++++++-------------

 ovn/controller/ovn-controller.c |  2 --

 ovn/controller/ovn-controller.h |  4 ----

 ovn/controller/pinctrl.c        | 11 +++++++++--

 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c

index 1380a3e6e..d94e43893 100644

--- a/ovn/controller/binding.c

+++ b/ovn/controller/binding.c

@@ -146,7 +146,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,

     const struct sbrec_port_binding *pb;

     SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,

                                        sbrec_port_binding_by_datapath) {

-        if (!strcmp(pb->type, "patch")) {

+        if (!strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) {

             const char *peer_name = smap_get(&pb->options, "peer");

             if (peer_name) {

                 const struct sbrec_port_binding *peer;

@@ -155,11 +155,18 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,

                                             peer_name);

                 if (peer && peer->datapath) {

-                    add_local_datapath__(sbrec_datapath_binding_by_key,

-                                         sbrec_port_binding_by_datapath,

-                                         sbrec_port_binding_by_name,

-                                         peer->datapath, false,

-                                         depth + 1, local_datapaths);

+                    if (!strcmp(pb->type, "patch")) {

+                        /* Add the datapath to local datapath only for patch

+                         * ports. For l3gateway ports, since gateway router

+                         * resides on one chassis, we don't need to add.

+                         * Otherwise, all other chassis might create patch

+                         * ports between br-int and the provider bridge. */

+                        add_local_datapath__(sbrec_datapath_binding_by_key,

+                                             sbrec_port_binding_by_datapath,

+                                             sbrec_port_binding_by_name,

+                                             peer->datapath, false,

+                                             depth + 1, local_datapaths);

+                    }

                     ld->n_peer_ports++;

                     if (ld->n_peer_ports > ld->n_allocated_peer_ports) {

                         ld->peer_ports =

@@ -172,13 +179,6 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,

                 }

             }

         }

-

-        ld->n_ports++;

-        if (ld->n_ports > ld->n_allocated_ports) {

-            ld->ports = x2nrealloc(ld->ports, &ld->n_allocated_ports,

-                                   sizeof *ld->ports);

-        }

-        ld->ports[ld->n_ports - 1] = pb;

     }

     sbrec_port_binding_index_destroy_row(target);

 }

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c

index 8d7ea89c4..b80d6c0dc 100644

--- a/ovn/controller/ovn-controller.c

+++ b/ovn/controller/ovn-controller.c

@@ -929,7 +929,6 @@ en_runtime_data_cleanup(void *data)

     HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,

                         &rt_data->local_datapaths) {

         free(cur_node->peer_ports);

-        free(cur_node->ports);

         hmap_remove(&rt_data->local_datapaths, &cur_node->hmap_node);

         free(cur_node);

     }

@@ -953,7 +952,6 @@ en_runtime_data_run(struct engine_node *node, void *data)

         struct local_datapath *cur_node, *next_node;

         HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, local_datapaths) {

             free(cur_node->peer_ports);

-            free(cur_node->ports);

             hmap_remove(local_datapaths, &cur_node->hmap_node);

             free(cur_node);

         }

diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h

index 09ef4b632..8bff57c56 100644

--- a/ovn/controller/ovn-controller.h

+++ b/ovn/controller/ovn-controller.h

@@ -60,10 +60,6 @@ struct local_datapath {

      * hypervisor. */

     bool has_local_l3gateway;

-    const struct sbrec_port_binding **ports;

-    size_t n_ports;

-    size_t n_allocated_ports;

-

     struct {

         const struct sbrec_port_binding *local;

         const struct sbrec_port_binding *remote;

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c

index 5636fc1e0..800bde7e7 100644

--- a/ovn/controller/pinctrl.c

+++ b/ovn/controller/pinctrl.c

@@ -2973,10 +2973,17 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,

     bool notify = false;

     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {

+        /* MAC_Binding.logical_port will always belong to a

+         * a router datapath. Hence we can skip logical switch

+         * datapaths.

+         * */

+        if (datapath_is_switch(ld->datapath)) {

+            continue;

+        }

-        for (size_t i = 0; i < ld->n_ports; i++) {

+        for (size_t i = 0; i < ld->n_peer_ports; i++) {

-            const struct sbrec_port_binding *pb = ld->ports[i];

+            const struct sbrec_port_binding *pb = ld->peer_ports[i].local;

             struct buffered_packets *cur_qp, *next_qp;

             HMAP_FOR_EACH_SAFE (cur_qp, next_qp, hmap_node,

                                 &buffered_packets_map) {

-- 

2.26.2