On Wed, Jun 17, 2015 at 10:54:25AM +0800, Dongjun wrote:
>
>
> On 2015/6/17 1:44, Daniele Di Proietto wrote:
> >
> >On 16/06/2015 07:40, "Pravin Shelar" <pshe...@nicira.com> wrote:
> >
> >>On Mon, Jun 8, 2015 at 7:42 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> >>>On Mon, Jun 8, 2015 at 6:13 PM, Wei li <l...@dtdream.com> wrote:
> >>>>When tx queue is shared among CPUS,the pkts always be flush in
> >>>>'netdev_dpdk_eth_send'
> >>>>So it is unnecessarily for flushing in netdev_dpdk_rxq_recv
> >>>>Otherwise tx will be accessed without locking
> >>>>
> >>>>Signed-off-by: Wei li <l...@dtdream.com>
> >>>>---
> >>>>v1->v2: fix typo
> >>>>
> >>>> lib/netdev-dpdk.c | 7 +++++--
> >>>> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>>>
> >>>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >>>>index 63243d8..1e0a483 100644
> >>>>--- a/lib/netdev-dpdk.c
> >>>>+++ b/lib/netdev-dpdk.c
> >>>>@@ -892,8 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,
> >>>>struct dp_packet **packets,
> >>>> int nb_rx;
> >>>>
> >>>> /* There is only one tx queue for this core. Do not flush other
> >>>>- * queueus. */
> >>>>- if (rxq_->queue_id == rte_lcore_id()) {
> >>>>+ * 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);
> >>>> }
> >>>Patch looks good, But Daniele has plan to get rid of queue flushing
> >>>logic. do you see problem with doing this?
> >>Daniele,
> >>I am not sure if we should fix this queue flushing logic if we are
> >>going to remove it. So I would like to discuss it first.
> >>You mentioned that the removing flushing logic results in performance
> >>drop. Can you explain it? How much is performance drop and which is
> >>the case?
> >Hi Pravin,
> >
> >sorry for the late reply. I suspected that removing the queues in
> >netdev-dpdk
> >was going to have a performance impact in the following scenario: a batch
> >of
> >packets from different megaflows with the same action (output on port 1).
> >Here's what happens:
> >
> >1) With the queues in netdev-dpdk. dpif-netdev calls netdev_send() for
> >each
> > packet. netdev_dpdk_send() just copies the pointer in the queue. Before
> > receiving the next batch dpdk_queue_flush() call rte_eth_tx_burst() with
> > a whole batch of packets
> >2) Without the queues in netdev-dpdk. dpif-netdev calls netdev_send() for
> >each
> > packet. netdev_dpdk_send() calls rte_eth_tx_burst() for each packet.
> >
> >Scenario 2 could be slower because rte_eth_tx_burst() is called for each
> >packet
> >(instead of each batch). After some testing this turns out not to be the
> >case.
> >It appears that the bottleneck is not rte_eth_tx_burst(), and copying the
> >pointers
> >in the temporary queue makes the code slower.
> Hi Daniele,
> I get something else impacting the performance too. Here it is:
> In netdev_dpdk_rxq_recv(), the flush condition is "txq->count != 0", and in
> dpdk_queue_pkts(), the flush condition is "txq->count == MAX_TX_QUEUE_LEN"
> or "(rte_get_timer_cycles() - txq->tsc) >= DRAIN_TSC".
> They don't match. Our thoughts is to get a batch flush, but
> netdev_dpdk_rxq_recv() may do it earlier sometime.
>
> I did some local change, stealing the cycle interval from dpdk_queue_pkts().
> I got performance improvement, about at least 10% from guest VNIC to dpdk
> phy NIC to outside.
> I didn't compare the performance to "Scenario 2" for "Scenario 2"
> modification is not that easy. :(
>
> $ git diff
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 3af1ee7..98d33c3 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -929,10 +929,13 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct
> dp_packet **packets,
>
> /* There is only one tx queue for this core. Do not flush other
> * queueus. */
> - if (rxq_->queue_id == rte_lcore_id()) {
> - dpdk_queue_flush(dev, rxq_->queue_id);
> - }
> + if (rxq_->queue_id == rte_lcore_id() &&
> OVS_LIKELY(!dev->txq_needs_locking)) {
> + struct dpdk_tx_queue *txq = &dev->tx_q[rxq_->queue_id];
>
> + if (txq->count != 0 && (rte_get_timer_cycles() - txq->tsc) >=
> DRAIN_TSC) {
> + dpdk_queue_flush__(dev, rxq_->queue_id); // instead of
> dpdk_queue_flush
> + }
> + }
> nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
> (struct rte_mbuf **) packets,
> NETDEV_MAX_BURST);
I haven't tried your patch yet, but indeed there is something
weird going on with batching.
I am now at 9Mpps running l2fwd inside of guest which is +100%
more than before just because increased the batching.
I can see batching starts with very packets (1~10).
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
