[dpdk-dev] add rx and tx byte counter statistics for PCAP PMD

Message ID 1435664375-25648-1-git-send-email-kd@allegro-packets.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Klaus Degner June 30, 2015, 11:39 a.m. UTC
  PCAP PMD vdev accounts only rx and tx packet statistics.
This patch adds support for rx and tx bytes statistics.

Signed-off-by: Klaus Degner <kd@allegro-packets.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
  

Comments

John McNamara July 8, 2015, 4:11 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Klaus Degner
> Sent: Tuesday, June 30, 2015 12:40 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] add rx and tx byte counter statistics for 
> PCAP PMD
> 
> PCAP PMD vdev accounts only rx and tx packet statistics.
> This patch adds support for rx and tx bytes statistics.

Hi,

Thanks for that. 

Just a few minor comments.

The subject line should contain the library/module being updated and the body is repeats, more or less, the same comment twice. Do a git log on the file to see the standard format.

>  	unsigned i;
> -	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
> +	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0, 
> +rx_b_total = 0, tx_b_total = 0;

This line exceeds the 80 character limit. Maybe separate the rx and tx initialisations onto separate lines. Or else put them all on separate lines in line with the Coding Guidelines:

   http://dpdk.readthedocs.org/en/latest/guidelines/coding_style.html#local-variables

Run checkpatch on the patch before submission to pick up any issues like this.

If you make those changes and resubmit as a V2 I'll ack it.

John.
--
  
Klaus Degner July 9, 2015, 3:14 p.m. UTC | #2
Hi John,

Thank you for your review.

I will submit a v2 of the patch.

Klaus

Am 08.07.15 um 18:11 schrieb Mcnamara, John:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Klaus Degner
>> Sent: Tuesday, June 30, 2015 12:40 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] add rx and tx byte counter statistics for 
>> PCAP PMD
>>
>> PCAP PMD vdev accounts only rx and tx packet statistics.
>> This patch adds support for rx and tx bytes statistics.
> Hi,
>
> Thanks for that. 
>
> Just a few minor comments.
>
> The subject line should contain the library/module being updated and the body is repeats, more or less, the same comment twice. Do a git log on the file to see the standard format.
>
>>  	unsigned i;
>> -	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
>> +	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0, 
>> +rx_b_total = 0, tx_b_total = 0;
> This line exceeds the 80 character limit. Maybe separate the rx and tx initialisations onto separate lines. Or else put them all on separate lines in line with the Coding Guidelines:
>
>    http://dpdk.readthedocs.org/en/latest/guidelines/coding_style.html#local-variables
>
> Run checkpatch on the patch before submission to pick up any issues like this.
>
> If you make those changes and resubmit as a V2 I'll ack it.
>
> John.
> -- 
>
>
>
  

Patch

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index a6ed5bd..cca75ec 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -68,6 +68,7 @@  struct pcap_rx_queue {
 	uint8_t in_port;
 	struct rte_mempool *mb_pool;
 	volatile unsigned long rx_pkts;
+	volatile unsigned long rx_bytes;
 	volatile unsigned long err_pkts;
 	char name[PATH_MAX];
 	char type[ETH_PCAP_ARG_MAXLEN];
@@ -77,6 +78,7 @@  struct pcap_tx_queue {
 	pcap_dumper_t *dumper;
 	pcap_t *pcap;
 	volatile unsigned long tx_pkts;
+	volatile unsigned long tx_bytes;
 	volatile unsigned long err_pkts;
 	char name[PATH_MAX];
 	char type[ETH_PCAP_ARG_MAXLEN];
@@ -140,6 +142,7 @@  eth_pcap_rx(void *queue,
 	struct pcap_rx_queue *pcap_q = queue;
 	uint16_t num_rx = 0;
 	uint16_t buf_size;
+	uint32_t bytes_rx = 0;
 
 	if (unlikely(pcap_q->pcap == NULL || nb_pkts == 0))
 		return 0;
@@ -170,6 +173,7 @@  eth_pcap_rx(void *queue,
 			mbuf->port = pcap_q->in_port;
 			bufs[num_rx] = mbuf;
 			num_rx++;
+			bytes_rx += header.len;
 		} else {
 			/* pcap packet will not fit in the mbuf, so drop packet */
 			RTE_LOG(ERR, PMD,
@@ -179,6 +183,7 @@  eth_pcap_rx(void *queue,
 		}
 	}
 	pcap_q->rx_pkts += num_rx;
+	pcap_q->rx_bytes += bytes_rx;
 	return num_rx;
 }
 
@@ -205,6 +210,7 @@  eth_pcap_tx_dumper(void *queue,
 	struct rte_mbuf *mbuf;
 	struct pcap_tx_queue *dumper_q = queue;
 	uint16_t num_tx = 0;
+	uint32_t bytes_tx = 0;
 	struct pcap_pkthdr header;
 
 	if (dumper_q->dumper == NULL || nb_pkts == 0)
@@ -220,6 +226,7 @@  eth_pcap_tx_dumper(void *queue,
 				rte_pktmbuf_mtod(mbuf, void*));
 		rte_pktmbuf_free(mbuf);
 		num_tx++;
+		bytes_tx += header.len;
 	}
 
 	/*
@@ -229,6 +236,7 @@  eth_pcap_tx_dumper(void *queue,
 	 */
 	pcap_dump_flush(dumper_q->dumper);
 	dumper_q->tx_pkts += num_tx;
+	dumper_q->tx_bytes += bytes_tx;
 	dumper_q->err_pkts += nb_pkts - num_tx;
 	return num_tx;
 }
@@ -402,25 +410,31 @@  eth_stats_get(struct rte_eth_dev *dev,
 		struct rte_eth_stats *igb_stats)
 {
 	unsigned i;
-	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
+	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0, rx_b_total = 0, tx_b_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_rx_queues;
 			i++) {
 		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
+		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
 		rx_total += igb_stats->q_ipackets[i];
+		rx_b_total += igb_stats->q_ibytes[i];
 	}
 
 	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && i < internal->nb_tx_queues;
 			i++) {
 		igb_stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
+		igb_stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		igb_stats->q_errors[i] = internal->tx_queue[i].err_pkts;
 		tx_total += igb_stats->q_opackets[i];
+		tx_b_total += igb_stats->q_obytes[i];
 		tx_err_total += igb_stats->q_errors[i];
 	}
 
 	igb_stats->ipackets = rx_total;
+	igb_stats->ibytes = rx_b_total;
 	igb_stats->opackets = tx_total;
+	igb_stats->obytes = tx_b_total;
 	igb_stats->oerrors = tx_err_total;
 }
 
@@ -429,10 +443,13 @@  eth_stats_reset(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct pmd_internals *internal = dev->data->dev_private;
-	for (i = 0; i < internal->nb_rx_queues; i++)
+	for (i = 0; i < internal->nb_rx_queues; i++) {
 		internal->rx_queue[i].rx_pkts = 0;
+		internal->rx_queue[i].rx_bytes = 0;
+	}
 	for (i = 0; i < internal->nb_tx_queues; i++) {
 		internal->tx_queue[i].tx_pkts = 0;
+		internal->tx_queue[i].tx_bytes = 0;
 		internal->tx_queue[i].err_pkts = 0;
 	}
 }