Hi Gao

How about the following incremental ?

Thanks Darrell



diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4808d38..648d719 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1843,64 +1843,58 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct 
dp_packet_batch *batch)
 #endif
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
     struct rte_mbuf *pkts[PKT_ARRAY_SIZE];
-    int txcnt_eth = batch->count;
-    int dropped = 0;
-    int newcnt = 0;
-    int i;
+    uint32_t cnt = batch->count;
+    uint32_t dropped = 0;
 
     if (dev->type != DPDK_DEV_VHOST) {
         /* Check if QoS has been configured for this netdev. */
-        txcnt_eth = netdev_dpdk_qos_run(dev,
-                                        (struct rte_mbuf **)batch->packets,
-                                        txcnt_eth, false);
-        if (txcnt_eth == 0) {
-            dropped = batch->count;
-            goto out;
-        }
+        cnt = netdev_dpdk_qos_run(dev, (struct rte_mbuf **) batch->packets,
+                                  cnt, false);
+        dropped += batch->count - cnt;
     }
 
     dp_packet_batch_apply_cutlen(batch);
 
-    for (i = 0; i < batch->count; i++) {
-        int size = dp_packet_size(batch->packets[i]);
+    uint32_t txcnt = 0;
+
+    for (uint32_t i = 0; i < cnt; i++) {
+
+        uint32_t size = dp_packet_size(batch->packets[i]);
 
         if (OVS_UNLIKELY(size > dev->max_packet_len)) {
-            VLOG_WARN_RL(&rl, "Too big size %d max_packet_len %d",
-                         (int) size, dev->max_packet_len);
+            VLOG_WARN_RL(&rl, "Too big size %u max_packet_len %d",
+                         size, dev->max_packet_len);
 
             dropped++;
             continue;
         }
 
-        pkts[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
+        pkts[txcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
 
-        if (!pkts[newcnt]) {
-            dropped += batch->count - i;
+        if (!pkts[txcnt]) {
+            dropped += cnt - i;
             break;
         }
 
         /* We have to do a copy for now */
-        memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),
+        memcpy(rte_pktmbuf_mtod(pkts[txcnt], void *),
                dp_packet_data(batch->packets[i]), size);
 
-        rte_pktmbuf_data_len(pkts[newcnt]) = size;
-        rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
+        rte_pktmbuf_data_len(pkts[txcnt]) = size;
+        rte_pktmbuf_pkt_len(pkts[txcnt]) = size;
 
-        newcnt++;
-        if (dev->type != DPDK_DEV_VHOST && newcnt >= txcnt_eth) {
-            dropped += batch->count - i - 1;
-            break;
-        }
+        txcnt++;
     }
 
-    if (dev->type == DPDK_DEV_VHOST) {
-        __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
-                                 newcnt);
-    } else {
-        dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, newcnt);
+    if (OVS_LIKELY(txcnt)) {
+        if (dev->type == DPDK_DEV_VHOST) {
+            __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
+                                     txcnt);
+        } else {
+            dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt);
+        }
     }
 
-out:
     if (OVS_UNLIKELY(dropped)) {
         rte_spinlock_lock(&dev->stats_lock);
         dev->stats.tx_dropped += dropped;


//////////////////////////////////////////////////////////////


On 8/29/17, 4:39 AM, "ovs-dev-boun...@openvswitch.org on behalf of Zhenyu Gao" 
<ovs-dev-boun...@openvswitch.org on behalf of sysugaozhe...@gmail.com> wrote:

    In dpdk_do_tx_copy function, all packets were copied to mbuf first,
    but QoS checking may drop some of them.
    Move the QoS checking in front of copying data to mbuf, it helps to
    reduce useless copy.
    
    Signed-off-by: Zhenyu Gao <sysugaozhe...@gmail.com>
    ---
     lib/netdev-dpdk.c | 55 
++++++++++++++++++++++++++++++++++++-------------------
     1 file changed, 36 insertions(+), 19 deletions(-)
    
    diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    index 1aaf6f7..50874b8 100644
    --- a/lib/netdev-dpdk.c
    +++ b/lib/netdev-dpdk.c
    @@ -279,7 +279,7 @@ struct dpdk_qos_ops {
          * For all QoS implementations it should always be non-null.
          */
         int (*qos_run)(struct qos_conf *qos_conf, struct rte_mbuf **pkts,
    -                   int pkt_cnt);
    +                   int pkt_cnt, bool may_steal);
     };
     
     /* dpdk_qos_ops for each type of user space QoS implementation */
    @@ -1501,7 +1501,8 @@ netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm 
*meter,
     
     static int
     netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
    -                        struct rte_mbuf **pkts, int pkt_cnt)
    +                        struct rte_mbuf **pkts, int pkt_cnt,
    +                        bool may_steal)
     {
         int i = 0;
         int cnt = 0;
    @@ -1517,7 +1518,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
                 }
                 cnt++;
             } else {
    -            rte_pktmbuf_free(pkt);
    +            if (may_steal) {
    +                rte_pktmbuf_free(pkt);
    +            }
             }
         }
     
    @@ -1526,12 +1529,13 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm 
