diff mbox series

[ovs-dev,v2,ovn,1/3] Add egress QoS mapping for non-tunnel interfaces

Message ID 1a3538a3253adf26f65b18a81b13451036c3986e.1569342607.git.lorenzo.bianconi@redhat.com
State Accepted
Headers show
Series Introduce localnet egress QoS support | expand

Commit Message

Lorenzo Bianconi Sept. 24, 2019, 4:39 p.m. UTC
Introduce add_localnet_egress_interface_mappings routine in order to collect as
egress interfaces all ovs bridge interfaces marked with ovn-egress-iface
in the external_ids column of ovs interface table.
ovn-egress-iface is used to indicate to which localnet ports QoS egress
shaping has to be applied.
Refactor add_bridge_mappings routine

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/binding.c        | 51 ++++++++++++++++++++++++-
 controller/binding.h        |  4 ++
 controller/ovn-controller.c |  3 +-
 controller/patch.c          | 76 +++++++++++++++++++++----------------
 controller/patch.h          |  4 ++
 5 files changed, 103 insertions(+), 35 deletions(-)

Comments

Dumitru Ceara Sept. 25, 2019, 11:41 a.m. UTC | #1
On Tue, Sep 24, 2019 at 6:40 PM Lorenzo Bianconi
<lorenzo.bianconi@redhat.com> wrote:
>
> Introduce add_localnet_egress_interface_mappings routine in order to collect as
> egress interfaces all ovs bridge interfaces marked with ovn-egress-iface
> in the external_ids column of ovs interface table.
> ovn-egress-iface is used to indicate to which localnet ports QoS egress
> shaping has to be applied.
> Refactor add_bridge_mappings routine
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Looks good to me. Thanks!

Acked-by: Dumitru Ceara <dceara@redhat.com>

