[ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

Ben Pfaff blp at ovn.org
Wed Jul 27 20:18:00 UTC 2016


On Wed, Jul 20, 2016 at 08:02:03PM +0530, bschanmu at redhat.com wrote:
> ovn-northd processes the list of Port_Bindings and hashes the list of
> queues per chassis. When it finds a port with qos_parameters and without
> a queue_id, it allocates a free queue for the chassis that this port belongs.
> The queue_id information is stored in the options field of Port_binding table.
> Adds an action set_queue to the ingress table 0 of the logical flows
> which will be translated to openflow set_queue by ovn-controller
> 
> ovn-controller opens the netdev corresponding to the tunnel interface's
> status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
> each SB port_binding that has queue_id set, it allocates a queue with the
> qos_parameters of that port. It also frees up unused queues.
> 
> This patch replaces the older approach of policing
> 
> Signed-off-by: Babu Shanmugam <bschanmu at redhat.com>

I suggest folding in the following changes.  Notably, set_queue(0); was
documented but didn't work because QDISC_MIN_QUEUE_ID was 1.

This series has no tests.  I think that at least the DSCP marking
feature could be tested with the existing OVN testing infrastructure.
Will you work on that?

Thanks,

Ben.

--8<--------------------------cut here-------------------------->8--

diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index c0fde28..230115f 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -24,7 +24,11 @@
 #include "openvswitch/dynamic-string.h"
 #include "util.h"
 
-#define QDISC_MIN_QUEUE_ID  1
+/* Valid arguments to set_queue() action.
+ *
+ * QDISC_MIN_QUEUE_ID is the default queue, so user-defined queues should
+ * start at QDISC_MIN_QUEUE_ID+1. */
+#define QDISC_MIN_QUEUE_ID  0
 #define QDISC_MAX_QUEUE_ID  0xf000
 
 struct expr;
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4eef2d9..dc803d2 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -309,7 +309,7 @@ chassis_queueid_in_use(const struct hmap *set, const char *chassis,
 static uint32_t
 allocate_chassis_queueid(struct hmap *set, const char *chassis)
 {
-    for (uint32_t queue_id = QDISC_MIN_QUEUE_ID;
+    for (uint32_t queue_id = QDISC_MIN_QUEUE_ID + 1;
          queue_id <= QDISC_MAX_QUEUE_ID;
          queue_id++) {
         if (!chassis_queueid_in_use(set, chassis, queue_id)) {
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 5e6f9c3..057a4f1 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1768,10 +1768,10 @@ tcp.flags = RST;
         interface, in bits.
       </column>
 
-      <column name="options" key="qdisc_queue_id">
+      <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
-        queue_id used in OpenFlow in struct ofp_action_enqueue. Value should
-        be in the range of 1 to 61440.
+        queue_id used in OpenFlow in struct ofp_action_enqueue.
       </column>
     </group>
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 86efcf5..84ee2c1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -655,6 +655,11 @@ reg1[0] = put_dhcp_opts(offerip=1.2.3.4, domain=1.2.3.4); => DHCP option domain
 # na
 na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport = inport; inport = ""; /* Allow sending out inport. */ output; }; => actions=controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.00.19.00.10.00.00.00.02.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00), prereqs=nd
 
+# set_queue
+set_queue(0); => actions=set_queue:0, prereqs=1
+set_queue(61440); => actions=set_queue:61440, prereqs=1
+set_queue(65535); => Queue ID 65535 for set_queue is not in valid range 0 to 61440.
+
 # Contradictionary prerequisites (allowed but not useful):
 ip4.src = ip6.src[0..31]; => actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd
 ip4.src <-> ip6.src[0..31]; => actions=push:NXM_NX_IPV6_SRC[0..31],push:NXM_OF_IP_SRC[],pop:NXM_NX_IPV6_SRC[0..31],pop:NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd



More information about the dev mailing list