[ovs-dev] [PATCH 1/2] netdev-dpdk: Fix vHost stats.

Ilya Maximets i.maximets at samsung.com
Thu Aug 18 13:12:34 UTC 2016



On 18.08.2016 15:25, Stokes, Ian wrote:
>> This patch introduces function 'netdev_dpdk_filter_packet_len()' which is
>> intended to find and remove all packets with 'pkt_len > max_packet_len'
>> from the Tx batch.
>>
>> It fixes inaccurate counting of 'tx_bytes' in vHost case if there was
>> dropped packets and allows to simplify send function.
>>
> 
> Thanks for the patch Ilya, minor comment below.
> 
>> Fixes: 0072e931b207 ("netdev-dpdk: add support for jumbo frames")
>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>> ---
>>  lib/netdev-dpdk.c | 52 +++++++++++++++++++++++++++++---------------------
>> --
>>  1 file changed, 29 insertions(+), 23 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index e5f2cdd..bd374d0
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1435,6 +1435,28 @@ netdev_dpdk_qos_run__(struct netdev_dpdk *dev,
>> struct rte_mbuf **pkts,
>>      return cnt;
>>  }
>>
>> +static int
>> +netdev_dpdk_filter_packet_len(struct netdev_dpdk *dev, struct rte_mbuf
>> **pkts,
>> +                              int pkt_cnt) {
>> +    int i = 0;
>> +    int cnt = 0;
>> +    struct rte_mbuf *pkt;
>> +
>> +    for (i = 0; i < pkt_cnt; i++) {
>> +        pkt = pkts[i];
>> +        if (OVS_UNLIKELY(pkt->pkt_len > dev->max_packet_len)) {
>> +            VLOG_WARN_RL(&rl, "%s: Too big size %" PRIu32 "
>> max_packet_len %d",
>> +                         dev->up.name, pkt->pkt_len, dev-
>>> max_packet_len);
>> +            rte_pktmbuf_free(pkt);
>> +        } else if (i != cnt++) {
>> +            pkts[cnt - 1] = pkt;
>> +        }
>> +    }
>> +
>> +    return cnt;
>> +}
>> +
>>  static inline void
>>  netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats,
>>                                       struct dp_packet **packets, @@ -
>> 1459,8 +1481,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>      struct rte_mbuf **cur_pkts = (struct rte_mbuf **) pkts;
>>      unsigned int total_pkts = cnt;
>> -    unsigned int qos_pkts = 0;
>> -    unsigned int mtu_dropped = 0;
>> +    unsigned int dropped = 0;
>>      int i, retries = 0;
>>
>>      qid = dev->tx_q[qid % netdev->n_txq].map; @@ -1477,50 +1498,35 @@
>> __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>
>>      /* Check has QoS has been configured for the netdev */
>>      cnt = netdev_dpdk_qos_run__(dev, cur_pkts, cnt);
>> -    qos_pkts = total_pkts - cnt;
> 
> I think it would be better to call netdev_dpdk_filter_packet_len() before netdev_dpdk_qos_run__ above.
> If the packet is too large for the netdev it will be dropped inevitably, we should avoid the overhead of QoS before dropping it, thoughts?

Sounds reasonable. I'll send v2.

>> +    cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt);
>> +    dropped = total_pkts - cnt;
>>
>>      do {
>>          int vhost_qid = qid * VIRTIO_QNUM + VIRTIO_RXQ;
>>          unsigned int tx_pkts;
>> -        unsigned int try_tx_pkts = cnt;
>>
>> -        for (i = 0; i < cnt; i++) {
>> -            if (cur_pkts[i]->pkt_len > dev->max_packet_len) {
>> -                try_tx_pkts = i;
>> -                break;
>> -            }
>> -        }
>> -        if (!try_tx_pkts) {
>> -            cur_pkts++;
>> -            mtu_dropped++;
>> -            cnt--;
>> -            continue;
>> -        }
>>          tx_pkts = rte_vhost_enqueue_burst(netdev_dpdk_get_vid(dev),
>> -                                          vhost_qid, cur_pkts,
>> try_tx_pkts);
>> +                                          vhost_qid, cur_pkts, cnt);
>>          if (OVS_LIKELY(tx_pkts)) {
>>              /* Packets have been sent.*/
>>              cnt -= tx_pkts;
>>              /* Prepare for possible retry.*/
>>              cur_pkts = &cur_pkts[tx_pkts];
>> -            if (tx_pkts != try_tx_pkts) {
>> -                retries++;
>> -            }
>>          } else {
>>              /* No packets sent - do not retry.*/
>>              break;
>>          }
>> -    } while (cnt && (retries <= VHOST_ENQ_RETRY_NUM));
>> +    } while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM));
>>
>>      rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
>>
>>      rte_spinlock_lock(&dev->stats_lock);
>>      netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts,
>> -                                         cnt + mtu_dropped + qos_pkts);
>> +                                         cnt + dropped);
>>      rte_spinlock_unlock(&dev->stats_lock);
>>
>>  out:
>> -    for (i = 0; i < total_pkts - qos_pkts; i++) {
>> +    for (i = 0; i < total_pkts - dropped; i++) {
>>          dp_packet_delete(pkts[i]);
>>      }
>>  }
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list