From 6eb0ae218a9803bc0ac34b92c980b1503110939e Mon Sep 17 00:00:00 2001 From: "Richardson, Bruce" Date: Tue, 26 Nov 2013 16:49:46 +0000 Subject: [PATCH] pcap: fix mbuf allocation A static list of 64 mbufs was being reused in Rx function. This caused two errors: 1) If more than 64 buffers were requested in a single burst, only the last 64 buffers are returned, the others are lost. 2) Application will free the mbuf being returned, but the receive function will reuse the buffer anyway. If some other allocation is done, there is suddenly multiple writers for the same mbuf. It is fixed by allocating mbuf on demand. In the same time, some length errors are fixed. Reported-by: Mats Liljegren Reported-by: Robert Sanford Signed-off-by: Bruce Richardson Acked-by: Thomas Monjalon --- lib/librte_pmd_pcap/rte_eth_pcap.c | 37 +++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c index 50de8850ab..8e60972358 100644 --- a/lib/librte_pmd_pcap/rte_eth_pcap.c +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c @@ -118,32 +118,47 @@ eth_pcap_rx(void *queue, struct pcap_pkthdr header; const u_char *packet; struct rte_mbuf *mbuf; - static struct rte_mbuf *mbufs[RTE_ETH_PCAP_MBUFS] = { 0 }; struct pcap_rx_queue *pcap_q = queue; + struct rte_pktmbuf_pool_private *mbp_priv; uint16_t num_rx = 0; + uint16_t buf_size; if (unlikely(pcap_q->pcap == NULL || nb_pkts == 0)) return 0; - if(unlikely(!mbufs[0])) - for (i = 0; i < RTE_ETH_PCAP_MBUFS; i++) - mbufs[i] = rte_pktmbuf_alloc(pcap_q->mb_pool); - /* Reads the given number of packets from the pcap file one by one * and copies the packet data into a newly allocated mbuf to return. */ for (i = 0; i < nb_pkts; i++) { - mbuf = mbufs[i % RTE_ETH_PCAP_MBUFS]; + /* Get the next PCAP packet */ packet = pcap_next(pcap_q->pcap, &header); if (unlikely(packet == NULL)) break; + else + mbuf = rte_pktmbuf_alloc(pcap_q->mb_pool); if (unlikely(mbuf == NULL)) break; - rte_memcpy(mbuf->pkt.data, packet, header.len); - mbuf->pkt.data_len = (uint16_t)header.len; - mbuf->pkt.pkt_len = mbuf->pkt.data_len; - bufs[i] = mbuf; - num_rx++; + + /* Now get the space available for data in the mbuf */ + mbp_priv = (struct rte_pktmbuf_pool_private *) + ((char *)pcap_q->mb_pool + sizeof(struct rte_mempool)); + buf_size = (uint16_t) (mbp_priv->mbuf_data_room_size - + RTE_PKTMBUF_HEADROOM); + + if (header.len <= buf_size) { + /* pcap packet will fit in the mbuf, go ahead and copy */ + rte_memcpy(mbuf->pkt.data, packet, header.len); + mbuf->pkt.data_len = (uint16_t)header.len; + mbuf->pkt.pkt_len = mbuf->pkt.data_len; + bufs[i] = mbuf; + num_rx++; + } else { + /* pcap packet will not fit in the mbuf, so drop packet */ + RTE_LOG(ERR, PMD, + "PCAP packet %d bytes will not fit in mbuf (%d bytes)\n", + header.len, buf_size); + rte_pktmbuf_free(mbuf); + } } pcap_q->rx_pkts += num_rx; return num_rx; -- 2.20.1