app/procinfo: add device private info dump
Checks
Commit Message
This patch adds support for dump the device private info from a running
application. It can help developers locate the problem.
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
app/proc-info/main.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
Comments
On Sat, 19 Feb 2022 09:59:16 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:
> +static void
> +show_port_private_info(void)
> +{
> + int i;
> +
> + snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
> + STATS_BDR_STR(10, bdr_str);
> +
> + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> + /* Skip if port is not in mask */
> + if ((enabled_port_mask & (1ul << i)) == 0)
> + continue;
> +
> + /* Skip if port is unused */
> + if (!rte_eth_dev_is_valid_port(i))
> + continue;
Maybe use RTE_ETH_FOREACH_DEV(i) here?
Procinfo is somewhat inconsistent, some code uses, and some does not.
The difference is that FOREACH skips ports that are "owned" i.e
associated with another port.
There probably should be a clear policy in the comments about
how this command should handle ports. My preference would be
that it shows all valid ports, all the time since this is a diagnostic
command used to debug misconfiguration.
There is RTE_ETH_FOREACH_VALID_DEV but it is marked internal?
20/02/2022 02:04, Stephen Hemminger:
> On Sat, 19 Feb 2022 09:59:16 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
>
> > +static void
> > +show_port_private_info(void)
> > +{
> > + int i;
> > +
> > + snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
> > + STATS_BDR_STR(10, bdr_str);
> > +
> > + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> > + /* Skip if port is not in mask */
> > + if ((enabled_port_mask & (1ul << i)) == 0)
> > + continue;
> > +
> > + /* Skip if port is unused */
> > + if (!rte_eth_dev_is_valid_port(i))
> > + continue;
>
> Maybe use RTE_ETH_FOREACH_DEV(i) here?
>
> Procinfo is somewhat inconsistent, some code uses, and some does not.
> The difference is that FOREACH skips ports that are "owned" i.e
> associated with another port.
Yes RTE_ETH_FOREACH_DEV is for general usage,
you get only the ports you are supposed to manage.
> There probably should be a clear policy in the comments about
> how this command should handle ports. My preference would be
> that it shows all valid ports, all the time since this is a diagnostic
> command used to debug misconfiguration.
>
> There is RTE_ETH_FOREACH_VALID_DEV but it is marked internal?
Yes, you get it right, RTE_ETH_FOREACH_VALID_DEV gets all ports
and that should be used only internally or for debugging.
If we expose it for debugging purpose, there is a risk of confusion.
The goal was to "force" applications to adopt good behaviour,
using RTE_ETH_FOREACH_DEV.
It means RTE_MAX_ETHPORTS must be used for debugging.
Is it a good decision?
Hi,
在 2022/2/20 16:56, Thomas Monjalon 写道:
> 20/02/2022 02:04, Stephen Hemminger:
>> On Sat, 19 Feb 2022 09:59:16 +0800
>> "Min Hu (Connor)" <humin29@huawei.com> wrote:
>>
>>> +static void
>>> +show_port_private_info(void)
>>> +{
>>> + int i;
>>> +
>>> + snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
>>> + STATS_BDR_STR(10, bdr_str);
>>> +
>>> + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
>>> + /* Skip if port is not in mask */
>>> + if ((enabled_port_mask & (1ul << i)) == 0)
>>> + continue;
>>> +
>>> + /* Skip if port is unused */
>>> + if (!rte_eth_dev_is_valid_port(i))
>>> + continue;
>>
>> Maybe use RTE_ETH_FOREACH_DEV(i) here?
>>
>> Procinfo is somewhat inconsistent, some code uses, and some does not.
>> The difference is that FOREACH skips ports that are "owned" i.e
>> associated with another port.
>
> Yes RTE_ETH_FOREACH_DEV is for general usage,
> you get only the ports you are supposed to manage.
>
>> There probably should be a clear policy in the comments about
>> how this command should handle ports. My preference would be
>> that it shows all valid ports, all the time since this is a diagnostic
>> command used to debug misconfiguration.
>>
>> There is RTE_ETH_FOREACH_VALID_DEV but it is marked internal?
>
> Yes, you get it right, RTE_ETH_FOREACH_VALID_DEV gets all ports
> and that should be used only internally or for debugging.
> If we expose it for debugging purpose, there is a risk of confusion.
> The goal was to "force" applications to adopt good behaviour,
> using RTE_ETH_FOREACH_DEV.
Agree with using RTE_ETH_FOREACH_DEV, v2 has been sent out.
> It means RTE_MAX_ETHPORTS must be used for debugging.
> Is it a good decision?
>
>
> .
>
On Mon, 21 Feb 2022 10:26:38 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:
> Hi,
>
> 在 2022/2/20 16:56, Thomas Monjalon 写道:
> > 20/02/2022 02:04, Stephen Hemminger:
> >> On Sat, 19 Feb 2022 09:59:16 +0800
> >> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> >>
> >>> +static void
> >>> +show_port_private_info(void)
> >>> +{
> >>> + int i;
> >>> +
> >>> + snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
> >>> + STATS_BDR_STR(10, bdr_str);
> >>> +
> >>> + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> >>> + /* Skip if port is not in mask */
> >>> + if ((enabled_port_mask & (1ul << i)) == 0)
> >>> + continue;
> >>> +
> >>> + /* Skip if port is unused */
> >>> + if (!rte_eth_dev_is_valid_port(i))
> >>> + continue;
> >>
> >> Maybe use RTE_ETH_FOREACH_DEV(i) here?
> >>
> >> Procinfo is somewhat inconsistent, some code uses, and some does not.
> >> The difference is that FOREACH skips ports that are "owned" i.e
> >> associated with another port.
> >
> > Yes RTE_ETH_FOREACH_DEV is for general usage,
> > you get only the ports you are supposed to manage.
> >
> >> There probably should be a clear policy in the comments about
> >> how this command should handle ports. My preference would be
> >> that it shows all valid ports, all the time since this is a diagnostic
> >> command used to debug misconfiguration.
> >>
> >> There is RTE_ETH_FOREACH_VALID_DEV but it is marked internal?
> >
> > Yes, you get it right, RTE_ETH_FOREACH_VALID_DEV gets all ports
> > and that should be used only internally or for debugging.
> > If we expose it for debugging purpose, there is a risk of confusion.
> > The goal was to "force" applications to adopt good behaviour,
> > using RTE_ETH_FOREACH_DEV.
> Agree with using RTE_ETH_FOREACH_DEV, v2 has been sent out.
>
> > It means RTE_MAX_ETHPORTS must be used for debugging.
> > Is it a good decision?
> >
> >
> > .
> >
Maybe procinfo should have a flag (--all) to show all devices.
HI,
在 2022/2/22 1:04, Stephen Hemminger 写道:
> On Mon, 21 Feb 2022 10:26:38 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
>
>> Hi,
>>
>> 在 2022/2/20 16:56, Thomas Monjalon 写道:
>>> 20/02/2022 02:04, Stephen Hemminger:
>>>> On Sat, 19 Feb 2022 09:59:16 +0800
>>>> "Min Hu (Connor)" <humin29@huawei.com> wrote:
>>>>
>>>>> +static void
>>>>> +show_port_private_info(void)
>>>>> +{
>>>>> + int i;
>>>>> +
>>>>> + snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
>>>>> + STATS_BDR_STR(10, bdr_str);
>>>>> +
>>>>> + for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
>>>>> + /* Skip if port is not in mask */
>>>>> + if ((enabled_port_mask & (1ul << i)) == 0)
>>>>> + continue;
>>>>> +
>>>>> + /* Skip if port is unused */
>>>>> + if (!rte_eth_dev_is_valid_port(i))
>>>>> + continue;
>>>>
>>>> Maybe use RTE_ETH_FOREACH_DEV(i) here?
>>>>
>>>> Procinfo is somewhat inconsistent, some code uses, and some does not.
>>>> The difference is that FOREACH skips ports that are "owned" i.e
>>>> associated with another port.
>>>
>>> Yes RTE_ETH_FOREACH_DEV is for general usage,
>>> you get only the ports you are supposed to manage.
>>>
>>>> There probably should be a clear policy in the comments about
>>>> how this command should handle ports. My preference would be
>>>> that it shows all valid ports, all the time since this is a diagnostic
>>>> command used to debug misconfiguration.
>>>>
>>>> There is RTE_ETH_FOREACH_VALID_DEV but it is marked internal?
>>>
>>> Yes, you get it right, RTE_ETH_FOREACH_VALID_DEV gets all ports
>>> and that should be used only internally or for debugging.
>>> If we expose it for debugging purpose, there is a risk of confusion.
>>> The goal was to "force" applications to adopt good behaviour,
>>> using RTE_ETH_FOREACH_DEV.
>> Agree with using RTE_ETH_FOREACH_DEV, v2 has been sent out.
>>
>>> It means RTE_MAX_ETHPORTS must be used for debugging.
>>> Is it a good decision?
>>>
>>>
>>> .
>>>
>
> Maybe procinfo should have a flag (--all) to show all devices.
How about keep the patch as v1 shows, like that:
+ for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ /* Skip if port is not in mask */
+ if ((enabled_port_mask & (1ul << i)) == 0)
+ continue;
+
+ /* Skip if port is unused */
+ if (!rte_eth_dev_is_valid_port(i))
+ continue;
This can show all devices.
> .
>
@@ -84,6 +84,8 @@ static char bdr_str[MAX_STRING_LEN];
/**< Enable show port. */
static uint32_t enable_shw_port;
+/**< Enable show port. */
+static uint32_t enable_shw_port_priv;
/**< Enable show tm. */
static uint32_t enable_shw_tm;
/**< Enable show crypto. */
@@ -123,6 +125,7 @@ proc_info_usage(const char *prgname)
" --collectd-format: to print statistics to STDOUT in expected by collectd format\n"
" --host-id STRING: host id used to identify the system process is running on\n"
" --show-port: to display ports information\n"
+ " --show-port-private: to display ports private information\n"
" --show-tm: to display traffic manager information for ports\n"
" --show-crypto: to display crypto information\n"
" --show-ring[=name]: to display ring information\n"
@@ -232,6 +235,7 @@ proc_info_parse_args(int argc, char **argv)
{"xstats-ids", 1, NULL, 1},
{"host-id", 0, NULL, 0},
{"show-port", 0, NULL, 0},
+ {"show-port-private", 0, NULL, 0},
{"show-tm", 0, NULL, 0},
{"show-crypto", 0, NULL, 0},
{"show-ring", optional_argument, NULL, 0},
@@ -284,6 +288,9 @@ proc_info_parse_args(int argc, char **argv)
else if (!strncmp(long_option[option_index].name,
"show-port", MAX_LONG_OPT_SZ))
enable_shw_port = 1;
+ else if (!strncmp(long_option[option_index].name,
+ "show-port-private", MAX_LONG_OPT_SZ))
+ enable_shw_port_priv = 1;
else if (!strncmp(long_option[option_index].name,
"show-tm", MAX_LONG_OPT_SZ))
enable_shw_tm = 1;
@@ -887,6 +894,29 @@ show_port(void)
}
}
+static void
+show_port_private_info(void)
+{
+ int i;
+
+ snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD Private ");
+ STATS_BDR_STR(10, bdr_str);
+
+ for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ /* Skip if port is not in mask */
+ if ((enabled_port_mask & (1ul << i)) == 0)
+ continue;
+
+ /* Skip if port is unused */
+ if (!rte_eth_dev_is_valid_port(i))
+ continue;
+
+ snprintf(bdr_str, MAX_STRING_LEN, " Port %u ", i);
+ STATS_BDR_STR(5, bdr_str);
+ rte_eth_dev_priv_dump(i, stdout);
+ }
+}
+
static void
display_nodecap_info(int is_leaf, struct rte_tm_node_capabilities *cap)
{
@@ -1549,6 +1579,8 @@ main(int argc, char **argv)
/* show information for PMD */
if (enable_shw_port)
show_port();
+ if (enable_shw_port_priv)
+ show_port_private_info();
if (enable_shw_tm)
show_tm();
if (enable_shw_crypto)