virtio-net: make Tx drop-less.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>

drivers/virtio-net.cc

@@ -554,7 +554,7 @@ void net::fill_rx_ring()
 
 inline int net::tx_locked(struct mbuf* m_head)
 {
-    return _txq.try_xmit_one_locked(m_head);
+    return _txq.xmit_one_locked(m_head);
 }
 
 inline int net::txq::xmit_prep(mbuf* m_head, net_req*& cooky)
@@ -566,6 +566,8 @@ inline int net::txq::xmit_prep(mbuf* m_head, net_req*& cooky)
     if (m_head->M_dat.MH.MH_pkthdr.csum_flags != 0) {
         m = offload(m_head, &req->mhdr.hdr);
         if ((m_head = m) == nullptr) {
+            stats.tx_err++;
+
             delete req;
 
             /* The buffer is not well-formed */
@@ -577,17 +579,12 @@ inline int net::txq::xmit_prep(mbuf* m_head, net_req*& cooky)
     return 0;
 }
 
-int net::txq::try_xmit_one_locked(mbuf *m_head)
+int net::txq::try_xmit_one_locked(net_req* req)
 {
-    mbuf* m;
+    mbuf *m_head = req->um.get(), *m;
     u16 vec_sz = 0;
     u64 tx_bytes = 0;
-    net_req* req;
 
-    int rc = xmit_prep(m_head, req);
-    if (rc) {
-        goto out;
-    }
 
     if (_parent->_mergeable_bufs) {
         req->mhdr.num_buffers = 0;
@@ -608,53 +605,64 @@ int net::txq::try_xmit_one_locked(mbuf *m_head)
         }
     }
 
+    req->tx_bytes = tx_bytes;
+
     if (!vqueue->avail_ring_has_room(vec_sz)) {
         if (vqueue->used_ring_not_empty()) {
             trace_virtio_net_tx_no_space_calling_gc(_parent->_ifn->if_index);
             gc();
+            if (!vqueue->avail_ring_has_room(vec_sz)) {
+                return ENOBUFS;
+            }
         } else {
-            net_d("%s: no room", __FUNCTION__);
-            delete req;
-
-            rc = ENOBUFS;
-            goto out;
+            return ENOBUFS;
         }
     }
 
     if (!vqueue->add_buf(req)) {
-        trace_virtio_net_tx_failed_add_buf(_parent->_ifn->if_index);
-        delete req;
-
-        rc = ENOBUFS;
-        goto out;
+        assert(0);
     }
 
-    trace_virtio_net_tx_packet(_parent->_ifn->if_index, vec_sz);
-
-out:
+    return 0;
+}
 
-    /* Update the statistics */
-    switch (rc) {
-    case 0: /* success */
-        stats.tx_bytes += tx_bytes;
-        stats.tx_packets++;
+inline void net::txq::update_stats(net_req* req)
+{
+    stats.tx_bytes += req->tx_bytes;
+    stats.tx_packets++;
 
-        if (req->mhdr.hdr.flags & net_hdr::VIRTIO_NET_HDR_F_NEEDS_CSUM)
-            stats.tx_csum++;
+    if (req->mhdr.hdr.flags & net_hdr::VIRTIO_NET_HDR_F_NEEDS_CSUM)
+        stats.tx_csum++;
 
-        if (req->mhdr.hdr.gso_type)
-            stats.tx_tso++;
+    if (req->mhdr.hdr.gso_type)
+        stats.tx_tso++;
+}
 
-        break;
-    case ENOBUFS:
-        stats.tx_drops++;
+int net::txq::xmit_one_locked(mbuf* m_head)
+{
+    net_req* req;
+    int rc = xmit_prep(m_head, req);
+    if (rc) {
+        return rc;
+    }
 
-        break;
-    default:
-        stats.tx_err++;
+    if (try_xmit_one_locked(req)) {
+        do {
+            if (!vqueue->used_ring_not_empty()) {
+                do {
+                    sched::thread::yield();
+                } while (!vqueue->used_ring_not_empty());
+            }
+            gc();
+        } while (!vqueue->add_buf(req));
     }
 
-    return rc;
+    trace_virtio_net_tx_packet(_parent->_ifn->if_index, vqueue->_sg_vec.size());
+
+    // Update the statistics
+    update_stats(req);
+
+    return 0;
 }
 
 mbuf* net::txq::offload(mbuf* m, net_hdr* hdr)

drivers/virtio-net.hh

@@ -254,6 +254,7 @@ private:
         std::unique_ptr<struct mbuf, free_deleter> um;
 
         net_req() {memset(&mhdr,0,sizeof(mhdr));};
+        u64 tx_bytes;
     };
 
     std::string _driver_name;
@@ -305,19 +306,35 @@ private:
             vqueue(vq), _parent(parent) {};
 
         /**
-         * Try to send a Tx frame.
-         * @param m_head
+         * Transmit a single packet. Will wait for completions if there is no
+         * room on a HW ring.
          *
-         * @return 0 if packet has been successfully sent, EINVAL if a packet is
-         *         not well-formed and ENOBUFS if there was no room on a HW ring
-         *         to send the packet.
+         * @param req Tx request handle
+         *
+         * @return 0 if packet has been successfully sent and EINVAL if it was
+         *         not well-formed.
          */
-        int try_xmit_one_locked(mbuf* m_head);
+
+        int xmit_one_locked(mbuf* m_head);
 
         vring* vqueue;
         txq_stats stats = { 0 };
 
     private:
+        /**
+         * This is a private version of try_xmit_one_locked() that acually does
+         * the work.
+         * This function won't update Tx statistics - the caller should do this
+         * after the packet is actually sent.
+         * @param m_head
+         * @param req
+         *
+         * @return 0 if packet has been successfully sent, EINVAL if a packet is
+         *         not well-formed and ENOBUFS if there was no room on a HW ring
+         *         to send the packet.
+         */
+        int try_xmit_one_locked(net_req* req);
+
         /**
          * Checks the packet and returns the net_req (returned in a "cooky")
          * @param m_head
@@ -343,6 +360,12 @@ private:
          */
         mbuf* offload(mbuf* m, net_hdr* hdr);
 
+        /**
+         * Update Tx stats for a single packet in case of a successful xmit.
+         * @param req Appropriate net_req for this packet (we need its mhdr)
+         */
+        void update_stats(net_req* req);
+
         net* _parent;
     };