> ---
>  controller/binding.c        | 51 ++++++++++++++++++++++++-
>  controller/binding.h        |  4 ++
>  controller/ovn-controller.c |  3 +-
>  controller/patch.c          | 76 +++++++++++++++++++++----------------
>  controller/patch.h          |  4 ++
>  5 files changed, 103 insertions(+), 35 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 242163d59..aad9d39e6 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -18,6 +18,7 @@
>  #include "ha-chassis.h"
>  #include "lflow.h"
>  #include "lport.h"
> +#include "patch.h"
>
>  #include "lib/bitmap.h"
>  #include "openvswitch/poll-loop.h"
> @@ -532,6 +533,9 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
>          /* Add all localnet ports to local_lports so that we allocate ct zones
>           * for them. */
>          sset_add(local_lports, binding_rec->logical_port);
> +        if (qos_map && ovs_idl_txn) {
> +            get_qos_params(binding_rec, qos_map);
> +        }
>      } else if (!strcmp(binding_rec->type, "external")) {
>          if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
>                                        chassis_rec)) {
> @@ -619,10 +623,48 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
>      }
>  }
>
> +static void
> +add_localnet_egress_interface_mappings(
> +        const struct sbrec_port_binding *port_binding,
> +        struct shash *bridge_mappings, struct sset *egress_ifaces)
> +{
> +    const char *network = smap_get(&port_binding->options, "network_name");
> +    if (!network) {
> +        return;
> +    }
> +
> +    struct ovsrec_bridge *br_ln = shash_find_data(bridge_mappings, network);
> +    if (!br_ln) {
> +        return;
> +    }
> +
> +    /* Add egress-ifaces from the connected bridge */
> +    for (size_t i = 0; i < br_ln->n_ports; i++) {
> +        const struct ovsrec_port *port_rec = br_ln->ports[i];
> +
> +        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
> +            const struct ovsrec_interface *iface_rec;
> +
> +            iface_rec = port_rec->interfaces[j];
> +            bool is_egress_iface = smap_get_bool(&iface_rec->external_ids,
> +                                                 "ovn-egress-iface", false);
> +            if (!is_egress_iface) {
> +                continue;
> +            }
> +            sset_add(egress_ifaces, iface_rec->name);
> +        }
> +    }
> +}
> +
>  static void
>  consider_localnet_port(const struct sbrec_port_binding *binding_rec,
> +                       struct shash *bridge_mappings,
> +                       struct sset *egress_ifaces,
>                         struct hmap *local_datapaths)
>  {
> +    add_localnet_egress_interface_mappings(binding_rec,
> +            bridge_mappings, egress_ifaces);
> +
>      struct local_datapath *ld
>          = get_local_datapath(local_datapaths,
>                               binding_rec->datapath->tunnel_key);
> @@ -655,6 +697,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>              const struct ovsrec_bridge *br_int,
>              const struct sbrec_chassis *chassis_rec,
>              const struct sset *active_tunnels,
> +            const struct ovsrec_bridge_table *bridge_table,
> +            const struct ovsrec_open_vswitch_table *ovs_table,
>              struct hmap *local_datapaths, struct sset *local_lports,
>              struct sset *local_lport_ids)
>  {
> @@ -663,6 +707,7 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>      }
>
>      const struct sbrec_port_binding *binding_rec;
> +    struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
>      struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
>      struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
>      struct hmap qos_map;
> @@ -688,14 +733,18 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>
>      }
>
> +    add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
> +
>      /* Run through each binding record to see if it is a localnet port
>       * on local datapaths discovered from above loop, and update the
>       * corresponding local datapath accordingly. */
>      SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) {
>          if (!strcmp(binding_rec->type, "localnet")) {
> -            consider_localnet_port(binding_rec, local_datapaths);
> +            consider_localnet_port(binding_rec, &bridge_mappings,
> +                                   &egress_ifaces, local_datapaths);
>          }
>      }
> +    shash_destroy(&bridge_mappings);
>
>      if (!sset_is_empty(&egress_ifaces)
>          && set_noop_qos(ovs_idl_txn, port_table, qos_table, &egress_ifaces)) {
> diff --git a/controller/binding.h b/controller/binding.h
> index bae162ede..924891c1b 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -26,6 +26,8 @@ struct ovsdb_idl_txn;
>  struct ovsrec_bridge;
>  struct ovsrec_port_table;
>  struct ovsrec_qos_table;
> +struct ovsrec_bridge_table;
> +struct ovsrec_open_vswitch_table;
>  struct sbrec_chassis;
>  struct sbrec_port_binding_table;
>  struct sset;
> @@ -42,6 +44,8 @@ void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                   const struct ovsrec_bridge *br_int,
>                   const struct sbrec_chassis *,
>                   const struct sset *active_tunnels,
> +                 const struct ovsrec_bridge_table *bridge_table,
> +                 const struct ovsrec_open_vswitch_table *ovs_table,
>                   struct hmap *local_datapaths,
>                   struct sset *local_lports, struct sset *local_lport_ids);
>  bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 33ece59be..b46a1d151 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1082,7 +1082,8 @@ en_runtime_data_run(struct engine_node *node)
>                  sbrec_port_binding_by_name,
>                  port_table, qos_table, pb_table,
>                  br_int, chassis,
> -                active_tunnels, local_datapaths,
> +                active_tunnels, bridge_table,
> +                ovs_table, local_datapaths,
>                  local_lports, local_lport_ids);
>
>      update_ct_zones(local_lports, local_datapaths, ct_zones,
> diff --git a/controller/patch.c b/controller/patch.c
> index a6770c6d5..f2053de7b 100644
> --- a/controller/patch.c
> +++ b/controller/patch.c
> @@ -129,6 +129,48 @@ remove_port(const struct ovsrec_bridge_table *bridge_table,
>      }
>  }
>
> +void
> +add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table,
> +                        const struct ovsrec_bridge_table *bridge_table,
> +                        struct shash *bridge_mappings)
> +{
> +    const struct ovsrec_open_vswitch *cfg;
> +
> +    cfg = ovsrec_open_vswitch_table_first(ovs_table);
> +    if (cfg) {
> +        const char *mappings_cfg;
> +        char *cur, *next, *start;
> +
> +        mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings");
> +        if (!mappings_cfg || !mappings_cfg[0]) {
> +            return;
> +        }
> +
> +        next = start = xstrdup(mappings_cfg);
> +        while ((cur = strsep(&next, ",")) && *cur) {
> +            const struct ovsrec_bridge *ovs_bridge;
> +            char *network, *bridge = cur;
> +
> +            network = strsep(&bridge, ":");
> +            if (!bridge || !*network || !*bridge) {
> +                VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'",
> +                         mappings_cfg);
> +                break;
> +            }
> +
> +            ovs_bridge = get_bridge(bridge_table, bridge);
> +            if (!ovs_bridge) {
> +                VLOG_WARN("Bridge '%s' not found for network '%s'",
> +                          bridge, network);
> +                continue;
> +            }
> +
> +            shash_add(bridge_mappings, network, ovs_bridge);
> +        }
> +        free(start);
> +    }
> +}
> +
>  /* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for
>   * the local bridge mappings.  Removes any patch ports for bridge mappings that
>   * already existed from 'existing_ports'. */
> @@ -142,41 +184,9 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
>                      const struct sbrec_chassis *chassis)
>  {
>      /* Get ovn-bridge-mappings. */
> -    const char *mappings_cfg = "";
> -    const struct ovsrec_open_vswitch *cfg;
> -    cfg = ovsrec_open_vswitch_table_first(ovs_table);
> -    if (cfg) {
> -        mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings");
> -        if (!mappings_cfg || !mappings_cfg[0]) {
> -            return;
> -        }
> -    }
> -
> -    /* Parse bridge mappings. */
>      struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
> -    char *cur, *next, *start;
> -    next = start = xstrdup(mappings_cfg);
> -    while ((cur = strsep(&next, ",")) && *cur) {
> -        char *network, *bridge = cur;
> -        const struct ovsrec_bridge *ovs_bridge;
> -
> -        network = strsep(&bridge, ":");
> -        if (!bridge || !*network || !*bridge) {
> -            VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'",
> -                    mappings_cfg);
> -            break;
> -        }
>
> -        ovs_bridge = get_bridge(bridge_table, bridge);
> -        if (!ovs_bridge) {
> -            VLOG_WARN("Bridge '%s' not found for network '%s'",
> -                    bridge, network);
> -            continue;
> -        }
> -
> -        shash_add(&bridge_mappings, network, ovs_bridge);
> -    }
> -    free(start);
> +    add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
>
>      const struct sbrec_port_binding *binding;
>      SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, port_binding_table) {
> diff --git a/controller/patch.h b/controller/patch.h
> index 9018e4967..49b0b2e90 100644
> --- a/controller/patch.h
> +++ b/controller/patch.h
> @@ -30,7 +30,11 @@ struct ovsrec_open_vswitch_table;
>  struct ovsrec_port_table;
>  struct sbrec_port_binding_table;
>  struct sbrec_chassis;
> +struct shash;
>
> +void add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table,
> +                             const struct ovsrec_bridge_table *bridge_table,
> +                             struct shash *bridge_mappings);
>  void patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
>                 const struct ovsrec_bridge_table *,
>                 const struct ovsrec_open_vswitch_table *,
> --
> 2.21.0
>
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 242163d59..aad9d39e6 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -18,6 +18,7 @@ 
 #include "ha-chassis.h"
 #include "lflow.h"
 #include "lport.h"
