| Message ID | 20250106110917.1276665-2-roid@nvidia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,1/5] dpdk: Add ovs-appctl dpdk/get-mempool-stats command. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 06/01/2025 11:09, Roi Dayan via dev wrote: > From: Eli Britstein <elibr@nvidia.com> > > New appctl 'dpdk/get-memzone-stats' implemented to get result of > 'rte_memzone_dump()' function. > > Could be used for debugging. > > Signed-off-by: Eli Britstein <elibr@nvidia.com> > Acked-by: Roi Dayan <roid@nvidia.com> > --- > lib/dpdk.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > Hi Roi, Code LGTM. It needs an update to dpdk-unixctl.man and NEWS so users can see it's added. See Eli's previous commit as example: https://github.com/openvswitch/ovs/commit/ad256c31407d347eb185b39cb8fefc9df607e3ed (it also needs a small rebase without mempool-stats patch) thanks, Kevin. > diff --git a/lib/dpdk.c b/lib/dpdk.c > index aaf0b2ed8046..b585327ea971 100644 > --- a/lib/dpdk.c > +++ b/lib/dpdk.c > @@ -324,6 +324,12 @@ malloc_dump_mempool_stats_wrapper(FILE *stream) > rte_mempool_walk(dump_mempool_stats, stream); > } > > +static void > +malloc_dump_memzone_stats_wrapper(FILE *stream) > +{ > + rte_memzone_dump(stream); > +} > + > static bool > dpdk_init__(const struct smap *ovs_other_config) > { > @@ -454,6 +460,9 @@ dpdk_init__(const struct smap *ovs_other_config) > unixctl_command_register("dpdk/get-mempool-stats", "", 0, 0, > dpdk_unixctl_mem_stream, > malloc_dump_mempool_stats_wrapper); > + unixctl_command_register("dpdk/get-memzone-stats", "", 0, 0, > + dpdk_unixctl_mem_stream, > + malloc_dump_memzone_stats_wrapper); > > /* We are called from the main thread here */ > RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
On 1/14/25 20:37, Kevin Traynor wrote: > On 06/01/2025 11:09, Roi Dayan via dev wrote: >> From: Eli Britstein <elibr@nvidia.com> >> >> New appctl 'dpdk/get-memzone-stats' implemented to get result of >> 'rte_memzone_dump()' function. >> >> Could be used for debugging. >> >> Signed-off-by: Eli Britstein <elibr@nvidia.com> >> Acked-by: Roi Dayan <roid@nvidia.com> >> --- >> lib/dpdk.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> > > Hi Roi, > > Code LGTM. It needs an update to dpdk-unixctl.man and NEWS so users can > see it's added. > > See Eli's previous commit as example: > https://github.com/openvswitch/ovs/commit/ad256c31407d347eb185b39cb8fefc9df607e3ed > > (it also needs a small rebase without mempool-stats patch) > > thanks, > Kevin. > >> diff --git a/lib/dpdk.c b/lib/dpdk.c >> index aaf0b2ed8046..b585327ea971 100644 >> --- a/lib/dpdk.c >> +++ b/lib/dpdk.c >> @@ -324,6 +324,12 @@ malloc_dump_mempool_stats_wrapper(FILE *stream) >> rte_mempool_walk(dump_mempool_stats, stream); >> } >> >> +static void >> +malloc_dump_memzone_stats_wrapper(FILE *stream) >> +{ >> + rte_memzone_dump(stream); >> +} This function also seems redundant. Unless there are some compiler warnings, it should be possible to pass rte_memzone_dump directly to the dpdk_unixctl_mem_stream without a wrapper. Best regards, Ilya Maximets. >> + >> static bool >> dpdk_init__(const struct smap *ovs_other_config) >> { >> @@ -454,6 +460,9 @@ dpdk_init__(const struct smap *ovs_other_config) >> unixctl_command_register("dpdk/get-mempool-stats", "", 0, 0, >> dpdk_unixctl_mem_stream, >> malloc_dump_mempool_stats_wrapper); >> + unixctl_command_register("dpdk/get-memzone-stats", "", 0, 0, >> + dpdk_unixctl_mem_stream, >> + malloc_dump_memzone_stats_wrapper); >> >> /* We are called from the main thread here */ >> RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID; > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 14/01/2025 23:40, Ilya Maximets wrote: > On 1/14/25 20:37, Kevin Traynor wrote: >> On 06/01/2025 11:09, Roi Dayan via dev wrote: >>> From: Eli Britstein <elibr@nvidia.com> >>> >>> New appctl 'dpdk/get-memzone-stats' implemented to get result of >>> 'rte_memzone_dump()' function. >>> >>> Could be used for debugging. >>> >>> Signed-off-by: Eli Britstein <elibr@nvidia.com> >>> Acked-by: Roi Dayan <roid@nvidia.com> >>> --- >>> lib/dpdk.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >> >> Hi Roi, >> >> Code LGTM. It needs an update to dpdk-unixctl.man and NEWS so users can >> see it's added. >> >> See Eli's previous commit as example: >> https://github.com/openvswitch/ovs/commit/ad256c31407d347eb185b39cb8fefc9df607e3ed >> >> (it also needs a small rebase without mempool-stats patch) >> >> thanks, >> Kevin. >> >>> diff --git a/lib/dpdk.c b/lib/dpdk.c >>> index aaf0b2ed8046..b585327ea971 100644 >>> --- a/lib/dpdk.c >>> +++ b/lib/dpdk.c >>> @@ -324,6 +324,12 @@ malloc_dump_mempool_stats_wrapper(FILE *stream) >>> rte_mempool_walk(dump_mempool_stats, stream); >>> } >>> >>> +static void >>> +malloc_dump_memzone_stats_wrapper(FILE *stream) >>> +{ >>> + rte_memzone_dump(stream); >>> +} > > This function also seems redundant. Unless there are some compiler > warnings, it should be possible to pass rte_memzone_dump directly to > the dpdk_unixctl_mem_stream without a wrapper. > > Best regards, Ilya Maximets. > Thanks. I'll send v2 with updated and rebased patch and the oom-score patch. >>> + >>> static bool >>> dpdk_init__(const struct smap *ovs_other_config) >>> { >>> @@ -454,6 +460,9 @@ dpdk_init__(const struct smap *ovs_other_config) >>> unixctl_command_register("dpdk/get-mempool-stats", "", 0, 0, >>> dpdk_unixctl_mem_stream, >>> malloc_dump_mempool_stats_wrapper); >>> + unixctl_command_register("dpdk/get-memzone-stats", "", 0, 0, >>> + dpdk_unixctl_mem_stream, >>> + malloc_dump_memzone_stats_wrapper); >>> >>> /* We are called from the main thread here */ >>> RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID; >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/lib/dpdk.c b/lib/dpdk.c index aaf0b2ed8046..b585327ea971 100644 --- a/lib/dpdk.c +++ b/lib/dpdk.c @@ -324,6 +324,12 @@ malloc_dump_mempool_stats_wrapper(FILE *stream) rte_mempool_walk(dump_mempool_stats, stream); } +static void +malloc_dump_memzone_stats_wrapper(FILE *stream) +{ + rte_memzone_dump(stream); +} + static bool dpdk_init__(const struct smap *ovs_other_config) { @@ -454,6 +460,9 @@ dpdk_init__(const struct smap *ovs_other_config) unixctl_command_register("dpdk/get-mempool-stats", "", 0, 0, dpdk_unixctl_mem_stream, malloc_dump_mempool_stats_wrapper); + unixctl_command_register("dpdk/get-memzone-stats", "", 0, 0, + dpdk_unixctl_mem_stream, + malloc_dump_memzone_stats_wrapper); /* We are called from the main thread here */ RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;