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 <[email protected]>
> ---
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 <[email protected]>
> 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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
