[ovs-dev] [PATCH] netdev-dpdk: Use instant sending instead of queueing of packets.

Daniele Di Proietto diproiettod at ovn.org
Mon Nov 14 18:15:23 UTC 2016


Hi Cascardo,

I agree, the tx queueing mechanism in 2.5 never really worked because it
made wrong assumptions about the rxq distribution and introduced latency
(at least), as you point out.

Thanks for doing the backport, since it was non trivial.

Usually when we do a backport we use the commit message (including the
tags) from the original commit.  Would you mind doing that?  You can append
the numbers you found as well.

The patch looks good to me, but I still see a compiler warning below.

This might introduce some throughput degradation in certain usecases, but I
think it's more important to fix the broken logic.

Would you mind sending a v2?  I'll be happy to apply it

Thanks,

Daniele

2016-10-24 12:01 GMT-07:00 Thadeu Lima de Souza Cascardo <
cascardo at redhat.com>:

> Backport commit b59cc14e032da370021794bfd1bdd3d67e88a9a3 from master.
>
> This improves latency compared to current openvswitch 2.5.
>
> Without the patch:
>
> Device 0->1:
>   Throughput: 6.8683 Mpps
>   Min. Latency: 39.9780 usec
>   Avg. Latency: 61.1226 usec
>   Max. Latency: 89.1110 usec
>
> Device 1->0:
>   Throughput: 6.8683 Mpps
>   Min. Latency: 41.0660 usec
>   Avg. Latency: 58.7778 usec
>   Max. Latency: 89.4720 usec
>
> With the patch:
>
> Device 0->1:
>   Throughput: 6.3941 Mpps
>   Min. Latency: 10.5410 usec
>   Avg. Latency: 14.1309 usec
>   Max. Latency: 28.9880 usec
>
> Device 1->0:
>   Throughput: 6.3941 Mpps
>   Min. Latency: 11.9780 usec
>   Avg. Latency: 18.0692 usec
>   Max. Latency: 29.5200
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
> Cc: Ilya Maximets <i.maximets at samsung.com>
> Cc: Daniele Di Proietto <diproiettod at vmware.com>
> ---
>  lib/netdev-dpdk.c | 102 ++++++++----------------------
> ------------------------
>  1 file changed, 14 insertions(+), 88 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6e68c07..cbe361a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -131,7 +131,6 @@ static const struct rte_eth_conf port_conf = {
>      },
>  };
>
>
>
[...]

@@ -969,32 +948,18 @@ dpdk_queue_flush__(struct netdev_dpdk *dev, int qid)
>          nb_tx += ret;
>      }
>
> -    if (OVS_UNLIKELY(nb_tx != txq->count)) {
> +    if (OVS_UNLIKELY(nb_tx != cnt)) {
>          /* free buffers, which we couldn't transmit, one at a time (each
>           * packet could come from a different mempool) */
>          int i;
>
> -        for (i = nb_tx; i < txq->count; i++) {
> -            rte_pktmbuf_free(txq->burst_pkts[i]);
> +        for (i = nb_tx; i < cnt; i++) {
> +            rte_pktmbuf_free(pkts[i]);
>          }
>          rte_spinlock_lock(&dev->stats_lock);
> -        dev->stats.tx_dropped += txq->count-nb_tx;
> +        dev->stats.tx_dropped += cnt - nb_tx;
>          rte_spinlock_unlock(&dev->stats_lock);
>      }
> -
> -    txq->count = 0;
> -    txq->tsc = rte_get_timer_cycles();
> -}
> -
> -static inline void
> -dpdk_queue_flush(struct netdev_dpdk *dev, int qid)
> -{
> -    struct dpdk_tx_queue *txq = &dev->tx_q[qid];
> -
> -    if (txq->count == 0) {
> -        return;
> -    }
> -    dpdk_queue_flush__(dev, qid);
>  }
>
>  static bool
> @@ -1080,15 +1045,6 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,
> struct dp_packet **packets,
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>

'dev' and 'netdev' are unused now, we can remove the above lines.




>      int nb_rx;
>
> -    /* There is only one tx queue for this core.  Do not flush other
> -     * queues.
> -     * Do not flush tx queue which is shared among CPUs
> -     * since it is always flushed */
> -    if (rxq_->queue_id == rte_lcore_id() &&
> -        OVS_LIKELY(!dev->txq_needs_locking)) {
> -        dpdk_queue_flush(dev, rxq_->queue_id);
> -    }
> -
>      nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
>                               (struct rte_mbuf **) packets,
>                               NETDEV_MAX_BURST);
>


More information about the dev mailing list