20/02/2022 02:04, Stephen Hemminger:
> On Sat, 19 Feb 2022 09:59:16 +0800
> "Min Hu (Connor)" <humi...@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?


Reply via email to