[dpdk-dev] [PATCH v2] net/failsafe: stat support enhancement

[Gaëtan Rivet]

Hi Matan,

You should send your v[N] In-Reply-To the Message-Id of your v[N-1]
to help people reading.

On Thu, Sep 07, 2017 at 02:31:13PM +0300, Matan Azrad wrote:
> The previous stats code returned only the current TX sub
> device stats.
> 
> This enhancement extends it to return the sum of all sub
> devices stats with history of removed sub-devices.
> 
> Dedicated stats accumulator saves the stat history of all
> sub device remove events.
> 
> Each failsafe sub device contains the last stats asked by
> the user and updates the accumulator in removal time.
> 
> I would like to implement ultimate snapshot on removal time.
> The stats_get API needs to be changed to return error in the
> case it is too late to retrieve statistics.
> By this way, failsafe can get stats snapshot in removal interrupt
> callback for each PMD which can give stats after removal event.
> 
> Signed-off-by: Matan Azrad <matan at mellanox.com>

Acked-by: Gaetan Rivet <gaetan.rivet at 6wind.com>

> ---
>  drivers/net/failsafe/failsafe_ether.c   | 35 ++++++++++++++++++++++++++++++++-
>  drivers/net/failsafe/failsafe_ops.c     | 16 +++++++++++----
>  drivers/net/failsafe/failsafe_private.h |  5 +++++
>  3 files changed, 51 insertions(+), 5 deletions(-)
> 
> 
> V2:
> 1. Fix failsafe conventions.
> 2. Move the stats saving from the interrupt to the remove func.
> 
> 
> diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
> index a3a8cce..dc2e6d1 100644
> --- a/drivers/net/failsafe/failsafe_ether.c
> +++ b/drivers/net/failsafe/failsafe_ether.c
> @@ -308,6 +308,14 @@ fs_dev_remove(struct sub_device *sdev)
>  	failsafe_hotplug_alarm_install(sdev->fs_dev);
>  }
>  
> +static void
> +fs_dev_stats_save(struct sub_device *sdev)
> +{
> +	failsafe_stats_increment(&PRIV(sdev->fs_dev)->stats_accumulator,
> +			&sdev->stats_snapshot);
> +	memset(&sdev->stats_snapshot, 0, sizeof(struct rte_eth_stats));
> +}
> +
>  static inline int
>  fs_rxtx_clean(struct sub_device *sdev)
>  {
> @@ -329,8 +337,10 @@ failsafe_dev_remove(struct rte_eth_dev *dev)
>  	uint8_t i;
>  
>  	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
> -		if (sdev->remove && fs_rxtx_clean(sdev))
> +		if (sdev->remove && fs_rxtx_clean(sdev)) {
> +			fs_dev_stats_save(sdev);
>  			fs_dev_remove(sdev);
> +		}
>  }
>  
>  int
> @@ -399,6 +409,29 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
>  	return ret;
>  }
>  
> +void
> +failsafe_stats_increment(struct rte_eth_stats *to, struct rte_eth_stats *from)
> +{
> +	uint32_t i;
> +
> +	RTE_ASSERT(to != NULL && from != NULL);
> +	to->ipackets += from->ipackets;
> +	to->opackets += from->opackets;
> +	to->ibytes += from->ibytes;
> +	to->obytes += from->obytes;
> +	to->imissed += from->imissed;
> +	to->ierrors += from->ierrors;
> +	to->oerrors += from->oerrors;
> +	to->rx_nombuf += from->rx_nombuf;
> +	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
> +		to->q_ipackets[i] += from->q_ipackets[i];
> +		to->q_opackets[i] += from->q_opackets[i];
> +		to->q_ibytes[i] += from->q_ibytes[i];
> +		to->q_obytes[i] += from->q_obytes[i];
> +		to->q_errors[i] += from->q_errors[i];
> +	}
> +}
> +
>  int
>  failsafe_eth_rmv_event_callback(uint8_t port_id __rte_unused,
>  				enum rte_eth_event_type event __rte_unused,
> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> index ff9ad15..e0f1b0b 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -586,9 +586,14 @@ static void
>  fs_stats_get(struct rte_eth_dev *dev,
>  	     struct rte_eth_stats *stats)
>  {
> -	if (TX_SUBDEV(dev) == NULL)
> -		return;
> -	rte_eth_stats_get(PORT_ID(TX_SUBDEV(dev)), stats);
> +	struct sub_device *sdev;
> +	uint8_t i;
> +
> +	rte_memcpy(stats, &PRIV(dev)->stats_accumulator, sizeof(*stats));
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +		rte_eth_stats_get(PORT_ID(sdev), &sdev->stats_snapshot);
> +		failsafe_stats_increment(stats, &sdev->stats_snapshot);
> +	}
>  }
>  
>  static void
> @@ -597,8 +602,11 @@ fs_stats_reset(struct rte_eth_dev *dev)
>  	struct sub_device *sdev;
>  	uint8_t i;
>  
> -	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
>  		rte_eth_stats_reset(PORT_ID(sdev));
> +		memset(&sdev->stats_snapshot, 0, sizeof(struct rte_eth_stats));
> +	}
> +	memset(&PRIV(dev)->stats_accumulator, 0, sizeof(struct rte_eth_stats));
>  }
>  
>  /**
> diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
> index 0361cf4..4861974 100644
> --- a/drivers/net/failsafe/failsafe_private.h
> +++ b/drivers/net/failsafe/failsafe_private.h
> @@ -102,6 +102,8 @@ struct sub_device {
>  	uint8_t sid;
>  	/* Device state machine */
>  	enum dev_state state;
> +	/* Last stats snapshot passed to user */
> +	struct rte_eth_stats stats_snapshot;
>  	/* Some device are defined as a command line */
>  	char *cmdline;
>  	/* fail-safe device backreference */
> @@ -140,6 +142,7 @@ struct fs_priv {
>  	 * synchronized state.
>  	 */
>  	enum dev_state state;
> +	struct rte_eth_stats stats_accumulator;
>  	unsigned int pending_alarm:1; /* An alarm is pending */
>  	/* flow isolation state */
>  	int flow_isolated:1;
> @@ -180,6 +183,8 @@ int failsafe_eal_uninit(struct rte_eth_dev *dev);
>  
>  int failsafe_eth_dev_state_sync(struct rte_eth_dev *dev);
>  void failsafe_dev_remove(struct rte_eth_dev *dev);
> +void failsafe_stats_increment(struct rte_eth_stats *to,
> +				struct rte_eth_stats *from);
>  int failsafe_eth_rmv_event_callback(uint8_t port_id,
>  				    enum rte_eth_event_type type,
>  				    void *arg, void *out);
> -- 
> 2.7.4
> 

-- 
Gaëtan Rivet
6WIND