diff mbox series

[ovs-dev,RFC,1/3] controller: fix interface qos aliasing

Message ID 6f0a7535ce0965113a3f2d150835a00dbceeae33.1679083119.git.lorenzo.bianconi@redhat.com
State Changes Requested
Headers show
Series rework OVN QoS implementation | expand

Commit Message

Lorenzo Bianconi March 17, 2023, 8 p.m. UTC 
In the current codebase it is not possible to specify two different QoS
rules for two different localnet ports, even if they are running on two
different datapaths. Both ports will be configured with the latest QoS
rule in the hashmap since it is not possible to link the QoS rule to the
corresponding OVS port. Fix the issue introducing "qos_ovs_port"
parameter in the logical swith port option column. The CMS is responsible
to configure it with the OVS port used for this QoS rule.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/binding.c | 13 +++++++++++--
 northd/northd.c      |  3 ++-
 ovn-nb.xml           |  5 +++++
 ovn-sb.xml           |  5 +++++
 tests/system-ovn.at  | 36 ++++++++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 5df62baef..add64b07d 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -111,6 +111,7 @@  binding_wait(void)
 
 struct qos_queue {
     struct hmap_node node;
+    char *port_name;
     uint32_t queue_id;
     uint32_t min_rate;
     uint32_t max_rate;
@@ -150,18 +151,20 @@  static void update_lport_tracking(const struct sbrec_port_binding *pb,
 static void
 get_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
 {
+    const char *ovs_port = smap_get(&pb->options, "qos_ovs_port");
     uint32_t min_rate = smap_get_int(&pb->options, "qos_min_rate", 0);
     uint32_t max_rate = smap_get_int(&pb->options, "qos_max_rate", 0);
     uint32_t burst = smap_get_int(&pb->options, "qos_burst", 0);
     uint32_t queue_id = smap_get_int(&pb->options, "qdisc_queue_id", 0);
 
-    if ((!min_rate && !max_rate && !burst) || !queue_id) {
+    if ((!min_rate && !max_rate && !burst) || !queue_id || !ovs_port) {
         /* Qos is not configured for this port. */
         return;
     }
 
     struct qos_queue *node = xzalloc(sizeof *node);
     hmap_insert(queue_map, &node->node, hash_int(queue_id, 0));
+    node->port_name = xstrdup(ovs_port);
     node->min_rate = min_rate;
     node->max_rate = max_rate;
     node->burst = burst;
@@ -314,7 +317,8 @@  setup_qos(const char *egress_iface, struct hmap *queue_map)
                 && sb_info->max_rate ==
                 smap_get_int(&queue_details, "max-rate", 0)
                 && sb_info->burst ==
-                smap_get_int(&queue_details, "burst", 0)) {
+                smap_get_int(&queue_details, "burst", 0) &&
+                !strcmp(egress_iface, sb_info->port_name)) {
                 /* This queue is consistent. */
                 hmap_insert(&consistent_queues, &sb_info->node,
                             hash_int(queue_id, 0));
@@ -338,6 +342,10 @@  setup_qos(const char *egress_iface, struct hmap *queue_map)
             continue;
         }
 
+        if (strcmp(egress_iface, sb_info->port_name)) {
+            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);
@@ -359,6 +367,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);
     }
 
diff --git a/northd/northd.c b/northd/northd.c
index fda02c324..e12cf1347 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -559,7 +559,8 @@  free_chassis_queueid(struct hmap *set, const struct uuid *uuid,
 static inline bool
 port_has_qos_params(const struct smap *opts)
 {
-    return (smap_get(opts, "qos_max_rate") ||
+    return smap_get(opts, "qos_ovs_port") &&
+           (smap_get(opts, "qos_max_rate") ||
             smap_get(opts, "qos_burst"));
 }
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 73f707aa0..0ff5d4788 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1142,6 +1142,11 @@ 
           interface, in bits.
         </column>
 
+        <column name="options" key="qos_ovs_port">
+          If set, indicates the corresponding OVS port where OVN will apply
+          the configured QoS rule.
+        </column>
+
         <column name="options" key="hostname">
           <p>
             If set, indicates the DHCPv4 option "Hostname" (option code 12)
diff --git a/ovn-sb.xml b/ovn-sb.xml
index a77f8f4ef..ddb82b08a 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3655,6 +3655,11 @@  tcp.flags = RST;
         interface, in bits.
       </column>
 
+      <column name="options" key="qos_ovs_port">
+        If set, indicates the corresponding OVS port where OVN will apply
+        the configured QoS rule.
+      </column>
+
       <column name="options" key="qdisc_queue_id"
               type='{"type": "integer", "minInteger": 1, "maxInteger": 61440}'>
         Indicates the queue number on the physical device. This is same as the
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index ad1188078..8fb45c6b6 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6554,23 +6554,49 @@  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"
 
+ovn-nbctl ls-add sw1
+
+ADD_NAMESPACES(sw11)
+ADD_VETH(sw11, sw11, br-int, "192.168.4.2/24", "f0:00:00:01:04:03")
+ovn-nbctl lsp-add sw1 sw11 \
+    -- lsp-set-addresses sw11 "f0:00:00:01:04:03 192.168.4.2"
+
 ADD_NAMESPACES(public)
 ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05")
 
+ADD_NAMESPACES(ext)
+ADD_VETH(ext, ext, br-ext, "192.168.3.2/24", "f0:00:00:01:02:06")
+
 AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext])
 ovn-nbctl lsp-add sw0 public \
         -- lsp-set-addresses public unknown \
         -- lsp-set-type public localnet \
         -- lsp-set-options public network_name=phynet
 
+ovn-nbctl lsp-add sw1 ext \
+        -- lsp-set-addresses ext unknown \
+        -- lsp-set-type ext localnet \
+        -- lsp-set-options ext network_name=phynet
+
 AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_min_rate=200000])
 AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_max_rate=300000])
 AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_burst=3000000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_ovs_port=\"ovs-public\"])
 AT_CHECK([ovs-vsctl set interface ovs-public external-ids:ovn-egress-iface=true])
+
+AT_CHECK([ovn-nbctl set Logical_Switch_Port ext options:qos_min_rate=400000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port ext options:qos_max_rate=600000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port ext options:qos_burst=6000000])
+AT_CHECK([ovn-nbctl set Logical_Switch_Port ext options:qos_ovs_port=\"ovs-ext\"])
+AT_CHECK([ovs-vsctl set interface ovs-ext external-ids:ovn-egress-iface=true])
+
 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'])
 
+OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-ext'])
+OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
+                grep -q 'class htb .* rate 400Kbit ceil 600Kbit burst 750000b cburst 750000b'])
 
 AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_max_rate=300000])
 OVS_WAIT_UNTIL([tc class show dev ovs-public | \
@@ -6580,6 +6606,16 @@  AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_min_rate=20000
 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 ext options:qos_max_rate=800000])
+OVS_WAIT_UNTIL([tc class show dev ovs-ext | \
+                grep -q 'class htb .* rate 400Kbit ceil 800Kbit burst 750000b cburst 750000b'])
+
+AT_CHECK([ovn-nbctl remove Logical_Switch_Port ext options qos_min_rate=400000])
+AT_CHECK([ovn-nbctl remove Logical_Switch_Port ext options qos_max_rate=800000])
+AT_CHECK([ovn-nbctl remove Logical_Switch_Port ext options qos_burst=6000000])
+
+OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1: dev ovs-ext')" = ""])
+
 kill $(pidof ovn-controller)
 
 as ovn-sb