RE: [PATCH v2] app/procinfo: show all non-owned ports

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 25 May 2022 08.04
> 
> On Tue, 24 May 2022 22:46:05 -0700
> Subendu Santra <subendu@arista.com> wrote:
> 
> > Show all non-owned ports when no port mask is specified
> >
> > show-port option without the mask option, displays only the last
> > non-owned port. Show all the non-owned ports instead.
> >
> > Fixes: 1dd6cffb6571 ("app/procinfo: provide way to request info on
> owned
> > ports")
> > Cc: stephen@networkplumber.org
> >
> > Signed-off-by: Subendu Santra <subendu@arista.com>
> > ---
> >  app/proc-info/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> > index 56070a3317..2be24b584e 100644
> > --- a/app/proc-info/main.c
> > +++ b/app/proc-info/main.c
> > @@ -1504,10 +1504,10 @@ main(int argc, char **argv)
> >  	if (nb_ports == 0)
> >  		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
> >
> > -	/* If no port mask was specified, then show non-owned ports */
> > +	/* If no port mask was specified, then show all non-owned ports
> */
> >  	if (enabled_port_mask == 0) {
> >  		RTE_ETH_FOREACH_DEV(i)
> > -			enabled_port_mask = 1ul << i;
> > +			enabled_port_mask |= (1ul << i);
> 
> Ok, looks good. parens on that line are unnecessary
> 
> Note: this still will have issues with >32 ports on 32 bit platforms.

The default max_ethports value in meson_options.txt is 32, so the probability is low.

> But other tools probably have same problem.

It was decided many years ago to extend the port_id type from uint8_t to uint16_t, mainly to support a high number of virtual ports. So it is not good that the applications have not been updated accordingly.

However, as Stephen also mentions, this is not unique to this tool, so we'll just ignore it.

> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Morten Brørup <mb@smartsharesystems.com>