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 <ma...@mellanox.com>
Acked-by: Gaetan Rivet <gaetan.ri...@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
