Re: [lng-odp] [API-NEXT PATCH 1/5] linux-generic: sockets: implement pktio statistics counters

Maxim Uvarov Thu, 12 Nov 2015 06:15:49 -0800

On 11/10/2015 21:29, Stuart Haslam wrote:

On Mon, Nov 09, 2015 at 02:21:49PM +0300, Maxim Uvarov wrote:

Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
---
  platform/linux-generic/Makefile.am                 |  1 +
  .../linux-generic/include/odp_packet_io_internal.h |  9 +++
  platform/linux-generic/odp_packet_io.c             | 53 +++++++++++++++++
  platform/linux-generic/pktio/socket.c              | 59 ++++++++++++++++++
  platform/linux-generic/pktio/socket_mmap.c         |  8 +++
  platform/linux-generic/pktio/stats_sysfs.c         | 69 ++++++++++++++++++++++
  6 files changed, 199 insertions(+)
  create mode 100644 platform/linux-generic/pktio/stats_sysfs.c


diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 610c79c..e3e4954 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -170,6 +170,7 @@ __LIB__libodp_la_SOURCES = \
                           pktio/netmap.c \
                           pktio/socket.c \
                           pktio/socket_mmap.c \
+                          pktio/stats_sysfs.c \
                           odp_pool.c \
                           odp_queue.c \
                           odp_rwlock.c \
diff --git a/platform/linux-generic/include/odp_packet_io_internal.h 
b/platform/linux-generic/include/odp_packet_io_internal.h
index 1a1118c..ec43e87 100644
--- a/platform/linux-generic/include/odp_packet_io_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_internal.h
@@ -84,6 +84,7 @@ struct pktio_entry {
                STATE_STOP
        } state;
        classifier_t cls;               /**< classifier linked with this pktio*/
+       odp_pktio_stats_t stats;        /**< statistic counters for pktio */
        char name[PKTIO_NAME_LEN];      /**< name of pktio provided to
                                           pktio_open() */
        odp_pktio_param_t param;
@@ -107,6 +108,8 @@ typedef struct pktio_if_ops {
        int (*close)(pktio_entry_t *pktio_entry);
        int (*start)(pktio_entry_t *pktio_entry);
        int (*stop)(pktio_entry_t *pktio_entry);
+       int (*stats)(pktio_entry_t *pktio_entry, odp_pktio_stats_t *stats);
+       int (*stats_reset)(pktio_entry_t *pktio_entry);
        int (*recv)(pktio_entry_t *pktio_entry, odp_packet_t pkt_table[],
                    unsigned len);
        int (*send)(pktio_entry_t *pktio_entry, odp_packet_t pkt_table[],
@@ -159,6 +162,12 @@ extern const pktio_if_ops_t pcap_pktio_ops;
  #endif
  extern const pktio_if_ops_t * const pktio_if_ops[];

+int sysfs_stats(pktio_entry_t *pktio_entry,

+               odp_pktio_stats_t *stats);
+int sock_stats_reset(pktio_entry_t *pktio_entry);
+int sock_stats(pktio_entry_t *pktio_entry,
+              odp_pktio_stats_t *stats);
+
  #ifdef __cplusplus
  }
  #endif
diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 3ef400f..ba97629 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -862,3 +862,56 @@ void odp_pktio_print(odp_pktio_t id)

ODP_PRINT("\n%s\n", str);

  }