*meter,
     
     static int
     ingress_policer_run(struct ingress_policer *policer, struct rte_mbuf 
**pkts,
    -                    int pkt_cnt)
    +                    int pkt_cnt, bool may_steal)
     {
         int cnt = 0;
     
         rte_spinlock_lock(&policer->policer_lock);
    -    cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, pkt_cnt);
    +    cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts,
    +                                  pkt_cnt, may_steal);
         rte_spinlock_unlock(&policer->policer_lock);
     
         return cnt;
    @@ -1635,7 +1639,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
             dropped = nb_rx;
             nb_rx = ingress_policer_run(policer,
                                         (struct rte_mbuf **) batch->packets,
    -                                    nb_rx);
    +                                    nb_rx, true);
             dropped -= nb_rx;
         }
     
    @@ -1672,7 +1676,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct 
dp_packet_batch *batch)
             dropped = nb_rx;
             nb_rx = ingress_policer_run(policer,
                                         (struct rte_mbuf **) batch->packets,
    -                                    nb_rx);
    +                                    nb_rx, true);
             dropped -= nb_rx;
         }
     
    @@ -1690,13 +1694,13 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct 
dp_packet_batch *batch)
     
     static inline int
     netdev_dpdk_qos_run(struct netdev_dpdk *dev, struct rte_mbuf **pkts,
    -                    int cnt)
    +                    int cnt, bool may_steal)
     {
         struct qos_conf *qos_conf = ovsrcu_get(struct qos_conf *, 
&dev->qos_conf);
     
         if (qos_conf) {
             rte_spinlock_lock(&qos_conf->lock);
    -        cnt = qos_conf->ops->qos_run(qos_conf, pkts, cnt);
    +        cnt = qos_conf->ops->qos_run(qos_conf, pkts, cnt, may_steal);
             rte_spinlock_unlock(&qos_conf->lock);
         }
     
    @@ -1770,7 +1774,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int 
qid,
     
         cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt);
         /* Check has QoS has been configured for the netdev */
    -    cnt = netdev_dpdk_qos_run(dev, cur_pkts, cnt);
    +    cnt = netdev_dpdk_qos_run(dev, cur_pkts, cnt, true);
         dropped = total_pkts - cnt;
     
         do {
    @@ -1816,10 +1820,22 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, 
struct dp_packet_batch *batch)
     #endif
         struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
         struct rte_mbuf *pkts[PKT_ARRAY_SIZE];
    +    int txcnt_eth = batch->count;
         int dropped = 0;
         int newcnt = 0;
         int i;
     
    +    if (dev->type != DPDK_DEV_VHOST) {
    +        /* Check if QoS has been configured for this netdev. */
    +        txcnt_eth = netdev_dpdk_qos_run(dev,
    +                                        (struct rte_mbuf **)batch->packets,
    +                                        txcnt_eth, false);
    +        if (txcnt_eth == 0) {
    +            dropped = batch->count;
    +            goto out;
    +        }
    +    }
    +
         dp_packet_batch_apply_cutlen(batch);
     
         for (i = 0; i < batch->count; i++) {
    @@ -1848,21 +1864,20 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, 
struct dp_packet_batch *batch)
             rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
     
             newcnt++;
    +        if (dev->type != DPDK_DEV_VHOST && newcnt >= txcnt_eth) {
    +            dropped += batch->count - i - 1;
    +            break;
    +        }
         }
     
         if (dev->type == DPDK_DEV_VHOST) {
             __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
                                      newcnt);
         } else {
    -        unsigned int qos_pkts = newcnt;
    -
    -        /* Check if QoS has been configured for this netdev. */
    -        newcnt = netdev_dpdk_qos_run(dev, pkts, newcnt);
    -
    -        dropped += qos_pkts - newcnt;
             dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, newcnt);
         }
     
    +out:
         if (OVS_UNLIKELY(dropped)) {
             rte_spinlock_lock(&dev->stats_lock);
             dev->stats.tx_dropped += dropped;
    @@ -1915,7 +1930,7 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
             dp_packet_batch_apply_cutlen(batch);
     
             cnt = netdev_dpdk_filter_packet_len(dev, pkts, cnt);
    -        cnt = netdev_dpdk_qos_run(dev, pkts, cnt);
    +        cnt = netdev_dpdk_qos_run(dev, pkts, cnt, true);
             dropped = batch->count - cnt;
     
             dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, cnt);
    @@ -3132,13 +3147,15 @@ egress_policer_qos_is_equal(const struct qos_conf 
*conf,
     }
     
     static int
    -egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int 
pkt_cnt)
    +egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int 
pkt_cnt,
    +                   bool may_steal)
     {
         int cnt = 0;
         struct egress_policer *policer =
             CONTAINER_OF(conf, struct egress_policer, qos_conf);
     
    -    cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, pkt_cnt);
    +    cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts,
    +                                  pkt_cnt, may_steal);
     
         return cnt;
     }
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    d...@openvswitch.org
    
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=EZzkW98aZhxGnYEEmB3u5e3vaFA90qSr3aIM1t-ggFY&s=fHRBBnkb29ujBAr27aGj1MCWkVzjfKqmJz3R9wEg99o&e=
 
    



_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to