+#include "patch.h"
 
 #include "lib/bitmap.h"
 #include "openvswitch/poll-loop.h"
@@ -532,6 +533,9 @@  consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
         /* Add all localnet ports to local_lports so that we allocate ct zones
          * for them. */
         sset_add(local_lports, binding_rec->logical_port);
+        if (qos_map && ovs_idl_txn) {
+            get_qos_params(binding_rec, qos_map);
+        }
     } else if (!strcmp(binding_rec->type, "external")) {
         if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
                                       chassis_rec)) {
@@ -619,10 +623,48 @@  consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
     }
 }
 
+static void
+add_localnet_egress_interface_mappings(
+        const struct sbrec_port_binding *port_binding,
+        struct shash *bridge_mappings, struct sset *egress_ifaces)
+{
+    const char *network = smap_get(&port_binding->options, "network_name");
+    if (!network) {
+        return;
+    }
+
+    struct ovsrec_bridge *br_ln = shash_find_data(bridge_mappings, network);
+    if (!br_ln) {
+        return;
+    }
+
+    /* Add egress-ifaces from the connected bridge */
+    for (size_t i = 0; i < br_ln->n_ports; i++) {
+        const struct ovsrec_port *port_rec = br_ln->ports[i];
+
+        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
+            const struct ovsrec_interface *iface_rec;
+
+            iface_rec = port_rec->interfaces[j];
+            bool is_egress_iface = smap_get_bool(&iface_rec->external_ids,
+                                                 "ovn-egress-iface", false);
+            if (!is_egress_iface) {
+                continue;
+            }
+            sset_add(egress_ifaces, iface_rec->name);
+        }
+    }
+}
+
 static void
 consider_localnet_port(const struct sbrec_port_binding *binding_rec,
+                       struct shash *bridge_mappings,
+                       struct sset *egress_ifaces,
                        struct hmap *local_datapaths)
 {
+    add_localnet_egress_interface_mappings(binding_rec,
+            bridge_mappings, egress_ifaces);
+
     struct local_datapath *ld
         = get_local_datapath(local_datapaths,
                              binding_rec->datapath->tunnel_key);
@@ -655,6 +697,8 @@  binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
             const struct ovsrec_bridge *br_int,
             const struct sbrec_chassis *chassis_rec,
             const struct sset *active_tunnels,
+            const struct ovsrec_bridge_table *bridge_table,
+            const struct ovsrec_open_vswitch_table *ovs_table,
             struct hmap *local_datapaths, struct sset *local_lports,
             struct sset *local_lport_ids)
 {
@@ -663,6 +707,7 @@  binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
     }
 
     const struct sbrec_port_binding *binding_rec;
