diff mbox series

[ovs-dev,RFC] binding: add the capability to apply QoS for lsp

Message ID 2fe56916610d2d85499165315b6828e37c5aa5ff.1666992899.git.lorenzo.bianconi@redhat.com
State Superseded
Headers show
Series [ovs-dev,RFC] binding: add the capability to apply QoS for lsp | expand

Commit Message

Lorenzo Bianconi Oct. 28, 2022, 9:35 p.m. UTC
Introduce the capability to apply QoS rules for logical switch ports
claimed by ovn-controller. Rely on shash instead of sset for
egress_ifaces.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2129742
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/binding.c        | 77 +++++++++++++++++++++++++++----------
 controller/binding.h        |  2 +-
 controller/ovn-controller.c |  9 ++---
 tests/system-ovn.at         | 27 +++++++++++++
 4 files changed, 88 insertions(+), 27 deletions(-)
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index c3d2b2e42..6d737d869 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -115,6 +115,7 @@  struct qos_queue {
     uint32_t min_rate;
     uint32_t max_rate;
     uint32_t burst;
+    char *port_name;
 };
 
 void
@@ -147,6 +148,8 @@  static void update_lport_tracking(const struct sbrec_port_binding *pb,
                                   struct hmap *tracked_dp_bindings,
                                   bool claimed);
 
+static bool is_lport_vif(const struct sbrec_port_binding *pb);
+
 static void
 get_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
 {
@@ -166,6 +169,7 @@  get_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
     node->max_rate = max_rate;
     node->burst = burst;
     node->queue_id = queue_id;
+    node->port_name = is_lport_vif(pb) ? xstrdup(pb->logical_port) : NULL;
 }
 
 static const struct ovsrec_qos *
