[dpdk-dev,v1,0/6] Extended xstats API in ethdev library to allow grouping of stats
Checks
Commit Message
27/04/2017 16:42, Michal Jastrzebski:
> From: Kuba Kozak <kubax.kozak@intel.com>
>
> This patchset fixes following patches:
>
> commit ea85e7d711b6 ("ethdev: retrieve xstats by ID")
> commit a954495245c4 ("ethdev: get xstats ID by name")
> commit 1223608adb9b ("app/proc-info: support xstats by ID")
> commit 25e38f09af9c ("net/e1000: support xstats by ID")
> commit 923419333f5a ("net/ixgbe: support xstats by ID")
>
> This patch addresses some API concerns with the xstat patchset
> applied in DPDK 17.05 RC2.
>
> For clarity the first patch reverts all changes from the above
> commits.
It looks crazy to restart the feature from scratch, not showing the fixes.
But after a quick look, there was so many lines and functions to fix
that it is maybe a good idea.
At least, the history will be clear.
[...]
> Kuba Kozak (6):
> ethdev: revert patches extending xstats API in ethdev
> ethdev: retrieve xstats by ID
> ethdev: get xstats ID by name
> app/proc-info: support xstats by ID and by name
> net/e1000: support xstats by ID
> net/ixgbe: support xstats by ID
I am doing these small changes in your patches:
Comments
01/05/2017 23:34, Thomas Monjalon:
> I am doing these small changes in your patches:
More explanations below
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -395,7 +395,7 @@ static const struct eth_dev_ops eth_igb_ops = {
> .link_update = eth_igb_link_update,
> .stats_get = eth_igb_stats_get,
> .xstats_get = eth_igb_xstats_get,
> - .xstats_get_by_id = eth_igb_xstats_get_by_id,
> + .xstats_get_by_id = eth_igb_xstats_get_by_id,
> .xstats_get_names_by_id = eth_igb_xstats_get_names_by_id,
> .xstats_get_names = eth_igb_xstats_get_names,
> .stats_reset = eth_igb_stats_reset,
>
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -532,7 +532,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
> .link_update = ixgbe_dev_link_update,
> .stats_get = ixgbe_dev_stats_get,
> .xstats_get = ixgbe_dev_xstats_get,
> - .xstats_get_by_id = ixgbe_dev_xstats_get_by_id,
> + .xstats_get_by_id = ixgbe_dev_xstats_get_by_id,
> .stats_reset = ixgbe_dev_stats_reset,
> .xstats_reset = ixgbe_dev_xstats_reset,
> .xstats_get_names = ixgbe_dev_xstats_get_names,
It is just for correct alignment.
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -188,7 +188,7 @@ rte_eth_dev_allocated(const char *name)
> static uint8_t
> rte_eth_dev_find_free_port(void)
> {
> - unsigned int i;
> + unsigned i;
>
> for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> if (rte_eth_devices[i].state == RTE_ETH_DEV_UNUSED)
It reverts a change you made in ixgbe patch.
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -185,7 +185,6 @@ extern "C" {
> #include "rte_ether.h"
> #include "rte_eth_ctrl.h"
> #include "rte_dev_info.h"
> -#include "rte_compat.h"
>
> struct rte_mbuf;
>
This include is not needed in this new version of xstats.
I will remove also the changes to release notes (API section),
as the API is not modified. They are just additional functions.
@@ -395,7 +395,7 @@ static const struct eth_dev_ops eth_igb_ops = {
.link_update = eth_igb_link_update,
.stats_get = eth_igb_stats_get,
.xstats_get = eth_igb_xstats_get,
- .xstats_get_by_id = eth_igb_xstats_get_by_id,
+ .xstats_get_by_id = eth_igb_xstats_get_by_id,
.xstats_get_names_by_id = eth_igb_xstats_get_names_by_id,
.xstats_get_names = eth_igb_xstats_get_names,
.stats_reset = eth_igb_stats_reset,
@@ -532,7 +532,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
.link_update = ixgbe_dev_link_update,
.stats_get = ixgbe_dev_stats_get,
.xstats_get = ixgbe_dev_xstats_get,
- .xstats_get_by_id = ixgbe_dev_xstats_get_by_id,
+ .xstats_get_by_id = ixgbe_dev_xstats_get_by_id,
.stats_reset = ixgbe_dev_stats_reset,
.xstats_reset = ixgbe_dev_xstats_reset,
.xstats_get_names = ixgbe_dev_xstats_get_names,
@@ -188,7 +188,7 @@ rte_eth_dev_allocated(const char *name)
static uint8_t
rte_eth_dev_find_free_port(void)
{
- unsigned int i;
+ unsigned i;
for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
if (rte_eth_devices[i].state == RTE_ETH_DEV_UNUSED)
@@ -185,7 +185,6 @@ extern "C" {
#include "rte_ether.h"
#include "rte_eth_ctrl.h"
#include "rte_dev_info.h"
-#include "rte_compat.h"
struct rte_mbuf;