diff mbox

[2/2] linux-generic: dpdk: implement pktio statistics

Message ID 1459862441-28467-2-git-send-email-matias.elo@nokia.com
State New
Headers show

Commit Message

Elo, Matias (Nokia - FI/Espoo) April 5, 2016, 1:20 p.m. UTC
Copied pktio statistics implementation from odp-dpdk branch.

Signed-off-by: Matias Elo <matias.elo@nokia.com>
---
 platform/linux-generic/pktio/dpdk.c | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Maxim Uvarov April 6, 2016, 1:53 p.m. UTC | #1
On 04/05/16 16:20, Matias Elo wrote:
> Copied pktio statistics implementation from odp-dpdk branch.
>
> Signed-off-by: Matias Elo <matias.elo@nokia.com>
> ---
>   platform/linux-generic/pktio/dpdk.c | 39 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
>
> diff --git a/platform/linux-generic/pktio/dpdk.c b/platform/linux-generic/pktio/dpdk.c
> index 57625dc..4318a9f 100644
> --- a/platform/linux-generic/pktio/dpdk.c
> +++ b/platform/linux-generic/pktio/dpdk.c
> @@ -580,6 +580,8 @@ static int dpdk_open(odp_pktio_t id ODP_UNUSED,
>   		odp_ticketlock_init(&pkt_dpdk->tx_lock[i]);
>   	}
>   
> +	rte_eth_stats_reset(pkt_dpdk->port_id);
> +
>   	return 0;
>   }
>   
> @@ -905,6 +907,41 @@ static int dpdk_link_status(pktio_entry_t *pktio_entry)
>   	return link.link_status;
>   }
>   
> +static void stats_convert(const struct rte_eth_stats *rte_stats,
> +			  odp_pktio_stats_t *stats)
> +{
> +	stats->in_octets = rte_stats->ibytes;
> +	stats->in_ucast_pkts = 0;
> +	stats->in_discards = rte_stats->imissed;
> +	stats->in_errors = rte_stats->ierrors;
> +	stats->in_unknown_protos = 0;
> +	stats->out_octets = rte_stats->obytes;
> +	stats->out_ucast_pkts = 0;
> +	stats->out_discards = 0;
> +	stats->out_errors = rte_stats->oerrors;

memset(0

If we will add/remove counters then it will be more safe to use memset 
then setting each element to 0.

> +}
> +
> +static int dpdk_stats(pktio_entry_t *pktio_entry, odp_pktio_stats_t *stats)
> +{
> +	int ret;
> +	struct rte_eth_stats rte_stats;
> +
> +	ret = rte_eth_stats_get(pktio_entry->s.pkt_dpdk.port_id, &rte_stats);
> +
> +	if (ret == 0) {
> +		stats_convert(&rte_stats, stats);
> +		return 0;
> +	} else {
> +		return (ret > 0) ? -ret : ret;
do we need such complex return code here?

rte_eth_stats_get() is int but returns only 0. At least for version I'm 
looking for.
Maybe just return -1?

Maxim.


> +	}
> +}
> +
> +static int dpdk_stats_reset(pktio_entry_t *pktio_entry)
> +{
> +	rte_eth_stats_reset(pktio_entry->s.pkt_dpdk.port_id);
> +	return 0;
> +}
> +
>   const pktio_if_ops_t dpdk_pktio_ops = {
>   	.name = "dpdk",
>   	.init_global = odp_dpdk_pktio_init_global,
> @@ -914,6 +951,8 @@ const pktio_if_ops_t dpdk_pktio_ops = {
>   	.close = dpdk_close,
>   	.start = dpdk_start,
>   	.stop = dpdk_stop,
> +	.stats = dpdk_stats,
> +	.stats_reset = dpdk_stats_reset,
>   	.recv_queue = dpdk_recv_queue,
>   	.send_queue = dpdk_send_queue,
>   	.link_status = dpdk_link_status,
Elo, Matias (Nokia - FI/Espoo) April 7, 2016, 6:48 a.m. UTC | #2
Thanks, comments below.

-Matias

> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT Maxim

> Uvarov

> Sent: Wednesday, April 06, 2016 4:54 PM

> To: lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [PATCH 2/2] linux-generic: dpdk: implement pktio statistics

> 

> On 04/05/16 16:20, Matias Elo wrote:

> > Copied pktio statistics implementation from odp-dpdk branch.

> >

> > Signed-off-by: Matias Elo <matias.elo@nokia.com>

> > ---

> >   platform/linux-generic/pktio/dpdk.c | 39

> +++++++++++++++++++++++++++++++++++++

> >   1 file changed, 39 insertions(+)

> >

> > diff --git a/platform/linux-generic/pktio/dpdk.c b/platform/linux-

> generic/pktio/dpdk.c

> > index 57625dc..4318a9f 100644

> > --- a/platform/linux-generic/pktio/dpdk.c

> > +++ b/platform/linux-generic/pktio/dpdk.c

> > @@ -580,6 +580,8 @@ static int dpdk_open(odp_pktio_t id ODP_UNUSED,

> >   		odp_ticketlock_init(&pkt_dpdk->tx_lock[i]);

> >   	}

> >

> > +	rte_eth_stats_reset(pkt_dpdk->port_id);

> > +

> >   	return 0;

> >   }

> >

> > @@ -905,6 +907,41 @@ static int dpdk_link_status(pktio_entry_t *pktio_entry)

> >   	return link.link_status;

> >   }

> >

> > +static void stats_convert(const struct rte_eth_stats *rte_stats,

> > +			  odp_pktio_stats_t *stats)

> > +{

> > +	stats->in_octets = rte_stats->ibytes;

> > +	stats->in_ucast_pkts = 0;

> > +	stats->in_discards = rte_stats->imissed;

> > +	stats->in_errors = rte_stats->ierrors;

> > +	stats->in_unknown_protos = 0;

> > +	stats->out_octets = rte_stats->obytes;

> > +	stats->out_ucast_pkts = 0;

> > +	stats->out_discards = 0;

> > +	stats->out_errors = rte_stats->oerrors;

> 

> memset(0

> 

> If we will add/remove counters then it will be more safe to use memset

> then setting each element to 0.


Using memset is definitely better. I'll fix this in v2.

> 

> > +}

> > +

> > +static int dpdk_stats(pktio_entry_t *pktio_entry, odp_pktio_stats_t *stats)

> > +{

> > +	int ret;

> > +	struct rte_eth_stats rte_stats;

> > +

> > +	ret = rte_eth_stats_get(pktio_entry->s.pkt_dpdk.port_id, &rte_stats);

> > +

> > +	if (ret == 0) {

> > +		stats_convert(&rte_stats, stats);

> > +		return 0;

> > +	} else {

> > +		return (ret > 0) ? -ret : ret;

> do we need such complex return code here?

> 

> rte_eth_stats_get() is int but returns only 0. At least for version I'm

> looking for.

> Maybe just return -1?


The reasoning for this was that according to the dpdk documentation rte_eth_stats_get() should return a non-zero in case of failure. Let's keep it simple and return simply -1.

> 

> Maxim.

> 

> 

> > +	}

> > +}

> > +

> > +static int dpdk_stats_reset(pktio_entry_t *pktio_entry)

> > +{

> > +	rte_eth_stats_reset(pktio_entry->s.pkt_dpdk.port_id);

> > +	return 0;

> > +}

> > +

> >   const pktio_if_ops_t dpdk_pktio_ops = {

> >   	.name = "dpdk",

> >   	.init_global = odp_dpdk_pktio_init_global,

> > @@ -914,6 +951,8 @@ const pktio_if_ops_t dpdk_pktio_ops = {

> >   	.close = dpdk_close,

> >   	.start = dpdk_start,

> >   	.stop = dpdk_stop,

> > +	.stats = dpdk_stats,

> > +	.stats_reset = dpdk_stats_reset,

> >   	.recv_queue = dpdk_recv_queue,

> >   	.send_queue = dpdk_send_queue,

> >   	.link_status = dpdk_link_status,

> 

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/pktio/dpdk.c b/platform/linux-generic/pktio/dpdk.c
index 57625dc..4318a9f 100644
--- a/platform/linux-generic/pktio/dpdk.c
+++ b/platform/linux-generic/pktio/dpdk.c
@@ -580,6 +580,8 @@  static int dpdk_open(odp_pktio_t id ODP_UNUSED,
 		odp_ticketlock_init(&pkt_dpdk->tx_lock[i]);
 	}
 
+	rte_eth_stats_reset(pkt_dpdk->port_id);
+
 	return 0;
 }
 
@@ -905,6 +907,41 @@  static int dpdk_link_status(pktio_entry_t *pktio_entry)
 	return link.link_status;
 }
 
+static void stats_convert(const struct rte_eth_stats *rte_stats,
+			  odp_pktio_stats_t *stats)
+{
+	stats->in_octets = rte_stats->ibytes;
+	stats->in_ucast_pkts = 0;
+	stats->in_discards = rte_stats->imissed;
+	stats->in_errors = rte_stats->ierrors;
+	stats->in_unknown_protos = 0;
+	stats->out_octets = rte_stats->obytes;
+	stats->out_ucast_pkts = 0;
+	stats->out_discards = 0;
+	stats->out_errors = rte_stats->oerrors;
+}
+
+static int dpdk_stats(pktio_entry_t *pktio_entry, odp_pktio_stats_t *stats)
+{
+	int ret;
+	struct rte_eth_stats rte_stats;
+
+	ret = rte_eth_stats_get(pktio_entry->s.pkt_dpdk.port_id, &rte_stats);
+
+	if (ret == 0) {
+		stats_convert(&rte_stats, stats);
+		return 0;
+	} else {
+		return (ret > 0) ? -ret : ret;
+	}
+}
+
+static int dpdk_stats_reset(pktio_entry_t *pktio_entry)
+{
+	rte_eth_stats_reset(pktio_entry->s.pkt_dpdk.port_id);
+	return 0;
+}
+
 const pktio_if_ops_t dpdk_pktio_ops = {
 	.name = "dpdk",
 	.init_global = odp_dpdk_pktio_init_global,
@@ -914,6 +951,8 @@  const pktio_if_ops_t dpdk_pktio_ops = {
 	.close = dpdk_close,
 	.start = dpdk_start,
 	.stop = dpdk_stop,
+	.stats = dpdk_stats,
+	.stats_reset = dpdk_stats_reset,
 	.recv_queue = dpdk_recv_queue,
 	.send_queue = dpdk_send_queue,
 	.link_status = dpdk_link_status,