Re: [PATCH] app/procinfo: add device private info dump

[Stephen Hemminger <stephen@networkplumber.org>]

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.