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); } for (i = 0; i < RTE_MAX_ETHPORTS; i++) { -- 2.28.0
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); } for (i = 0; i < RTE_MAX_ETHPORTS; i++) { -- 2.28.0
I have updated the v2 of the patch with the explanation.
Kindly advise if it looks ok.
Regards,
Subendu.
On Wed, May 25, 2022 at 11:22 AM 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);
> }
>
> for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> --
> 2.28.0
>
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.
But other tools probably have same problem.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 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>
Hi Thomas,
How does this patch look? Is there anything that needs to be done from my side?
Regards,
Subendu.
On Wed, May 25, 2022 at 12:44 PM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > 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>
>
25/05/2022 09:14, Morten Brørup: > > 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. I think it is easier to understand if using past tense for the previous behaviour. I reword it like this: The show-port option, without the mask option, was showing 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 + Cc: stable@dpdk.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 parens removed > > > > 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> Applied with small changes, thanks.