tx_lock protects the NIC/vhost queue from concurrent access.
Move it closer to the parts it protects and let packets duplication (when
source is not DPDK) and the egress policer run out of it.

Signed-off-by: David Marchand <david.march...@redhat.com>
---
I caught this by code review, but I imagine this could make the
contention on the tx lock even worse if broadcasting packets.

---
 lib/netdev-dpdk.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4c9f122b0..9dd43f2a9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2053,10 +2053,15 @@ netdev_dpdk_rxq_dealloc(struct netdev_rxq *rxq)
  * Returns the number of packets that weren't transmitted. */
 static inline int
 netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
-                         struct rte_mbuf **pkts, int cnt)
+                         struct rte_mbuf **pkts, int cnt, bool concurrent_txq)
 {
     uint32_t nb_tx = 0;
 
+    if (OVS_UNLIKELY(concurrent_txq)) {
+        qid = qid % dev->up.n_txq;
+        rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
+    }
+
     while (nb_tx != cnt) {
         uint32_t ret;
 
@@ -2068,6 +2073,10 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int 
qid,
         nb_tx += ret;
     }
 
+    if (OVS_UNLIKELY(concurrent_txq)) {
+        rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
+    }
+
     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) */
@@ -2412,11 +2421,6 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
         goto out;
     }
 
-    if (OVS_UNLIKELY(!rte_spinlock_trylock(&dev->tx_q[qid].tx_lock))) {
-        COVERAGE_INC(vhost_tx_contention);
-        rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
-    }
-
     cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt);
     sw_stats_add.tx_mtu_exceeded_drops = total_packets - cnt;
 
@@ -2427,6 +2431,11 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 
     n_packets_to_free = cnt;
 
+    if (OVS_UNLIKELY(!rte_spinlock_trylock(&dev->tx_q[qid].tx_lock))) {
+        COVERAGE_INC(vhost_tx_contention);
+        rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
+    }
+
     do {
         int vhost_qid = qid * VIRTIO_QNUM + VIRTIO_RXQ;
         unsigned int tx_pkts;
@@ -2468,7 +2477,8 @@ out:
 
 /* Tx function. Transmit packets indefinitely */
 static void
-dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch)
+dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch,
+                bool concurrent_txq)
     OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     const size_t batch_cnt = dp_packet_batch_size(batch);
@@ -2527,7 +2537,8 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct 
dp_packet_batch *batch)
             __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
                                      txcnt);
         } else {
-            tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt);
+            tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt,
+                                                  concurrent_txq);
         }
     }
 
@@ -2549,7 +2560,7 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 {
 
     if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
-        dpdk_do_tx_copy(netdev, qid, batch);
+        dpdk_do_tx_copy(netdev, qid, batch, true);
         dp_packet_delete_batch(batch, true);
     } else {
         __netdev_dpdk_vhost_send(netdev, qid, batch->packets,
@@ -2568,15 +2579,10 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
         return;
     }
 
-    if (OVS_UNLIKELY(concurrent_txq)) {
-        qid = qid % dev->up.n_txq;
-        rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
-    }
-
     if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
         struct netdev *netdev = &dev->up;
 
-        dpdk_do_tx_copy(netdev, qid, batch);
+        dpdk_do_tx_copy(netdev, qid, batch, concurrent_txq);
         dp_packet_delete_batch(batch, true);
     } else {
         struct netdev_dpdk_sw_stats *sw_stats = dev->sw_stats;
@@ -2591,7 +2597,8 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
         tx_cnt = netdev_dpdk_qos_run(dev, pkts, tx_cnt, true);
         qos_drops -= tx_cnt;
 
-        tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, tx_cnt);
+        tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, tx_cnt,
+                                              concurrent_txq);
 
         dropped = tx_failure + mtu_drops + qos_drops;
         if (OVS_UNLIKELY(dropped)) {
@@ -2603,10 +2610,6 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
             rte_spinlock_unlock(&dev->stats_lock);
         }
     }
-
-    if (OVS_UNLIKELY(concurrent_txq)) {
-        rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
-    }
 }
 
 static int
-- 
2.23.0

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

Reply via email to