All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: Alejandro Lucero <alejandro.lucero@netronome.com>, dev@dpdk.org
Cc: Bert van Leeuwen <bert.vanleeuwen@netronome.com>,
	thomas.monjalon@6wind.com
Subject: Re: [PATCH v2] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS
Date: Thu, 24 Nov 2016 17:59:06 +0100	[thread overview]
Message-ID: <1480006746.31853.14.camel@6wind.com> (raw)
In-Reply-To: <1479722378-23959-1-git-send-email-alejandro.lucero@netronome.com>

Hi,

On Mon, 2016-11-21 at 09:59 +0000, Alejandro Lucero wrote:
> From: Bert van Leeuwen <bert.vanleeuwen@netronome.com>
> 
> Arrays inside rte_eth_stats have size=RTE_ETHDEV_QUEUE_STAT_CNTRS.
> Some devices report more queues than that and this code blindly uses
> the reported number of queues by the device to fill those arrays up.
> This patch fixes the problem using MIN between the reported number of
> queues and RTE_ETHDEV_QUEUE_STAT_CNTRS.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> 

Reviewed-by: Olivier Matz <olivier.matz@6wind.com>


As a next step, I'm wondering if it would be possible to remove
this limitation. We could replace the tables in struct rte_eth_stats
by a pointer to an array allocated dynamically at pmd setup.

It would break the API, so it should be announced first. I'm thinking
of something like:

struct rte_eth_generic_stats {
        uint64_t ipackets;
        uint64_t opackets;
        uint64_t ibytes;
        uint64_t obytes;
        uint64_t imissed;
        uint64_t ierrors;
        uint64_t oerrors;
        uint64_t rx_nombuf
};

struct rte_eth_stats {
	struct rte_eth_generic_stats port_stats;
	struct rte_eth_generic_stats *queue_stats;
};

The queue_stats array would always be indexed by queue_id.
The xstats would continue to report the generic stats per-port and
per-queue.

About the mapping API, either we keep it as-is, or it could
become a driver-specific API.


Thomas, what do you think?

Regards,
Olivier

  reply	other threads:[~2016-11-24 16:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-21  9:59 [PATCH v2] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS Alejandro Lucero
2016-11-24 16:59 ` Olivier Matz [this message]
2016-11-28 11:13   ` Thomas Monjalon
2016-12-01 14:44     ` Alejandro Lucero
2016-12-05 12:53   ` Olivier Matz
2016-12-06 13:24     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480006746.31853.14.camel@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=alejandro.lucero@netronome.com \
    --cc=bert.vanleeuwen@netronome.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.