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

[Ben Pfaff]

On Wed, Sep 07, 2016 at 11:40:11AM +0530, bschanmu at redhat.com wrote:
> From: Babu Shanmugam <bschanmu at redhat.com>
> 
> 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>

Thanks for working on this.

I folded in the following and applied this to master.

diff --git a/NEWS b/NEWS
index 725754a..8480e3c 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v2.6.0
 ---------------------
+   - OVN:
+     * QoS is now implemented via egress shaping rather than ingress policing.
    - Fixed regression in table stats maintenance introduced in OVS
      2.3.0, wherein the number of OpenFlow table hits and misses was
      not accurate.
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 185f924..0bf6145 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -26,13 +26,6 @@
 #include "openvswitch/uuid.h"
 #include "util.h"
 
-/* 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;
 struct lexer;
 struct ofpbuf;
@@ -228,6 +221,13 @@ struct ovnact_put_dhcp_opts {
     size_t n_options;
 };
 
+/* 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
+
 /* OVNACT_SET_QUEUE. */
 struct ovnact_set_queue {
     struct ovnact ovnact;
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index f5607df..a35909f 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1289,6 +1289,18 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
         case OVNACT_PUT_DHCPV6_OPTS:
             execute_put_dhcp_opts(ovnact_get_PUT_DHCPV6_OPTS(a), uflow);
             break;
+
+        case OVNACT_SET_QUEUE:
+            /* The set_queue action is slippery from a logical perspective.  It
+             * has no visible effect as long as the packet remains on the same
+             * chassis: it can bounce from one logical datapath to another
+             * retaining the queue and even end up at a VM on the same chassis.
+             * Without taking the physical arrangement into account, we can't
+             * do anything with this action other than just to note that it
+             * happened.  If we ever add some physical knowledge to ovn-trace,
+             * though, it would be easy enough to track the queue information
+             * by adjusting uflow->skb_priority. */
+            break;
         }
 
     }