The driver was not reporting multi segment in offload
flags but it can handle it.

The logic for handling transmit would allocate a worst case
size buffer on the stack which could be up to 9K.
If packet was transmitted larger than that it would get
truncated and log would get flooded.

Instead, allocate a buffer on stack only if necessary
and gracefully handle errors by incrementing transmit error
counter.

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

diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index f323c0b0df..22c9cb5928 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -22,7 +22,7 @@
 #include "pcap_osdep.h"
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
-#define RTE_ETH_PCAP_SNAPLEN RTE_ETHER_MAX_JUMBO_FRAME_LEN
+#define RTE_ETH_PCAP_SNAPLEN (RTE_ETHER_MAX_JUMBO_FRAME_LEN - 
RTE_ETHER_CRC_LEN)
 #define RTE_ETH_PCAP_PROMISC 1
 #define RTE_ETH_PCAP_TIMEOUT -1
 
@@ -370,6 +370,22 @@ calculate_timestamp(struct timeval *ts) {
        }
 }
 
+/* Like rte_pktmbuf_read() but allocate if needed */
+static inline const void *
+pcap_pktmbuf_read(const struct rte_mbuf *m,
+                 uint32_t off, uint32_t len, void **buf)
+{
+       if (likely(off + len <= rte_pktmbuf_data_len(m))) {
+               return rte_pktmbuf_mtod_offset(m, char *, off);
+       } else {
+               *buf = malloc(len);
+               if (likely(*buf != NULL))
+                       return rte_pktmbuf_read(m, off, len, *buf);
+               else
+                       return NULL;
+       }
+}
+
 /*
  * Callback to handle writing packets to a pcap file.
  */
@@ -377,46 +393,43 @@ static uint16_t
 eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
        unsigned int i;
-       struct rte_mbuf *mbuf;
        struct pmd_process_private *pp;
        struct pcap_tx_queue *dumper_q = queue;
+       pcap_dumper_t *dumper;
        uint16_t num_tx = 0;
        uint32_t tx_bytes = 0;
        struct pcap_pkthdr header;
-       pcap_dumper_t *dumper;
-       unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
-       size_t len, caplen;
 
        pp = rte_eth_devices[dumper_q->port_id].process_private;
        dumper = pp->tx_dumper[dumper_q->queue_id];
 
-       if (dumper == NULL || nb_pkts == 0)
+       if (unlikely(dumper == NULL || nb_pkts == 0))
                return 0;
 
-       /* writes the nb_pkts packets to the previously opened pcap file
-        * dumper */
+       /* writes the nb_pkts packets to the previously opened pcap file dumper 
*/
        for (i = 0; i < nb_pkts; i++) {
-               mbuf = bufs[i];
-               len = caplen = rte_pktmbuf_pkt_len(mbuf);
-               if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
-                               len > sizeof(temp_data))) {
-                       caplen = sizeof(temp_data);
-               }
+               struct rte_mbuf *mbuf = bufs[i];
+               uint32_t len = rte_pktmbuf_pkt_len(mbuf);
+               void *temp = NULL;
+               const uint8_t *data;
 
                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));
+               header.caplen = len;
+
+               data = pcap_pktmbuf_read(mbuf, 0, len, &temp);
+               if (unlikely(data == NULL)) {
+                       ++dumper_q->tx_stat.err_pkts;
+                       continue;
+               }
+
+               pcap_dump((u_char *)dumper, &header, data);
 
                num_tx++;
-               tx_bytes += caplen;
-               rte_pktmbuf_free(mbuf);
+               tx_bytes += len;
+               free(temp);
        }
+       rte_pktmbuf_free_bulk(bufs, nb_pkts);
 
        /*
         * Since there's no place to hook a callback when the forwarding
@@ -444,15 +457,15 @@ eth_tx_drop(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
        if (unlikely(nb_pkts == 0))
                return 0;
 
-       for (i = 0; i < nb_pkts; i++) {
+       for (i = 0; i < nb_pkts; i++)
                tx_bytes += bufs[i]->pkt_len;
-               rte_pktmbuf_free(bufs[i]);
-       }
+
+       rte_pktmbuf_free_bulk(bufs, nb_pkts);
 
        tx_queue->tx_stat.pkts += nb_pkts;
        tx_queue->tx_stat.bytes += tx_bytes;
 
-       return i;
+       return nb_pkts;
 }
 
 /*
@@ -462,15 +475,11 @@ static uint16_t
 eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
        unsigned int i;
-       int ret;
-       struct rte_mbuf *mbuf;
        struct pmd_process_private *pp;
        struct pcap_tx_queue *tx_queue = queue;
        uint16_t num_tx = 0;
        uint32_t tx_bytes = 0;
        pcap_t *pcap;
-       unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
-       size_t len;
 
        pp = rte_eth_devices[tx_queue->port_id].process_private;
        pcap = pp->tx_pcap[tx_queue->queue_id];
@@ -479,35 +488,31 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
                return 0;
 
        for (i = 0; i < nb_pkts; i++) {
-               mbuf = bufs[i];
-               len = rte_pktmbuf_pkt_len(mbuf);
-               if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
-                               len > sizeof(temp_data))) {
-                       PMD_LOG(ERR,
-                               "Dropping multi segment PCAP packet. Size (%zd) 
> max size (%zd).",
-                               len, sizeof(temp_data));
-                       rte_pktmbuf_free(mbuf);
+               struct rte_mbuf *mbuf = bufs[i];
+               uint32_t len = rte_pktmbuf_pkt_len(mbuf);
+               void *temp = NULL;
+               const uint8_t *data;
+
+               data = pcap_pktmbuf_read(mbuf, 0, len, &temp);
+               if (unlikely(data == NULL)) {
+                       ++tx_queue->tx_stat.err_pkts;
                        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;
-               rte_pktmbuf_free(mbuf);
+               if (likely(pcap_sendpacket(pcap, data, len) == 0)) {
+                       num_tx++;
+                       tx_bytes += len;
+               } else {
+                       ++tx_queue->tx_stat.err_pkts;
+               }
+
        }
 
        tx_queue->tx_stat.pkts += num_tx;
        tx_queue->tx_stat.bytes += tx_bytes;
-       tx_queue->tx_stat.err_pkts += i - num_tx;
+       tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
 
-       return i;
+       return nb_pkts;
 }
 
 /*
@@ -745,6 +750,7 @@ eth_dev_info(struct rte_eth_dev *dev,
        dev_info->max_rx_queues = dev->data->nb_rx_queues;
        dev_info->max_tx_queues = dev->data->nb_tx_queues;
        dev_info->min_rx_bufsize = 0;
+       dev_info->tx_offload_capa = RTE_ETH_TX_OFFLOAD_MULTI_SEGS;
 
        return 0;
 }
-- 
2.51.0

Reply via email to