+    struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
     struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
     struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
     struct hmap qos_map;
@@ -688,14 +733,18 @@  binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 
     }
 
+    add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
+
     /* Run through each binding record to see if it is a localnet port
      * on local datapaths discovered from above loop, and update the
      * corresponding local datapath accordingly. */
     SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) {
         if (!strcmp(binding_rec->type, "localnet")) {
-            consider_localnet_port(binding_rec, local_datapaths);
+            consider_localnet_port(binding_rec, &bridge_mappings,
+                                   &egress_ifaces, local_datapaths);
         }
     }
+    shash_destroy(&bridge_mappings);
 
     if (!sset_is_empty(&egress_ifaces)
         && set_noop_qos(ovs_idl_txn, port_table, qos_table, &egress_ifaces)) {
diff --git a/controller/binding.h b/controller/binding.h
index bae162ede..924891c1b 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -26,6 +26,8 @@  struct ovsdb_idl_txn;
 struct ovsrec_bridge;
 struct ovsrec_port_table;
 struct ovsrec_qos_table;
+struct ovsrec_bridge_table;
+struct ovsrec_open_vswitch_table;
 struct sbrec_chassis;
 struct sbrec_port_binding_table;
 struct sset;
@@ -42,6 +44,8 @@  void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
                  const struct ovsrec_bridge *br_int,
                  const struct sbrec_chassis *,
                  const struct sset *active_tunnels,
+                 const struct ovsrec_bridge_table *bridge_table,
+                 const struct ovsrec_open_vswitch_table *ovs_table,
                  struct hmap *local_datapaths,
                  struct sset *local_lports, struct sset *local_lport_ids);
 bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 33ece59be..b46a1d151 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1082,7 +1082,8 @@  en_runtime_data_run(struct engine_node *node)
                 sbrec_port_binding_by_name,
                 port_table, qos_table, pb_table,
                 br_int, chassis,