+
+int odp_pktio_stats(odp_pktio_t pktio,
+                   odp_pktio_stats_t *stats)
+{
+       pktio_entry_t *entry;
+       int ret = -1;
+
+       entry = get_pktio_entry(pktio);
+       if (entry == NULL) {
+               ODP_DBG("pktio entry %d does not exist\n", pktio);
+               return -1;
+       }
+
+       lock_entry(entry);
+
+       if (odp_unlikely(is_free(entry))) {
+               unlock_entry(entry);
+               ODP_DBG("already freed pktio\n");
+               return -1;
+       }
+
+       if (entry->s.ops->stats)
+               ret = entry->s.ops->stats(entry, stats);
+       unlock_entry(entry);
+
+       return ret;
+}
+
+int odp_pktio_stats_reset(odp_pktio_t pktio)
+{
+       pktio_entry_t *entry;
+       int ret = -1;
+
+       entry = get_pktio_entry(pktio);
+       if (entry == NULL) {
+               ODP_DBG("pktio entry %d does not exist\n", pktio);
+               return -1;
+       }
+
+       lock_entry(entry);
+
+       if (odp_unlikely(is_free(entry))) {
+               unlock_entry(entry);
+               ODP_DBG("already freed pktio\n");
+               return -1;
+       }
+
+       if (entry->s.ops->stats)
+               ret = entry->s.ops->stats_reset(entry);
+       unlock_entry(entry);
+
+       return ret;
+}
diff --git a/platform/linux-generic/pktio/socket.c 
b/platform/linux-generic/pktio/socket.c
index 5f5e0ae..0bda57b 100644
--- a/platform/linux-generic/pktio/socket.c
+++ b/platform/linux-generic/pktio/socket.c
@@ -254,6 +254,12 @@ static int sock_setup_pkt(pktio_entry_t *pktio_entry, 
const char *netdev,
                goto error;
        }

+ err = sock_stats_reset(pktio_entry);

+       if (err != 0) {
+               ODP_ERR("reset stats\n");
+               goto error;
+       }
+
        return 0;

error:

@@ -467,6 +473,57 @@ static int sock_promisc_mode_get(pktio_entry_t 
*pktio_entry)
                                   pktio_entry->s.name);
  }

+int sock_stats(pktio_entry_t *pktio_entry,

+              odp_pktio_stats_t *stats)
+{
+       odp_pktio_stats_t cur_stats;
+       int ret;
+
+       memset(&cur_stats, 0, sizeof(odp_pktio_stats_t));
+       ret = sysfs_stats(pktio_entry, &cur_stats);
+       if (ret)
+               ODP_ABORT("sysfs stats error\n");
+
+       stats->in_octets = cur_stats.in_octets -
+                               pktio_entry->s.stats.in_octets;
+       stats->in_ucast_pkts = cur_stats.in_ucast_pkts -
+                               pktio_entry->s.stats.in_ucast_pkts;
+       stats->in_discards = cur_stats.in_discards -
+                               pktio_entry->s.stats.in_discards;
+       stats->in_errors = cur_stats.in_errors -
+                               pktio_entry->s.stats.in_errors;
+       stats->in_unknown_protos = cur_stats.in_unknown_protos -
+                               pktio_entry->s.stats.in_unknown_protos;
+
+       stats->out_octets = cur_stats.out_octets -
+                               pktio_entry->s.stats.out_octets;
+       stats->out_ucast_pkts = cur_stats.out_ucast_pkts -
+                               pktio_entry->s.stats.out_ucast_pkts;
+       stats->out_discards = cur_stats.out_discards -
+                               pktio_entry->s.stats.out_discards;
+       stats->out_errors = cur_stats.out_errors -
+                               pktio_entry->s.stats.out_errors;
+
+       return 0;
+}
+
+int sock_stats_reset(pktio_entry_t *pktio_entry)
+{
+       int err = 0;
+       odp_pktio_stats_t cur;
+
+       memset(&cur, 0, sizeof(odp_pktio_stats_t));
+       err = sysfs_stats(pktio_entry, &cur);
+       if (err != 0) {
+               __odp_errno = errno;
+               ODP_ERR("sysfs stats error\n");
+       } else {
+               memcpy(&pktio_entry->s.stats, &cur, sizeof(odp_pktio_stats_t));
+       }
+
+       return err;
+}
+
  const pktio_if_ops_t sock_mmsg_pktio_ops = {
        .init = NULL,
        .term = NULL,
@@ -474,6 +531,8 @@ const pktio_if_ops_t sock_mmsg_pktio_ops = {
        .close = sock_close,
        .start = NULL,
        .stop = NULL,
+       .stats = sock_stats,
+       .stats_reset = sock_stats_reset,
        .recv = sock_mmsg_recv,
        .send = sock_mmsg_send,
        .mtu_get = sock_mtu_get,
diff --git a/platform/linux-generic/pktio/socket_mmap.c 
b/platform/linux-generic/pktio/socket_mmap.c
index 79ff82d..2031485 100644
--- a/platform/linux-generic/pktio/socket_mmap.c
+++ b/platform/linux-generic/pktio/socket_mmap.c
@@ -467,6 +467,12 @@ static int sock_mmap_open(odp_pktio_t id ODP_UNUSED,
                        goto error;
        }

+ ret = sock_stats_reset(pktio_entry);

+       if (ret != 0) {
+               ODP_ERR("reset stats\n");
+               goto error;
+       }
+
        return 0;

error:

@@ -525,6 +531,8 @@ const pktio_if_ops_t sock_mmap_pktio_ops = {
        .close = sock_mmap_close,
        .start = NULL,
        .stop = NULL,
+       .stats = sock_stats,
+       .stats_reset = sock_stats_reset,
        .recv = sock_mmap_recv,
        .send = sock_mmap_send,
        .mtu_get = sock_mmap_mtu_get,
diff --git a/platform/linux-generic/pktio/stats_sysfs.c 
b/platform/linux-generic/pktio/stats_sysfs.c
new file mode 100644
index 0000000..5033b7e
--- /dev/null
+++ b/platform/linux-generic/pktio/stats_sysfs.c
@@ -0,0 +1,69 @@
+/* Copyright (c) 2015, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+#include <odp.h>
+#include <odp_packet_io_internal.h>
+#include <stdio.h>
+
+static uint64_t sysfs_get_val(const char *fname)
+{
+       FILE  *file;
+       char str[128];
+       uint64_t ret = -1;
+
+       file = fopen(fname, "rt");
+       if (file == NULL) {
+               /* File not found */
+               return 0;
+       }
+
+       if (fgets(str, sizeof(str), file) != NULL) {
+               /* Read cache line size */

??


thanks, copy paste error :)

+               if (sscanf(str, "%" SCNx64, &ret) != 1) {
+                       ODP_ERR("read %s\n", fname);
+                       ret = 0;
+               }
+       }
+
+       (void)fclose(file);
+       return ret;
+}

