diff mbox

[ovs-dev] ovn: add qos function.

Message ID OF52836ABC.EF7590E4-ON4825814B.000B216B-4825814B.000B8C9B@zte.com.cn
State Changes Requested
Headers show

Commit Message

wang qianyu June 26, 2017, 2:06 a.m. UTC
The current qos function is used for geneve
tunnel to control the traffic out the ovs. And have no to-port qos 
control.

This patch do the modification as follow
1. change the qos configuration with direction to consistent with neutron
qos rule. Add qos_ingress_max_rate, qos_ingress_burst, qos_egress_max_rate
and qos_egress_burst to instead old qos_max_rate and qos_burst.

2. By configure the interface table field of ingress_policing_rate and
ingress_policing_bust to implement the ingress qos control.

3. Add qos configuration to qos field of port table to implement egress 
qos
control.

4. Add dpdk qos support.

Change-Id: Ie90364cb5bee7f6b7e3086e0867997f7edab7038
---
 ovn/controller/binding.c | 525 
++++++++++++++++++++++++++++-------------------
 ovn/northd/ovn-northd.c  |  17 +-
 ovn/ovn-nb.xml           |  18 +-
 ovn/ovn-sb.xml           |  18 +-
 4 files changed, 360 insertions(+), 218 deletions(-)

 
       <column name="options" key="qdisc_queue_id"

Comments

Ben Pfaff July 12, 2017, 4:36 a.m. UTC | #1
On Mon, Jun 26, 2017 at 10:06:17AM +0800, wang.qianyu@zte.com.cn wrote:
> The current qos function is used for geneve
> tunnel to control the traffic out the ovs. And have no to-port qos 
> control.
> 
> This patch do the modification as follow
> 1. change the qos configuration with direction to consistent with neutron
> qos rule. Add qos_ingress_max_rate, qos_ingress_burst, qos_egress_max_rate
> and qos_egress_burst to instead old qos_max_rate and qos_burst.
> 
> 2. By configure the interface table field of ingress_policing_rate and
> ingress_policing_bust to implement the ingress qos control.
> 
> 3. Add qos configuration to qos field of port table to implement egress 
> qos
> control.
> 
> 4. Add dpdk qos support.
> 
> Change-Id: Ie90364cb5bee7f6b7e3086e0867997f7edab7038

Thanks for working on OVN.

The patch is wordwrapped, so it cannot be applied.

I believe that this patch breaks backward compatibility with existing
Neutron and other CMSes that run on top of OVN, as well as between
ovn-northd and ovn-controller of different versions.  Backward
compatibility is important to us, so it would be better to preserve it.
I think that this could be done by simply keeping the existing names for
qos_max_rate and qos_burst.  I understand the desire to have systematic
names; this could be done by supporting the old names as synonyms for
the equivalent new ones.

The patch needs a Signed-off-by.  Please read the OVS contribution info
for a description of how to add one and what it means (the meaning is
important).

Please add an item to NEWS.

I'll look forward to the next version of the patch.

Thanks,

Ben.
diff mbox

