DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2] app/procinfo: show all non-owned ports
@ 2022-05-25  5:46 Subendu Santra
  2022-05-25  6:04 ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Subendu Santra @ 2022-05-25  5:46 UTC (permalink / raw)
  To: Maryam Tahhan, Reshma Pattan, Stephen Hemminger, Hemant Agrawal
  Cc: dev, Subendu Santra

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-25  5:46 [PATCH v2] app/procinfo: show all non-owned ports Subendu Santra
@ 2022-05-25  6:04 ` Stephen Hemminger
  2022-05-25  7:14   ` Morten Brørup
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2022-05-25  6:04 UTC (permalink / raw)
  To: Subendu Santra; +Cc: Maryam Tahhan, Reshma Pattan, Hemant Agrawal, dev

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>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-25  6:04 ` Stephen Hemminger
@ 2022-05-25  7:14   ` Morten Brørup
  2022-06-06 14:20     ` Subendu Santra
  2022-06-26 15:26     ` Thomas Monjalon
  0 siblings, 2 replies; 7+ messages in thread
From: Morten Brørup @ 2022-05-25  7:14 UTC (permalink / raw)
  To: Stephen Hemminger, Subendu Santra
  Cc: Maryam Tahhan, Reshma Pattan, Hemant Agrawal, dev

> 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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-25  7:14   ` Morten Brørup
@ 2022-06-06 14:20     ` Subendu Santra
  2022-06-26 15:26     ` Thomas Monjalon
  1 sibling, 0 replies; 7+ messages in thread
From: Subendu Santra @ 2022-06-06 14:20 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Stephen Hemminger, Maryam Tahhan, Reshma Pattan, Hemant Agrawal,
	dev, Morten Brørup

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>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-25  7:14   ` Morten Brørup
  2022-06-06 14:20     ` Subendu Santra
@ 2022-06-26 15:26     ` Thomas Monjalon
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2022-06-26 15:26 UTC (permalink / raw)
  To: Subendu Santra
  Cc: Stephen Hemminger, Maryam Tahhan, Reshma Pattan, Hemant Agrawal,
	dev, Morten Brørup

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-25  5:52 ` [PATCH v2] " Subendu Santra
@ 2022-05-25  5:58   ` Subendu Santra
  0 siblings, 0 replies; 7+ messages in thread
From: Subendu Santra @ 2022-05-25  5:58 UTC (permalink / raw)
  To: Reshma Pattan, Stephen Hemminger, Hemant Agrawal, Thomas Monjalon; +Cc: dev

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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] app/procinfo: show all non-owned ports
  2022-05-10 13:21 [PATCH] " Subendu Santra
@ 2022-05-25  5:52 ` Subendu Santra
  2022-05-25  5:58   ` Subendu Santra
  0 siblings, 1 reply; 7+ messages in thread
From: Subendu Santra @ 2022-05-25  5:52 UTC (permalink / raw)
  To: Maryam Tahhan, Reshma Pattan, Stephen Hemminger, Hemant Agrawal
  Cc: dev, Subendu Santra

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-26 15:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25  5:46 [PATCH v2] app/procinfo: show all non-owned ports Subendu Santra
2022-05-25  6:04 ` Stephen Hemminger
2022-05-25  7:14   ` Morten Brørup
2022-06-06 14:20     ` Subendu Santra
2022-06-26 15:26     ` Thomas Monjalon
  -- strict thread matches above, loose matches on Subject: below --
2022-05-10 13:21 [PATCH] " Subendu Santra
2022-05-25  5:52 ` [PATCH v2] " Subendu Santra
2022-05-25  5:58   ` Subendu Santra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).