Error handling is a bit broken here, 0 isn't a good failure indication.
It would be better to return an int and take a uint64_t* parameter.


For unsupported counters we just have 0. So 0 here is not an error,
it's just zero.

Although I think it would actually be better to use ETHTOOL_GSTATS as
that should work for netmap too, in which case this helper wouldn't be
needed.


By doing that we should be depend on ethtool headers. And quite tricky
to rewrite eththool GPLv2 functions to code compatible BSD. Simple copy
paste will not work and I'm know how to rewrite it in different manner.

Sysfs was the most easy solution to read counters for me. Btw are there
any reasons to not export counters to /sysfs in netmap?

+
+int sysfs_stats(pktio_entry_t *pktio_entry,
+               odp_pktio_stats_t *stats)
+{
+       char fname[256];
+       const char *dev = pktio_entry->s.name;
+
+       sprintf(fname, "/sys/class/net/%s/statistics/rx_bytes", dev);
+       stats->in_octets = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/rx_packets", dev);
+       stats->in_ucast_pkts = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/rx_droppped", dev);
+       stats->in_discards = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/rx_errors", dev);
+       stats->in_errors = sysfs_get_val(fname);
+
+       stats->in_unknown_protos = 0;

I presume there's no easy way of getting this information, could do with
a comment noting that it's not supported.

In linux-generic we have software classifier. If it's unable to parsepacket

and detect protocol, then that counter have to be increased. I wanted to
send separate patch for it.

+
+       sprintf(fname, "/sys/class/net/%s/statistics/tx_bytes", dev);
+       stats->out_octets = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/tx_packets", dev);
+       stats->out_ucast_pkts = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/tx_dropped", dev);
+       stats->out_discards = sysfs_get_val(fname);
+
+       sprintf(fname, "/sys/class/net/%s/statistics/tx_errors", dev);
+       stats->out_errors = sysfs_get_val(fname);
+
+       return 0;
+}
+
--
1.9.1


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCH 1/5] linux-generic: sockets: implement pktio statistics counters

Reply via email to