diff mbox series

[v4] net/pcap: improve rx statistics

Message ID 20210909041658.10123-1-chenqiming_huawei@163.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v4] net/pcap: improve rx statistics |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Testing fail Testing issues
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Qiming Chen Sept. 9, 2021, 4:16 a.m. UTC 
  In the receiving direction, if alloc mbuf or jumbo process failed, there
is no err_pkts count, which makes it difficult to locate the problem.
Because alloc mbuf failed, the rx_nombuf field is counted.

Signed-off-by: Qiming Chen <chenqiming_huawei@163.com>
---
v2:
  Clear coding style issues.
v3:
  1) Send direction does not release mbuf.
  2) Failed to alloc mbuf is counted to the rx_nombuf field.
v4:
  Add rx_nombuf field.
---
 drivers/net/pcap/pcap_ethdev.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Ferruh Yigit Sept. 9, 2021, 8:05 a.m. UTC | #1 
On 9/9/2021 5:16 AM, Qiming Chen wrote:
> In the receiving direction, if alloc mbuf or jumbo process failed, there
> is no err_pkts count, which makes it difficult to locate the problem.
> Because alloc mbuf failed, the rx_nombuf field is counted.
> 

Please fix './devtools/check-git-log.sh' warnings.

> Signed-off-by: Qiming Chen <chenqiming_huawei@163.com>
> ---
> v2:
>   Clear coding style issues.
> v3:
>   1) Send direction does not release mbuf.
>   2) Failed to alloc mbuf is counted to the rx_nombuf field.
> v4:
>   Add rx_nombuf field.

<...>

> @@ -297,8 +298,10 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  			break;
>  
>  		mbuf = rte_pktmbuf_alloc(pcap_q->mb_pool);
> -		if (unlikely(mbuf == NULL))
> -			break;
> +		if (unlikely(mbuf == NULL)) {
> +			pcap_q->rx_stat.rx_nombuf++;
> +			continue;

Not sure to update to 'continue' here. I guess both works but if allocating an
mbuf failed, keeping continue to the loop may cause more mbuf allocation
failure, 'break' may give more time to have mbufs available.

Also the patch is related to adding stats, so lets not update the behavior in
this patch.

> +		}
>  
>  		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
>  			/* pcap packet will fit in the mbuf, can copy it */
> @@ -311,6 +314,7 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  						       mbuf,
>  						       packet,
>  						       header.caplen) == -1)) {
> +				pcap_q->rx_stat.rx_nombuf++;

This one should update 'err_pkts'.
diff mbox series

Patch

diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index a8774b7a43..47db6b9b33 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -51,6 +51,7 @@  struct queue_stat {
 	volatile unsigned long pkts;
 	volatile unsigned long bytes;
 	volatile unsigned long err_pkts;
+	volatile unsigned long rx_nombuf;
 };
 
 struct queue_missed_stat {
@@ -297,8 +298,10 @@  eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			break;
 
 		mbuf = rte_pktmbuf_alloc(pcap_q->mb_pool);
-		if (unlikely(mbuf == NULL))
-			break;
+		if (unlikely(mbuf == NULL)) {
+			pcap_q->rx_stat.rx_nombuf++;
+			continue;
+		}
 
 		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
 			/* pcap packet will fit in the mbuf, can copy it */
@@ -311,6 +314,7 @@  eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 						       mbuf,
 						       packet,
 						       header.caplen) == -1)) {
+				pcap_q->rx_stat.rx_nombuf++;
 				rte_pktmbuf_free(mbuf);
 				break;
 			}
@@ -742,7 +746,7 @@  eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	unsigned int i;
 	unsigned long rx_packets_total = 0, rx_bytes_total = 0;
-	unsigned long rx_missed_total = 0;
+	unsigned long rx_missed_total = 0, rx_nombuf = 0;
 	unsigned long tx_packets_total = 0, tx_bytes_total = 0;
 	unsigned long tx_packets_err_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
@@ -751,6 +755,7 @@  eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			i < dev->data->nb_rx_queues; i++) {
 		stats->q_ipackets[i] = internal->rx_queue[i].rx_stat.pkts;
 		stats->q_ibytes[i] = internal->rx_queue[i].rx_stat.bytes;
+		rx_nombuf += internal->rx_queue[i].rx_stat.rx_nombuf;
 		rx_packets_total += stats->q_ipackets[i];
 		rx_bytes_total += stats->q_ibytes[i];
 		rx_missed_total += queue_missed_stat_get(dev, i);
@@ -771,6 +776,7 @@  eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->opackets = tx_packets_total;
 	stats->obytes = tx_bytes_total;
 	stats->oerrors = tx_packets_err_total;
+	stats->rx_nombuf = rx_nombuf;
 
 	return 0;
 }
@@ -784,6 +790,8 @@  eth_stats_reset(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_stat.pkts = 0;
 		internal->rx_queue[i].rx_stat.bytes = 0;
+		internal->rx_queue[i].rx_stat.err_pkts = 0;
+		internal->rx_queue[i].rx_stat.rx_nombuf = 0;
 		queue_missed_stat_reset(dev, i);
 	}