| Message ID | 20210425162828.31982-2-elibr@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dpdk debug stats commands | expand |
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 <elibr@nvidia.com> > Reviewed-by: Salem Sol <salems@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. > + 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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 <elibr@nvidia.com> >> Reviewed-by: Salem Sol <salems@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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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) +{ + 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; + } + + 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); + 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); + } + + 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;