On 30 Apr 2021, at 14:51, Eelco Chaudron wrote:

On 25 Apr 2021, at 18:28, Eli Britstein wrote:

New appctl 'dpdk/get-socket-stats' implemented to get result of
'rte_malloc_get_socket_stats()' function for socket passed as an
argument. If no arguments passed, get the results for all available
sockets.

The information is malloc() stats per cpu socket, so I would rename the command to
dpdk/get-malloc-stats (comment in patch 2 for the name there).

Could be used for debugging.

Signed-off-by: Eli Britstein <el...@nvidia.com>
Reviewed-by: Salem Sol <sal...@nvidia.com>
---
 NEWS                 |  2 ++
 lib/dpdk-unixctl.man |  4 +++
lib/dpdk.c | 60 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/NEWS b/NEWS
index 95cf922aa..e0c0f0408 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,8 @@ Post-v2.15.0
* New option '--no-record-hostname' to disable hostname configuration
        in ovsdb on startup.
* New command 'record-hostname-if-not-set' to update hostname in ovsdb.
+   - DPDK:
+     * New debug appctl command 'dpdk/get-socket-stats'.

 v2.15.0 - 15 Feb 2021
diff --git a/lib/dpdk-unixctl.man b/lib/dpdk-unixctl.man
index 2d6d576f2..37f02d6c7 100644
--- a/lib/dpdk-unixctl.man
+++ b/lib/dpdk-unixctl.man
@@ -10,5 +10,9 @@ list of words separated by spaces: a word can be either a logging \fBlevel\fR \fBnotice\fR, \fBinfo\fR or \fBdebug\fR) or a \fBpattern\fR matching DPDK components (see \fBdpdk/log-list\fR command on \fBovs\-appctl\fR(8)) separated
 by a colon from the logging \fBlevel\fR to apply.
+.IP "\fBdpdk/get-socket-stats\fR [\fIsocket\fR]"
+Prints the statistics information about DPDK socket \fIsocket\fR.
+If called without arguments, information of all the available sockets will
+be printed.
 .RE
 .
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 319540394..5a6b2015c 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -25,6 +25,7 @@
 #include <rte_cpuflags.h>
 #include <rte_errno.h>
 #include <rte_log.h>
+#include <rte_malloc.h>
 #include <rte_memzone.h>
 #include <rte_version.h>

@@ -356,6 +357,63 @@ dpdk_unixctl_log_set(struct unixctl_conn *conn, int argc, const char *argv[],
     unixctl_command_reply(conn, NULL);
 }

+static void +dpdk_unixctl_get_socket_stats(struct unixctl_conn *conn,
+                              int argc, const char *argv[],
+                              void *aux OVS_UNUSED)

Guess function also need to change as we are not getting sockets stats, but malloc stats per socket.

+{
+    struct rte_malloc_socket_stats socket_stats;
+    unsigned int socket_max = rte_socket_count();
+    unsigned int socket_min = 0;
+    unsigned int socket;
+    size_t size;
+    FILE *stream;
+    char *response = NULL;
+
+    if (argc == 2) {
+        socket_min = atoi(argv[1]);
+        socket_max = socket_min + 1;
+    }
+
+    stream = open_memstream(&response, &size);
+    if (!stream) {
+        response = xasprintf("Unable to open memstream: %s.",
+                             ovs_strerror(errno));
+        unixctl_command_reply_error(conn, response);
+        goto out;

Don’t think we need a stream in this case, see below.
+    }
+
+    for (socket = socket_min; socket < socket_max; socket++) {
+        if (rte_malloc_get_socket_stats(socket, &socket_stats)) {
+ response = xasprintf("Unable to get stats for socket %d: %s.",
+                                 socket, ovs_strerror(errno));
+            fclose(stream);
+            unixctl_command_reply_error(conn, response);

If I put in an invalid socket id, the error message is not printed. Did not further research, but if I replace “response” here with a const string it gets printed.

Looking about it again, you might need to close the stream first, free(response), then call xasprintf() followed by unixctl_command_reply_error().

+            goto out;
+        }
+        fprintf(stream,
+                "Socket = %d\n"
+                "heap_totalsz_bytes = %lu\n"
+                "heap_freesz_bytes = %lu\n"
+                "greatest_free_size = %lu\n"
+                "free_count = %u\n"
+                "alloc_count = %u\n"
+                "heap_allocsz_bytes = %lu\n", socket,
+                socket_stats.heap_totalsz_bytes,
+                socket_stats.heap_freesz_bytes,
+                socket_stats.greatest_free_size,
+                socket_stats.free_count,
+                socket_stats.alloc_count,
+                socket_stats.heap_allocsz_bytes);

I think here you do not need to put it to a stream, but you should use the normal APIs for this, see dpif_netdev_pmd_info() for an example, i.e., pass the result to the unixctl_command_reply().

+    }
+
+    fclose(stream);
+
+    unixctl_command_reply(conn, response);
+out:
+    free(response);
+}
+
 static bool
 dpdk_init__(const struct smap *ovs_other_config)
 {
@@ -525,6 +583,8 @@ dpdk_init__(const struct smap *ovs_other_config)
                              dpdk_unixctl_mem_stream, rte_log_dump);
unixctl_command_register("dpdk/log-set", "{level | pattern:level}", 0,
                              INT_MAX, dpdk_unixctl_log_set, NULL);
+ unixctl_command_register("dpdk/get-socket-stats", "[socket]", 0, INT_MAX,
+                             dpdk_unixctl_get_socket_stats, NULL);

     /* We are called from the main thread here */
     RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
--
2.28.0.2311.g225365fb51

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to