Patch

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index bb76608..f40273f 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -1,4 +1,4 @@ 
-/* Copyright (c) 2015, 2016, 2017 Nicira, Inc.
+/* Copyright (c) 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,13 +33,26 @@  VLOG_DEFINE_THIS_MODULE(binding);
 
 #define OVN_QOS_TYPE "linux-htb"
 
+#define IS_SUB_PORT(PB)  (PB->parent_port && \
+        PB->parent_port[0])
+
 struct qos_queue {
     struct hmap_node node;
     uint32_t queue_id;
-    uint32_t max_rate;
-    uint32_t burst;
+    uint32_t ingress_max_rate;
+    uint32_t ingress_burst;
+    uint32_t egress_max_rate;
+    uint32_t egress_burst;
 };
 
+static void
+add_column_noalert(struct ovsdb_idl *idl,
+                   const struct ovsdb_idl_column *column)
+{
+    ovsdb_idl_add_column(idl, column);
+    ovsdb_idl_omit_alert(idl, column);
+}
+
 void
 binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -59,16 +72,28 @@  binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_name);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_external_ids);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_status);
+    add_column_noalert(ovs_idl, 
&ovsrec_interface_col_ingress_policing_rate);
+    add_column_noalert(ovs_idl, 
&ovsrec_interface_col_ingress_policing_burst);
+    add_column_noalert(ovs_idl, &ovsrec_port_col_qos);
 
     ovsdb_idl_add_table(ovs_idl, &ovsrec_table_qos);
-    ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
+    add_column_noalert(ovs_idl, &ovsrec_qos_col_external_ids);
+    add_column_noalert(ovs_idl, &ovsrec_qos_col_other_config);
+    add_column_noalert(ovs_idl, &ovsrec_qos_col_type);
+    add_column_noalert(ovs_idl, &ovsrec_qos_col_queues);
+
+    ovsdb_idl_add_table(ovs_idl, &ovsrec_table_queue);
+    add_column_noalert(ovs_idl, &ovsrec_queue_col_external_ids);
+    add_column_noalert(ovs_idl, &ovsrec_queue_col_other_config);
+    add_column_noalert(ovs_idl, &ovsrec_queue_col_dscp);
 }
 
 static void
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
                     struct shash *lport_to_iface,
                     struct sset *local_lports,
-                    struct sset *egress_ifaces)
+                    struct sset *egress_ifaces,
+                    struct shash *lport_to_pport)
 {
     int i;
 
@@ -90,6 +115,7 @@  get_local_iface_ids(const struct ovsrec_bridge *br_int,
 
             if (iface_id && ofport > 0) {
                 shash_add(lport_to_iface, iface_id, iface_rec);
+                shash_add(lport_to_pport, iface_id, port_rec);
                 sset_add(local_lports, iface_id);
             }
 
@@ -163,206 +189,39 @@  add_local_datapath(const struct ldatapath_index 
*ldatapaths,
                          local_datapaths);
 }
 
-static void
-get_qos_params(const struct sbrec_port_binding *pb, struct hmap 
*queue_map)
+static  bool
+get_qos_params(const struct sbrec_port_binding *pb, struct qos_queue 
*queue)
 {
-    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 ingress_max_rate = smap_get_int(&pb->options, 
"qos_ingress_max_rate", 0);
+    uint32_t ingress_burst = smap_get_int(&pb->options, 
"qos_ingress_burst", 0);
+    uint32_t egress_max_rate = smap_get_int(&pb->options, 
"qos_egress_max_rate", 0);
+    uint32_t egress_burst = smap_get_int(&pb->options, 
"qos_egress_burst", 0);
     uint32_t queue_id = smap_get_int(&pb->options, "qdisc_queue_id", 0);
-
-    if ((!max_rate && !burst) || !queue_id) {
+ 
+    if (!queue_id) {
         /* 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->max_rate = max_rate;
-    node->burst = burst;
-    node->queue_id = queue_id;
-}
-
-static const struct ovsrec_qos *
-get_noop_qos(struct controller_ctx *ctx)
-{
-    const struct ovsrec_qos *qos;
-    OVSREC_QOS_FOR_EACH (qos, ctx->ovs_idl) {
-        if (!strcmp(qos->type, "linux-noop")) {
-            return qos;
-        }
-    }
-
-    if (!ctx->ovs_idl_txn) {
-        return NULL;
-    }
-    qos = ovsrec_qos_insert(ctx->ovs_idl_txn);
-    ovsrec_qos_set_type(qos, "linux-noop");
-    return qos;
-}
-
-static bool
-set_noop_qos(struct controller_ctx *ctx, struct sset *egress_ifaces)
-{
-    if (!ctx->ovs_idl_txn) {
-        return false;
-    }
-
-    const struct ovsrec_qos *noop_qos = get_noop_qos(ctx);
-    if (!noop_qos) {
         return false;
     }
 
-    const struct ovsrec_port *port;
-    size_t count = 0;
-
-    OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) {
-        if (sset_contains(egress_ifaces, port->name)) {
-            ovsrec_port_set_qos(port, noop_qos);
-            count++;
-        }
-        if (sset_count(egress_ifaces) == count) {
-            break;
-        }
-    }
+    queue->ingress_max_rate = ingress_max_rate;
+    queue->ingress_burst = ingress_burst;
+    queue->egress_max_rate = egress_max_rate;
+    queue->egress_burst = egress_burst;
+ 
+    queue->queue_id = queue_id;
     return true;
 }
 
 static void
-set_qos_type(struct netdev *netdev, const char *type)
-{
-    int error = netdev_set_qos(netdev, type, NULL);
-    if (error) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-        VLOG_WARN_RL(&rl, "%s: could not set qdisc type \"%s\" (%s)",
-                     netdev_get_name(netdev), type, ovs_strerror(error));
-    }
-}
-
-static void
-setup_qos(const char *egress_iface, struct hmap *queue_map)
-{
-    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
-    struct netdev *netdev_phy;
-
-    if (!egress_iface) {
-        /* Queues cannot be configured. */
-        return;
-    }
-
-    int error = netdev_open(egress_iface, NULL, &netdev_phy);
-    if (error) {
-        VLOG_WARN_RL(&rl, "%s: could not open netdev (%s)",
-                     egress_iface, ovs_strerror(error));
-        return;
-    }
-
-    /* Check current qdisc. */
-    const char *qdisc_type;
-    struct smap qdisc_details;
-
-    smap_init(&qdisc_details);
-    if (netdev_get_qos(netdev_phy, &qdisc_type, &qdisc_details) != 0 ||
-        qdisc_type[0] == '\0') {
-        smap_destroy(&qdisc_details);
-        netdev_close(netdev_phy);
-        /* Qos is not supported. */
-        return;
-    }
-    smap_destroy(&qdisc_details);
-
-    /* If we're not actually being requested to do any QoS:
-     *
-     *     - If the current qdisc type is OVN_QOS_TYPE, then we clear the 
qdisc
-     *       type to "".  Otherwise, it's possible that our own leftover 
qdisc
-     *       settings could cause strange behavior on egress.  Also, QoS 
is
-     *       expensive and may waste CPU time even if it's not really in 
use.
-     *
-     *       OVN isn't the only software that can configure qdiscs, and
-     *       physical interfaces are shared resources, so there is some 
risk in
-     *       this strategy: we could disrupt some other program's QoS.
-     *       Probably, to entirely avoid this possibility we would need 
to add
-     *       a configuration setting.
-     *
-     *     - Otherwise leave the qdisc alone. */
-    if (hmap_is_empty(queue_map)) {
-        if (!strcmp(qdisc_type, OVN_QOS_TYPE)) {
-            set_qos_type(netdev_phy, "");
-        }
-        netdev_close(netdev_phy);
-        return;
-    }
-
-    /* Configure qdisc. */
-    if (strcmp(qdisc_type, OVN_QOS_TYPE)) {
-        set_qos_type(netdev_phy, OVN_QOS_TYPE);
-    }
-
-    /* Check and delete if needed. */
-    struct netdev_queue_dump dump;
-    unsigned int queue_id;
-    struct smap queue_details;
-    struct qos_queue *sb_info;
-    struct hmap consistent_queues;
-
-    smap_init(&queue_details);
-    hmap_init(&consistent_queues);
-    NETDEV_QUEUE_FOR_EACH (&queue_id, &queue_details, &dump, netdev_phy) 
{
-        bool is_queue_needed = false;
-
-        HMAP_FOR_EACH_WITH_HASH (sb_info, node, hash_int(queue_id, 0),
-                                 queue_map) {
-            is_queue_needed = true;
-            if (sb_info->max_rate ==
-                smap_get_int(&queue_details, "max-rate", 0)
-                && sb_info->burst == smap_get_int(&queue_details, 
"burst", 0)) {
-                /* This queue is consistent. */
-                hmap_insert(&consistent_queues, &sb_info->node,
-                            hash_int(queue_id, 0));
-                break;
-            }
-        }
-
-        if (!is_queue_needed) {
-            error = netdev_delete_queue(netdev_phy, queue_id);
-            if (error) {
-                VLOG_WARN_RL(&rl, "%s: could not delete queue %u (%s)",
-                             egress_iface, queue_id, 
ovs_strerror(error));
-            }
-        }
-    }
-
-    /* Create/Update queues. */
-    HMAP_FOR_EACH (sb_info, node, queue_map) {
-        if (hmap_contains(&consistent_queues, &sb_info->node)) {
-            hmap_remove(&consistent_queues, &sb_info->node);
-            continue;
-        }
-
-        smap_clear(&queue_details);
-        smap_add_format(&queue_details, "max-rate", "%d", 
sb_info->max_rate);
-        smap_add_format(&queue_details, "burst", "%d", sb_info->burst);
-        error = netdev_set_queue(netdev_phy, sb_info->queue_id,
-                                 &queue_details);
-        if (error) {
-            VLOG_WARN_RL(&rl, "%s: could not configure queue %u (%s)",
-                         egress_iface, sb_info->queue_id, 
ovs_strerror(error));
-        }
-    }
-    smap_destroy(&queue_details);
-    hmap_destroy(&consistent_queues);
-    netdev_close(netdev_phy);
-}
-
-static void
 consider_local_datapath(struct controller_ctx *ctx,
                         const struct ldatapath_index *ldatapaths,
                         const struct lport_index *lports,
                         const struct sbrec_chassis *chassis_rec,
                         const struct sbrec_port_binding *binding_rec,
-                        struct hmap *qos_map,
                         struct hmap *local_datapaths,
                         struct shash *lport_to_iface,
-                        struct sset *local_lports)
+                        struct sset *local_lports,
+                        struct shash *lport_to_pport)
 {
     const struct ovsrec_interface *iface_rec
         = shash_find_data(lport_to_iface, binding_rec->logical_port);
@@ -374,12 +233,15 @@  consider_local_datapath(struct controller_ctx *ctx,
         if (binding_rec->parent_port && binding_rec->parent_port[0]) {
             /* Add child logical port to the set of all local ports. */
             sset_add(local_lports, binding_rec->logical_port);
+            const struct  ovsrec_port *port_rec =
+                shash_find_data(lport_to_pport, 
binding_rec->parent_port);
+            if (port_rec) {
+                VLOG_INFO("port_rec exist, port-name is %s", 
binding_rec->logical_port);
+                shash_add(lport_to_pport, binding_rec->logical_port, 
port_rec);
+            }
         }
         add_local_datapath(ldatapaths, lports, binding_rec->datapath,
                            false, local_datapaths);
-        if (iface_rec && qos_map && ctx->ovs_idl_txn) {
-            get_qos_params(binding_rec, qos_map);
-        }
         /* This port is in our chassis unless it is a localport. */
            if (strcmp(binding_rec->type, "localport")) {
             our_chassis = true;
@@ -442,6 +304,266 @@  consider_local_datapath(struct controller_ctx *ctx,
     }
 }
 
+static void
+del_qos(const struct ovsrec_qos *qos)
+{
+    int n_queue;
+    struct shash queues = SHASH_INITIALIZER(&queues);
+    char uuid_s[UUID_LEN + 1];
+    for (n_queue = 0; n_queue< qos->n_queues; n_queue++) {
+        struct ovsrec_queue *queue = qos->value_queues[n_queue];
+        sprintf(uuid_s, UUID_FMT, UUID_ARGS(&queue->header_.uuid));
+        shash_add(&queues, uuid_s, queue);
+    }
+    ovsrec_qos_delete(qos);
+ 
+    struct shash_node *node, *next_node;
+    SHASH_FOR_EACH_SAFE(node, next_node, &queues) {
+        ovsrec_queue_delete(node->data);
+    }
+}
+
+static void
+clear_port_qos(struct ovsrec_port *port_rec,
+               const struct sbrec_port_binding *binding_rec)
+{
+    if (IS_SUB_PORT(binding_rec) && port_rec->qos) {
+        int n_queue;
+        for (n_queue = 0; n_queue< port_rec->qos->n_queues; n_queue++) {
+            struct ovsrec_queue *queue = 
port_rec->qos->value_queues[n_queue];
+            const char *port_id = smap_get(&queue->external_ids, 
"port_id");
+            if (!strcmp(binding_rec->logical_port, port_id)) {
+                ovsrec_qos_update_queues_delkey(port_rec->qos,
+ port_rec->qos->key_queues[n_queue]);
+                ovsrec_queue_delete(queue);
+                break;
+            }
+        }
+    } else if (port_rec->qos) {
+        if (port_rec->qos) {
+            struct ovsrec_qos *qos = port_rec->qos;
+            ovsrec_port_set_qos(port_rec, NULL);
+            del_qos(qos);
+        }
+ 
+        for (int j = 0; j < port_rec->n_interfaces; j++) {
+            struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
+            const char *iface_id = smap_get(&iface_rec->external_ids,
+                                                "iface-id");
+            if (iface_id && !strcmp(binding_rec->logical_port, iface_id)) 
{
+                if (iface_rec->ingress_policing_burst != 0) {
+ ovsrec_interface_set_ingress_policing_burst(iface_rec,
+                                          0);
+                }
+                if (iface_rec->ingress_policing_rate != 0) {
+                    ovsrec_interface_set_ingress_policing_rate(iface_rec,
+                                          0);
+                }
+            }
+        }
+    }
+}
+
+static char *
+get_port_id(const struct sbrec_port_binding *binding_rec)
+{
+    if (IS_SUB_PORT(binding_rec)) {
+        return binding_rec->parent_port;
+    } else {
+        return binding_rec->logical_port;
+    }
+}
+
+static void
+set_interface_param( struct ovsrec_port *port_rec,
+                           struct qos_queue *qos_param,
+                           const struct sbrec_port_binding *binding_rec)
+{
+    for (int j = 0; j < port_rec->n_interfaces; j++) {
+        /* configure interface  policing burst and policing rate*/
+        struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
+        const char *iface_id = smap_get(&iface_rec->external_ids,
+                                            "iface-id");
+        if (iface_id && !strcmp(binding_rec->logical_port, iface_id)) {
+            if(qos_param->ingress_burst
+                != iface_rec->ingress_policing_burst) {
+                ovsrec_interface_set_ingress_policing_burst(iface_rec,
+ (int64_t)qos_param->ingress_burst);
+            }
+
+            if(qos_param->ingress_max_rate
+                != iface_rec->ingress_policing_rate) {
+                ovsrec_interface_set_ingress_policing_rate(iface_rec,
+ (int64_t)qos_param->ingress_max_rate);
+            }
+            break;
+        }
+    }
+}
+
+static const struct ovsrec_queue *
+get_queue_by_port(struct controller_ctx *ctx,
+                              const char *port_name)
+{
+    const struct ovsrec_queue *queue, *next_queue;
+    OVSREC_QUEUE_FOR_EACH_SAFE(queue, next_queue, ctx->ovs_idl) {
+        const char *port_id =  smap_get(&queue->external_ids, "port_id");
+        if (port_id && !strcmp(port_name, port_id)) {
+            return queue;
+        }
+    }
+    struct ovsrec_queue *newqueue = 
+        ovsrec_queue_insert(ctx->ovs_idl_txn);
+    ovsrec_queue_update_external_ids_setkey(newqueue, "port_id",
+                                   port_name);
+    return newqueue;
+ 
+}
+
+static void
+set_queue(struct controller_ctx *ctx,
+          const struct ovsrec_qos *qos,
+          struct qos_queue *qos_param,
+          const struct sbrec_port_binding *binding_rec)
+{
+    int n_queue;
+    for (n_queue = 0; n_queue< qos->n_queues; n_queue++) {
+        struct ovsrec_queue *queue = qos->value_queues[n_queue];
+        const char *port_id = smap_get(&queue->external_ids, "port_id");
+        if (!strcmp(port_id, binding_rec->logical_port)) {
+            struct smap other_config;
+            smap_init(&other_config);
+            smap_add_format(&other_config, "max-rate", "%d",
+                            qos_param->egress_max_rate*1000);
+            smap_add_format(&other_config, "burst", "%d",
+                            qos_param->egress_burst*1000);
+            ovsrec_queue_set_other_config(queue, &other_config);
+            return;
+        }
+    }
+
+    const struct ovsrec_queue * queue = 
+        get_queue_by_port(ctx, binding_rec->logical_port);
+    struct smap other_config;
+    smap_init(&other_config);
+    smap_add_format(&other_config, "max-rate", "%d",
+                    qos_param->egress_max_rate);
+    smap_add_format(&other_config, "burst", "%d",
+                    qos_param->egress_burst);
+    ovsrec_queue_set_other_config(queue, &other_config);
+    ovsrec_qos_update_queues_setkey(qos,
+                                    (int64_t)qos_param->queue_id,
+                                    queue);
+
+}
+
+static void
+set_qos_para(const struct ovsrec_qos *qos, 
+                           const struct qos_queue *qos_param,
+                           const char *dptype)
+{
+    if (!strcmp(dptype, "netdev")) {
+        ovsrec_qos_set_type(qos, "egress-policer");
+        struct smap other_config;
+        smap_init(&other_config);
+        smap_add_format(&other_config, "cir", "%d",
+                        qos_param->egress_max_rate*1000/8);
+        smap_add_format(&other_config, "cbs", "%d",
+                        qos_param->egress_burst*1000/8);
+        ovsrec_qos_set_other_config(qos, &other_config);
+    } else {
+        ovsrec_qos_set_type(qos, "linux-htb");
+    }
+}
+
+
+static const struct ovsrec_qos *
+get_qos_by_port(struct controller_ctx *ctx,
+                                  const char *port_name) 
+{
+    const struct ovsrec_qos *qos, *next_qos;
+    OVSREC_QOS_FOR_EACH_SAFE(qos, next_qos, ctx->ovs_idl) {
+        const char *port_id =  smap_get(&qos->external_ids, "port_id");
+        if (port_id && !strcmp(port_name, port_id)) {
+            return qos;
+        }
+    } 
+    struct ovsrec_qos *qosnew = ovsrec_qos_insert(ctx->ovs_idl_txn);
+     /* set port_id to external_id of qos */
+    struct smap new_ids;
+    smap_clone(&new_ids, &qosnew->external_ids);
+    smap_add(&new_ids, "port_id", port_name);
+    ovsrec_qos_set_external_ids(qosnew, &new_ids);
+    smap_destroy(&new_ids);
+    return qosnew;
+}
+ 
+static void
+setup_qos(struct controller_ctx *ctx,
+          const struct shash *lport_to_pport,
+          const char *dptype)
+{
+ 
+    struct qos_queue qos_param;
+    struct sset exist_pb = SSET_INITIALIZER(&exist_pb);
+    if (!ctx->ovs_idl_txn) {
+        return;
+    }
+    /* add queue and qos */
+    const struct sbrec_port_binding *binding_rec, *next_bind;
+    SBREC_PORT_BINDING_FOR_EACH_SAFE(binding_rec, next_bind, 
ctx->ovnsb_idl) {
+        sset_add(&exist_pb, binding_rec->logical_port);
+ 
+        struct ovsrec_port *port_rec = shash_find_data(lport_to_pport,
+ binding_rec->logical_port);
+        if (!port_rec) {
+            continue;
+        }
+        if (!get_qos_params(binding_rec, &qos_param)) {
+            clear_port_qos(port_rec, binding_rec);
+            continue;
+        }
+        if (port_rec->qos) {
+            if (!IS_SUB_PORT(binding_rec)) {
+                /* not sub port */
+                set_interface_param(port_rec, &qos_param,
+                                    binding_rec);
+            }
+            set_qos_para(port_rec->qos, &qos_param, dptype);
+            set_queue(ctx, port_rec->qos, &qos_param, binding_rec);
+        } else {
+            char *port_name = get_port_id(binding_rec);
+            const struct ovsrec_qos *qos = get_qos_by_port(ctx, 
port_name);
+            set_qos_para(qos, &qos_param, dptype);
+            ovsrec_port_set_qos(port_rec, qos);
+            if (!IS_SUB_PORT(binding_rec)) {
+                /* not sub port */
+                set_interface_param(port_rec, &qos_param, binding_rec);
+            }
+            set_queue(ctx, qos, &qos_param, binding_rec);
+        }
+    }
+    /* delete not used qos */
+    const struct ovsrec_qos *qos, *next_qos;
+    OVSREC_QOS_FOR_EACH_SAFE(qos, next_qos, ctx->ovs_idl) {
+        const char *port_id =  smap_get(&qos->external_ids, "port_id");
+        if (port_id && !shash_find_data(lport_to_pport, port_id)) {
+            del_qos(qos);
+        }
+    }
+
+    /* delete not used queue */
+    const struct ovsrec_queue *queue, *next_queue;
+    OVSREC_QUEUE_FOR_EACH_SAFE(queue, next_queue, ctx->ovs_idl) {
+        const char *port_id =  smap_get(&queue->external_ids, "port_id");
+        if (port_id && !sset_contains(&exist_pb, port_id)) {
+            ovsrec_queue_delete(queue);
+        }
+    }
+    sset_destroy(&exist_pb);
+ 
+}
+
 void
 binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge 
*br_int,
             const struct sbrec_chassis *chassis_rec,
@@ -455,13 +577,11 @@  binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 
     const struct sbrec_port_binding *binding_rec;
     struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
+    struct shash lport_to_pport = SHASH_INITIALIZER(&lport_to_pport);
     struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
-    struct hmap qos_map;
-
-    hmap_init(&qos_map);
     if (br_int) {
         get_local_iface_ids(br_int, &lport_to_iface, local_lports,
-                            &egress_ifaces);
+                            &egress_ifaces, &lport_to_pport);
     }
 
     /* Run through each binding record to see if it is resident on this
@@ -470,23 +590,16 @@  binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
     SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
         consider_local_datapath(ctx, ldatapaths, lports,
                                 chassis_rec, binding_rec,
-                                sset_is_empty(&egress_ifaces) ? NULL :
-                                &qos_map, local_datapaths, 
&lport_to_iface,
-                                local_lports);
+                                local_datapaths, &lport_to_iface,
+                                local_lports, &lport_to_pport);
 
     }
 
-    if (!sset_is_empty(&egress_ifaces)
-        && set_noop_qos(ctx, &egress_ifaces)) {
-        const char *entry;
-        SSET_FOR_EACH (entry, &egress_ifaces) {
-            setup_qos(entry, &qos_map);
-        }
-    }
+    setup_qos(ctx, &lport_to_pport, br_int->datapath_type);
 
     shash_destroy(&lport_to_iface);
+    shash_destroy(&lport_to_pport);
     sset_destroy(&egress_ifaces);
-    hmap_destroy(&qos_map);
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index be3b371..c683953 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1,4 +1,4 @@ 
-/*
+/*
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at:
@@ -371,8 +371,10 @@  free_chassis_queueid(struct hmap *set, struct 
sbrec_chassis *chassis,
 static inline bool
 port_has_qos_params(const struct smap *opts)
 {
-    return (smap_get(opts, "qos_max_rate") ||
-            smap_get(opts, "qos_burst"));
+    return (smap_get(opts, "qos_ingress_max_rate") ||
+            smap_get(opts, "qos_ingress_burst") ||
+            smap_get(opts, "qos_egress_max_rate") ||
+            smap_get(opts, "qos_egress_burst"));
 }
 
 
@@ -3715,12 +3717,19 @@  build_lswitch_flows(struct hmap *datapaths, struct 
hmap *ports,
         }
 
         ds_clear(&match);
+        ds_clear(&actions);
         ds_put_format(&match, "outport == %s", op->json_key);
         if (lsp_is_enabled(op->nbsp)) {
             build_port_security_l2("eth.dst", op->ps_addrs, 
op->n_ps_addrs,
                                    &match);
+            const char *queue_id = smap_get(&op->sb->options,
+                                            "qdisc_queue_id");
+            if (queue_id) {
+                ds_put_format(&actions, "set_queue(%s); ", queue_id);
+            }
+            ds_put_cstr(&actions, "output;");
             ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 50,
-                          ds_cstr(&match), "output;");
+                          ds_cstr(&match), ds_cstr(&actions));
         } else {
             ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 150,
                           ds_cstr(&match), "drop;");
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 32c10c1..56229e9 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -435,14 +435,24 @@ 
           (empty string)
         </p>
 
-        <column name="options" key="qos_max_rate">
+        <column name="options" key="qos_ingress_max_rate">
           If set, indicates the maximum rate for data sent from this 
interface,
-          in bit/s. The traffic will be shaped according to this limit.
+          in kbps. The traffic will be shaped according to this limit.
         </column>
 
-        <column name="options" key="qos_burst">
+        <column name="options" key="qos_ingress_burst">
           If set, indicates the maximum burst size for data sent from 
this
-          interface, in bits.
+          interface, in kb.
+        </column>
+
+        <column name="options" key="qos_egress_max_rate">
+          If set, indicates the maximum rate for data sent to this 
interface,
+          in kbps. The traffic will be shaped according to this limit.
+        </column>
+
+        <column name="options" key="qos_egress_burst">
+          If set, indicates the maximum burst size for data sent to this
+          interface, in kb.
         </column>
       </group>
     </group>
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index b22d1ac..c8b940d 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -2103,14 +2103,24 @@  tcp.flags = RST;
         (empty string)
       </p>
 
-      <column name="options" key="qos_max_rate">
+      <column name="options" key="qos_ingress_max_rate">
         If set, indicates the maximum rate for data sent from this 
interface,
-        in bit/s. The traffic will be shaped according to this limit.
+        in kbps. The traffic will be shaped according to this limit.
       </column>
 
-      <column name="options" key="qos_burst">
+      <column name="options" key="qos_ingress_burst">
         If set, indicates the maximum burst size for data sent from this
-        interface, in bits.
+        interface, in kb.
+      </column>
+
+      <column name="options" key="qos_egress_max_rate">
+        If set, indicates the maximum rate for data sent to this 
interface,
+        in kbps. The traffic will be shaped according to this limit.
+      </column>
+
+      <column name="options" key="qos_egress_burst">
+        If set, indicates the maximum burst size for data sent to this
+        interface, in kb.
       </column>