[ovs-dev] [PATCH] netdev-afxdp: Convert AFXDP_DEBUG to custom stats.

William Tu u9012063 at gmail.com
Tue Jul 23 22:19:42 UTC 2019


On Tue, Jul 23, 2019 at 06:16:46PM +0300, Ilya Maximets wrote:
> These are valid statistics of a network interface and should be
> exposed via custom stats.
> 
> The same MACRO trick as in vswitchd/bridge.c is used to reduce code
> duplication and easily add new stats if necessary in the future.
> 
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
Thanks for the patch.

I notice two things

1) ovs-vsctl list int
shows that 
statistics          : {"queue_0_rx_dropped"=0,
"queue_0_rx_invalid_descs"=0, "queue_0_tx_invalid_descs"=0,
rx_bytes=1230, rx_packets=15, tx_bytes=4429, tx_packets=37}

There is an extra quote for afxdp counters, I don't know whether
it's expected or not.

2) Should we account and collect the "queue_N_rx_dropped" into
standard stats (struct netdev_stats)?
So the netdev_stas->rx_dropped += queue_0_rx_dropped + queue_1_rx_dropped ...

Otherwise, looks good to me.

Acked-by: William Tu <u9012063 at gmail.com>

>  lib/netdev-afxdp.c | 64 ++++++++++++++++++++++++++++++++--------------
>  lib/netdev-afxdp.h |  4 +++
>  lib/netdev-linux.c |  1 +
>  3 files changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 6b0b93e7f..15cd6b5ab 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -448,21 +448,6 @@ xsk_destroy_all(struct netdev *netdev)
>      }
>  }
>  
> -static inline void OVS_UNUSED
> -log_xsk_stat(struct xsk_socket_info *xsk OVS_UNUSED) {
> -    struct xdp_statistics stat;
> -    socklen_t optlen;
> -
> -    optlen = sizeof stat;
> -    ovs_assert(getsockopt(xsk_socket__fd(xsk->xsk), SOL_XDP, XDP_STATISTICS,
> -               &stat, &optlen) == 0);
> -
> -    VLOG_DBG_RL(&rl, "rx dropped %llu, rx_invalid %llu, tx_invalid %llu",
> -                stat.rx_dropped,
> -                stat.rx_invalid_descs,
> -                stat.tx_invalid_descs);
> -}
> -
>  int
>  netdev_afxdp_set_config(struct netdev *netdev, const struct smap *args,
>                          char **errp OVS_UNUSED)
> @@ -711,10 +696,6 @@ netdev_afxdp_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet_batch *batch,
>          /* TODO: return the number of remaining packets in the queue. */
>          *qfill = 0;
>      }
> -
> -#ifdef AFXDP_DEBUG
> -    log_xsk_stat(xsk_info);
> -#endif
>      return 0;
>  }
>  
> @@ -1012,6 +993,51 @@ netdev_afxdp_destruct(struct netdev *netdev)
>      ovs_mutex_destroy(&dev->mutex);
>  }
>  
> +int
> +netdev_afxdp_get_custom_stats(const struct netdev *netdev,
> +                              struct netdev_custom_stats *custom_stats)
> +{
> +    struct netdev_linux *dev = netdev_linux_cast(netdev);
> +    struct xsk_socket_info *xsk_info;
> +    struct xdp_statistics stat;
> +    uint32_t i, c = 0;
> +    socklen_t optlen;
> +
> +    ovs_mutex_lock(&dev->mutex);
> +
> +#define XDP_CSTATS                                                           \
> +    XDP_CSTAT(rx_dropped)                                                    \
> +    XDP_CSTAT(rx_invalid_descs)                                              \
> +    XDP_CSTAT(tx_invalid_descs)
> +
> +#define XDP_CSTAT(NAME) + 1
> +    enum { N_XDP_CSTATS = XDP_CSTATS };
> +#undef XDP_CSTAT
> +
> +    custom_stats->counters = xcalloc(netdev_n_rxq(netdev) * N_XDP_CSTATS,
> +                                     sizeof *custom_stats->counters);
> +
> +    /* Account the stats for each xsk */
> +    for (i = 0; i < netdev_n_rxq(netdev); i++) {
> +        xsk_info = dev->xsks[i];
> +        optlen  = sizeof stat;
> +
> +        if (xsk_info && !getsockopt(xsk_socket__fd(xsk_info->xsk), SOL_XDP,
> +                                    XDP_STATISTICS, &stat, &optlen)) {
> +#define XDP_CSTAT(NAME)                                                      \
> +            snprintf(custom_stats->counters[c].name,                         \
> +                     NETDEV_CUSTOM_STATS_NAME_SIZE, "queue_%d_" #NAME, i);   \
> +            custom_stats->counters[c++].value = stat.NAME;
> +            XDP_CSTATS;
> +#undef XDP_CSTAT
> +        }
> +    }
> +    custom_stats->size = c;
> +    ovs_mutex_unlock(&dev->mutex);
> +
> +    return 0;
> +}
> +
>  int
>  netdev_afxdp_get_stats(const struct netdev *netdev,
>                         struct netdev_stats *stats)
> diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
> index 6a72bd9a8..e2f400b72 100644
> --- a/lib/netdev-afxdp.h
> +++ b/lib/netdev-afxdp.h
> @@ -33,6 +33,7 @@ struct smap;
>  struct dp_packet;
>  struct netdev_rxq;
>  struct netdev_stats;
> +struct netdev_custom_stats;
>  
>  int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_);
>  void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
> @@ -51,6 +52,9 @@ int netdev_afxdp_get_config(const struct netdev *netdev, struct smap *args);
>  int netdev_afxdp_get_numa_id(const struct netdev *netdev);
>  int netdev_afxdp_get_stats(const struct netdev *netdev_,
>                             struct netdev_stats *stats);
> +int netdev_afxdp_get_custom_stats(const struct netdev *netdev,
> +                                  struct netdev_custom_stats *custom_stats);
> +
>  
>  void free_afxdp_buf(struct dp_packet *p);
>  int netdev_afxdp_reconfigure(struct netdev *netdev);
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 2432cd176..f48192373 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -3293,6 +3293,7 @@ const struct netdev_class netdev_afxdp_class = {
>      .construct = netdev_afxdp_construct,
>      .destruct = netdev_afxdp_destruct,
>      .get_stats = netdev_afxdp_get_stats,
> +    .get_custom_stats = netdev_afxdp_get_custom_stats,
>      .get_status = netdev_linux_get_status,
>      .set_config = netdev_afxdp_set_config,
>      .get_config = netdev_afxdp_get_config,
> -- 
> 2.17.1
> 


More information about the dev mailing list