The error handling when pcap_sendpacket() was incorrect.
When underlying kernel socket buffer got full the send was counted as
an error. Malformed multi-segment mbufs where pkt_len exceeds actual
data were silently accepted.
Malformed mbufs 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 | 77 +++++++++++++++++++++-------------
1 file changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index 4cf5319839..1ab5b1755c 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);
}
@@ -434,9 +439,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs,
uint16_t nb_pkts)
pcap_dump_flush(dumper);
dumper_q->tx_stat.pkts += num_tx;
dumper_q->tx_stat.bytes += tx_bytes;
- dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
- return nb_pkts;
+ return i;
}
/*
@@ -461,7 +465,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 +497,41 @@ 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 {
+ /*
+ * No good way to separate back pressure from failure
here
+ * Assume it is EBUSY, ENOMEM, or EINTR, something that
can be retried.
+ */
+ 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