-                active_tunnels, local_datapaths,
+                active_tunnels, bridge_table,
+                ovs_table, local_datapaths,
                 local_lports, local_lport_ids);
 
     update_ct_zones(local_lports, local_datapaths, ct_zones,
diff --git a/controller/patch.c b/controller/patch.c
index a6770c6d5..f2053de7b 100644
--- a/controller/patch.c
+++ b/controller/patch.c
@@ -129,6 +129,48 @@  remove_port(const struct ovsrec_bridge_table *bridge_table,
     }
 }
 
+void
+add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table,
+                        const struct ovsrec_bridge_table *bridge_table,
+                        struct shash *bridge_mappings)
+{
+    const struct ovsrec_open_vswitch *cfg;
+
+    cfg = ovsrec_open_vswitch_table_first(ovs_table);
+    if (cfg) {
+        const char *mappings_cfg;
+        char *cur, *next, *start;
+
+        mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings");
+        if (!mappings_cfg || !mappings_cfg[0]) {
+            return;
+        }
+
+        next = start = xstrdup(mappings_cfg);
+        while ((cur = strsep(&next, ",")) && *cur) {
+            const struct ovsrec_bridge *ovs_bridge;
+            char *network, *bridge = cur;
+
+            network = strsep(&bridge, ":");
+            if (!bridge || !*network || !*bridge) {
+                VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'",
+                         mappings_cfg);
+                break;
+            }
+
+            ovs_bridge = get_bridge(bridge_table, bridge);
+            if (!ovs_bridge) {
+                VLOG_WARN("Bridge '%s' not found for network '%s'",
+                          bridge, network);
+                continue;
+            }
+
+            shash_add(bridge_mappings, network, ovs_bridge);
+        }
+        free(start);
+    }
+}
+
 /* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for
  * the local bridge mappings.  Removes any patch ports for bridge mappings that
  * already existed from 'existing_ports'. */
@@ -142,41 +184,9 @@  add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
                     const struct sbrec_chassis *chassis)
 {
     /* Get ovn-bridge-mappings. */
-    const char *mappings_cfg = "";
-    const struct ovsrec_open_vswitch *cfg;
-    cfg = ovsrec_open_vswitch_table_first(ovs_table);
-    if (cfg) {
-        mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings");
-        if (!mappings_cfg || !mappings_cfg[0]) {
-            return;
-        }
-    }
-
-    /* Parse bridge mappings. */
     struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
-    char *cur, *next, *start;
-    next = start = xstrdup(mappings_cfg);
-    while ((cur = strsep(&next, ",")) && *cur) {
-        char *network, *bridge = cur;
-        const struct ovsrec_bridge *ovs_bridge;
-
-        network = strsep(&bridge, ":");
-        if (!bridge || !*network || !*bridge) {
-            VLOG_ERR("Invalid ovn-bridge-mappings configuration: '%s'",
-                    mappings_cfg);
-            break;
-        }
 
-        ovs_bridge = get_bridge(bridge_table, bridge);
-        if (!ovs_bridge) {
-            VLOG_WARN("Bridge '%s' not found for network '%s'",
-                    bridge, network);
-            continue;
-        }
-
-        shash_add(&bridge_mappings, network, ovs_bridge);
-    }
-    free(start);
+    add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
 
     const struct sbrec_port_binding *binding;
     SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, port_binding_table) {
diff --git a/controller/patch.h b/controller/patch.h
index 9018e4967..49b0b2e90 100644
--- a/controller/patch.h
+++ b/controller/patch.h
@@ -30,7 +30,11 @@  struct ovsrec_open_vswitch_table;
 struct ovsrec_port_table;
 struct sbrec_port_binding_table;
 struct sbrec_chassis;
+struct shash;
 
+void add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table,
+                             const struct ovsrec_bridge_table *bridge_table,
+                             struct shash *bridge_mappings);
 void patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
                const struct ovsrec_bridge_table *,
                const struct ovsrec_open_vswitch_table *,