> 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.maxim...@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?

> +    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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to