@@ -191,7 +195,7 @@  static bool
 set_noop_qos(struct ovsdb_idl_txn *ovs_idl_txn,
              const struct ovsrec_port_table *port_table,
              const struct ovsrec_qos_table *qos_table,
-             struct sset *egress_ifaces)
+             struct shash *egress_ifaces)
 {
     if (!ovs_idl_txn) {
         return false;
@@ -206,11 +210,11 @@  set_noop_qos(struct ovsdb_idl_txn *ovs_idl_txn,
     size_t count = 0;
 
     OVSREC_PORT_TABLE_FOR_EACH (port, port_table) {
-        if (sset_contains(egress_ifaces, port->name)) {
+        if (shash_find(egress_ifaces, port->name)) {
             ovsrec_port_set_qos(port, noop_qos);
             count++;
         }
-        if (sset_count(egress_ifaces) == count) {
+        if (shash_count(egress_ifaces) == count) {
             break;
         }
     }
@@ -229,9 +233,10 @@  set_qos_type(struct netdev *netdev, const char *type)
 }
 
 static void
-setup_qos(const char *egress_iface, struct hmap *queue_map)
+setup_qos(struct shash_node *entry, struct hmap *queue_map)
 {
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+    const char *egress_iface = entry->name;
     struct netdev *netdev_phy;
 
     if (!egress_iface) {
@@ -331,6 +336,10 @@  setup_qos(const char *egress_iface, struct hmap *queue_map)
             continue;
         }
 
+        if (sb_info->port_name && strcmp(sb_info->port_name, entry->data)) {
+            continue;
+        }
+
         smap_clear(&queue_details);
         smap_add_format(&queue_details, "min-rate", "%d", sb_info->min_rate);
         smap_add_format(&queue_details, "max-rate", "%d", sb_info->max_rate);
@@ -352,6 +361,7 @@  destroy_qos_map(struct hmap *qos_map)
 {
     struct qos_queue *qos_queue;
     HMAP_FOR_EACH_POP (qos_queue, node, qos_map) {
+        free(qos_queue->port_name);
         free(qos_queue);
     }
 
@@ -397,7 +407,7 @@  sbrec_get_port_encap(const struct sbrec_chassis *chassis_rec,
 static void
 add_localnet_egress_interface_mappings(
         const struct sbrec_port_binding *port_binding,
-        struct shash *bridge_mappings, struct sset *egress_ifaces)
+        struct shash *bridge_mappings, struct shash *egress_ifaces)
 {
     const char *network = smap_get(&port_binding->options, "network_name");
     if (!network) {
@@ -422,7 +432,7 @@  add_localnet_egress_interface_mappings(
             if (!is_egress_iface) {
                 continue;
             }
-            sset_add(egress_ifaces, iface_rec->name);
+            shash_add(egress_ifaces, iface_rec->name, "");
         }
     }
 }
@@ -467,7 +477,7 @@  update_ld_multichassis_ports(const struct sbrec_port_binding *binding_rec,
 static void
 update_ld_localnet_port(const struct sbrec_port_binding *binding_rec,
                         struct shash *bridge_mappings,
-                        struct sset *egress_ifaces,
+                        struct shash *egress_ifaces,
                         struct hmap *local_datapaths)
 {
     /* Ignore localnet ports for unplugged networks. */
@@ -1512,6 +1522,28 @@  consider_vif_lport(const struct sbrec_port_binding *pb,
             b_lport = local_binding_add_lport(binding_lports, lbinding, pb,
                                               LP_VIF);
         }
+
+        for (size_t i = 0; i < b_ctx_in->br_int->n_ports; i++) {
+            const struct ovsrec_port *port_rec = b_ctx_in->br_int->ports[i];
+            if (!strcmp(port_rec->name, b_ctx_in->br_int->name)) {
+                continue;
+            }
+
+            for (size_t j = 0; j < port_rec->n_interfaces; j++) {
+                const struct ovsrec_interface *iface_rec
+                    = port_rec->interfaces[j];
+                if (!smap_get(&iface_rec->external_ids, "ovn-egress-iface")) {
+                    continue;
+                }
+
+                const char *iface_id =
+                    smap_get(&iface_rec->external_ids, "iface-id");
+                if (iface_id) {
+                    shash_add(b_ctx_out->egress_ifaces, iface_rec->name,
+                              iface_id);
+                }
+            }
+        }
     }
 
     return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
@@ -1854,14 +1886,14 @@  build_local_bindings(struct binding_ctx_in *b_ctx_in,
             &b_ctx_out->lbinding_data->bindings;
         for (j = 0; j < port_rec->n_interfaces; j++) {
             const struct ovsrec_interface *iface_rec;
+            struct local_binding *lbinding = NULL;
 
             iface_rec = port_rec->interfaces[j];
             iface_id = smap_get(&iface_rec->external_ids, "iface-id");
             int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
 
             if (iface_id && ofport > 0) {
-                struct local_binding *lbinding =
-                    local_binding_find(local_bindings, iface_id);
+                lbinding = local_binding_find(local_bindings, iface_id);
                 if (!lbinding) {
                     lbinding = local_binding_create(iface_id, iface_rec);
                     local_binding_add(local_bindings, lbinding);
@@ -1887,8 +1919,11 @@  build_local_bindings(struct binding_ctx_in *b_ctx_in,
                 const char *tunnel_iface
                     = smap_get(&iface_rec->status, "tunnel_egress_iface");
                 if (tunnel_iface) {
-                    sset_add(b_ctx_out->egress_ifaces, tunnel_iface);
+                    shash_add(b_ctx_out->egress_ifaces, tunnel_iface, "");
                 }
+            } else if (lbinding && smap_get(&iface_rec->external_ids,
+                                            "ovn-egress-iface")) {
+                shash_add(b_ctx_out->egress_ifaces, iface_rec->name, iface_id);
             }
         }
     }
@@ -1910,7 +1945,7 @@  binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
     }
 
     struct hmap *qos_map_ptr =
-        !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
+        !shash_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
 
     struct ovs_list localnet_lports = OVS_LIST_INITIALIZER(&localnet_lports);
     struct ovs_list external_lports = OVS_LIST_INITIALIZER(&external_lports);
@@ -2043,11 +2078,11 @@  binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
 
     shash_destroy(&bridge_mappings);
 
-    if (!sset_is_empty(b_ctx_out->egress_ifaces)
+    if (!shash_is_empty(b_ctx_out->egress_ifaces)
         && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table,
                         b_ctx_in->qos_table, b_ctx_out->egress_ifaces)) {
-        const char *entry;
-        SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
+        struct shash_node *entry;
+        SHASH_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
             setup_qos(entry, &qos_map);
         }
     }
@@ -2405,7 +2440,7 @@  binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
         }
 
         if (smap_get(&iface_rec->external_ids, "ovn-egress-iface") ||
-            sset_contains(b_ctx_out->egress_ifaces, iface_rec->name)) {
+            shash_find(b_ctx_out->egress_ifaces, iface_rec->name)) {
             handled = false;
             break;
         }
@@ -2453,7 +2488,7 @@  binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
 
     struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
     struct hmap *qos_map_ptr =
-        sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
+        shash_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
 
     /*
      * We consider an OVS interface for claiming if the following
@@ -2494,8 +2529,8 @@  binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
                                                b_ctx_in->port_table,
                                                b_ctx_in->qos_table,
                                                b_ctx_out->egress_ifaces)) {
-        const char *entry;
-        SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
+        struct shash_node *entry;
+        SHASH_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
             setup_qos(entry, &qos_map);
         }
     }
@@ -2937,7 +2972,7 @@  delete_done:
 
     struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
     struct hmap *qos_map_ptr =
-        sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
+        shash_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
 
     SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
                                                b_ctx_in->port_binding_table) {
@@ -3017,8 +3052,8 @@  delete_done:
                                                b_ctx_in->port_table,
                                                b_ctx_in->qos_table,
                                                b_ctx_out->egress_ifaces)) {
-        const char *entry;
-        SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
+        struct shash_node *entry;
+        SHASH_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
             setup_qos(entry, &qos_map);
         }
     }
diff --git a/controller/binding.h b/controller/binding.h
index ad959a9e6..9a348b5fb 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -91,7 +91,7 @@  struct binding_ctx_out {
      */
     bool non_vif_ports_changed;
 
-    struct sset *egress_ifaces;
+    struct shash *egress_ifaces;
     /* smap of OVS interface name as key and
      * OVS interface external_ids:iface-id as value. */
     struct smap *local_iface_ids;
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 8895c7a2b..14cfeb480 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1237,7 +1237,7 @@  struct ed_type_runtime_data {
     struct sset active_tunnels;
 
     /* runtime data engine private data. */
-    struct sset egress_ifaces;
+    struct shash egress_ifaces;
     struct smap local_iface_ids;
 
     /* Tracked data. See below for more details and comments. */
@@ -1333,7 +1333,7 @@  en_runtime_data_init(struct engine_node *node OVS_UNUSED,
     sset_init(&data->local_lports);
     related_lports_init(&data->related_lports);
     sset_init(&data->active_tunnels);
-    sset_init(&data->egress_ifaces);
+    shash_init(&data->egress_ifaces);
     smap_init(&data->local_iface_ids);
     local_binding_data_init(&data->lbinding_data);
     shash_init(&data->local_active_ports_ipv6_pd);
@@ -1353,7 +1353,7 @@  en_runtime_data_cleanup(void *data)
     sset_destroy(&rt_data->local_lports);
     related_lports_destroy(&rt_data->related_lports);
     sset_destroy(&rt_data->active_tunnels);
-    sset_destroy(&rt_data->egress_ifaces);
+    shash_destroy(&rt_data->egress_ifaces);
     smap_destroy(&rt_data->local_iface_ids);
     local_datapaths_destroy(&rt_data->local_datapaths);
     shash_destroy(&rt_data->local_active_ports_ipv6_pd);
@@ -1471,13 +1471,12 @@  en_runtime_data_run(struct engine_node *node, void *data)
         sset_destroy(local_lports);
         related_lports_destroy(&rt_data->related_lports);
         sset_destroy(active_tunnels);
-        sset_destroy(&rt_data->egress_ifaces);
+        shash_clear(&rt_data->egress_ifaces);
         smap_destroy(&rt_data->local_iface_ids);
         hmap_init(local_datapaths);
         sset_init(local_lports);
         related_lports_init(&rt_data->related_lports);
         sset_init(active_tunnels);
-        sset_init(&rt_data->egress_ifaces);
         smap_init(&rt_data->local_iface_ids);
         local_binding_data_init(&rt_data->lbinding_data);
     }
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 20c058415..4a0ae73c3 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6335,6 +6335,10 @@  ADD_NAMESPACES(sw01)
 ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03")
 ovn-nbctl lsp-add sw0 sw01 \
     -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2"
+ADD_NAMESPACES(sw02)
+ADD_VETH(sw02, sw02, br-int, "192.168.1.3/24", "f2:00:00:01:02:03")
+ovn-nbctl lsp-add sw0 sw02 \
+    -- lsp-set-addresses sw02 "f2:00:00:01:02:03 192.168.1.3"
 
 ADD_NAMESPACES(public)
 ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05")
@@ -6353,11 +6357,34 @@  OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
 OVS_WAIT_UNTIL([tc class show dev ovs-public | \
                 grep -q 'class htb .* rate 200Kbit ceil 300Kbit burst 375000b cburst 375000b'])
 
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw01 options:qos_min_rate=400000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw01 options:qos_max_rate=800000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw01 options:qos_burst=5000000])
+AT_CHECK([ovs-vsctl set interface ovs-sw01 external-ids:ovn-egress-iface=true])
+
+OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-sw01'])
+OVS_WAIT_UNTIL([tc class show dev ovs-sw01 | \
+                grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst 625000b cburst 625000b'])
+
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 options:qos_min_rate=600000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 options:qos_max_rate=6000000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 options:qos_burst=6000000])
+AT_CHECK([ovs-vsctl set interface ovs-sw02 external-ids:ovn-egress-iface=true])
+
+OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-sw02'])
+OVS_WAIT_UNTIL([tc class show dev ovs-sw02 | \
+                grep -q 'class htb .* rate 600Kbit ceil 6Mbit burst 750000b cburst 750000b'])
+
 AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_min_rate=200000])
 AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_max_rate=300000])
 AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_burst=3000000])
 OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1: dev ovs-public')" = ""])
 
+AT_CHECK([ovn-nbctl set Logical_Switch_Port sw01 options:qos_min_rate=200000])
+OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-sw01'])
+OVS_WAIT_UNTIL([tc class show dev ovs-sw01 | \
+                grep -q 'class htb .* rate 200Kbit ceil 800Kbit burst 625000b cburst 625000b'])
+
 kill $(pidof ovn-controller)
 
 as ovn-sb