Fix error accounting: backpressure from pcap_sendpacket() (kernel
socket buffer full) was incorrectly counted as errors. Malformed
multi-segment mbufs where pkt_len exceeds actual data were silently
accepted; they are now detected via rte_pktmbuf_read() failure
and counted as errors.

On Linux, pcap_sendpacket() calls send() on a blocking PF_PACKET
socket with default kernel buffer sizes and no TX ring (PACKET_TX_RING).
The send() call only blocks when the kernel socket send buffer is full,
providing limited backpressure. Backpressure is not an error.

Fixes: fbbbf553f268 ("net/pcap: fix concurrent multiseg Tx")
Cc: [email protected]

Signed-off-by: Stephen Hemminger <[email protected]>
---
 drivers/net/pcap/pcap_ethdev.c | 71 +++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index 4cf5319839..42c5a37177 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -407,22 +407,27 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, 
uint16_t nb_pkts)
         * dumper */
        for (i = 0; i < nb_pkts; i++) {
                struct rte_mbuf *mbuf = bufs[i];
-               size_t len, caplen;
+               uint32_t len, caplen;
+               const uint8_t *data;
 
                len = caplen = rte_pktmbuf_pkt_len(mbuf);
 
                calculate_timestamp(&header.ts);
                header.len = len;
                header.caplen = caplen;
-               /* rte_pktmbuf_read() returns a pointer to the data directly
-                * in the mbuf (when the mbuf is contiguous) or, otherwise,
-                * a pointer to temp_data after copying into it.
-                */
-               pcap_dump((u_char *)dumper, &header,
-                       rte_pktmbuf_read(mbuf, 0, caplen, temp_data));
 
-               num_tx++;
-               tx_bytes += caplen;
+               data = rte_pktmbuf_read(mbuf, 0, caplen, temp_data);
+               if (unlikely(data == NULL)) {
+                       /* This only happens if mbuf is bogus pkt_len > 
data_len */
+                       PMD_LOG(ERR, "rte_pktmbuf_read failed");
+                       dumper_q->tx_stat.err_pkts++;
+               } else {
+                       pcap_dump((u_char *)dumper, &header, data);
+
+                       num_tx++;
+                       tx_bytes += caplen;
+               }
+
                rte_pktmbuf_free(mbuf);
        }
 
@@ -461,7 +466,17 @@ eth_tx_drop(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
 }
 
 /*
- * Callback to handle sending packets through a real NIC.
+ * Send a burst of packets to a pcap device.
+ *
+ * On Linux, pcap_sendpacket() calls send() on a blocking PF_PACKET
+ * socket with default kernel buffer sizes and no TX ring (PACKET_TX_RING).
+ * The send() call only blocks when the kernel socket send buffer is full,
+ * providing limited backpressure.
+ *
+ * On error, pcap_sendpacket() returns non-zero and the loop breaks,
+ * leaving remaining packets unsent.
+ *
+ * Bottom line: backpressure is not an error.
  */
 static uint16_t
 eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
@@ -483,34 +498,38 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
 
        for (i = 0; i < nb_pkts; i++) {
                struct rte_mbuf *mbuf = bufs[i];
-               size_t len = rte_pktmbuf_pkt_len(mbuf);
-               int ret;
+               uint32_t len = rte_pktmbuf_pkt_len(mbuf);
+               const uint8_t *data;
 
-               if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
-                               len > RTE_ETH_PCAP_SNAPSHOT_LEN)) {
+               if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) && len > 
RTE_ETH_PCAP_SNAPSHOT_LEN)) {
                        PMD_LOG(ERR,
-                               "Dropping multi segment PCAP packet. Size (%zd) 
> max size (%u).",
+                               "Dropping multi segment PCAP packet. Size (%u) 
> max size (%u).",
                                len, RTE_ETH_PCAP_SNAPSHOT_LEN);
+                       tx_queue->tx_stat.err_pkts++;
                        rte_pktmbuf_free(mbuf);
                        continue;
                }
 
-               /* rte_pktmbuf_read() returns a pointer to the data directly
-                * in the mbuf (when the mbuf is contiguous) or, otherwise,
-                * a pointer to temp_data after copying into it.
-                */
-               ret = pcap_sendpacket(pcap,
-                       rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
-               if (unlikely(ret != 0))
-                       break;
-               num_tx++;
-               tx_bytes += len;
+               data = rte_pktmbuf_read(mbuf, 0, len, temp_data);
+               if (unlikely(data == NULL)) {
+                       /* This only happens if mbuf is bogus pkt_len > 
data_len */
+                       PMD_LOG(ERR, "rte_pktmbuf_read failed");
+                       tx_queue->tx_stat.err_pkts++;
+               } else {
+                       /* Unfortunately, libpcap collapses transient (-EBUSY) 
and hard errors. */
+                       if (pcap_sendpacket(pcap, data, len) != 0) {
+                               PMD_LOG(ERR, "pcap_sendpacket() failed: %s", 
pcap_geterr(pcap));
+                               break;
+                       }
+                       num_tx++;
+                       tx_bytes += len;
+               }
+
                rte_pktmbuf_free(mbuf);
        }
 
        tx_queue->tx_stat.pkts += num_tx;
        tx_queue->tx_stat.bytes += tx_bytes;
-       tx_queue->tx_stat.err_pkts += i - num_tx;
 
        return i;
 }
-- 
2.51.0

Reply via email to