DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
@ 2022-04-24  5:34 Subendu Santra
  2022-05-03  5:29 ` Subendu Santra
  2022-05-03  8:47 ` Thomas Monjalon
  0 siblings, 2 replies; 12+ messages in thread
From: Subendu Santra @ 2022-04-24  5:34 UTC (permalink / raw)
  To: stephen
  Cc: dev, hemant.agrawal, maryam.tahhan, reshma.pattan, Sriram Rajagopalan

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi Stephen,

We were going through the patch set: https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/ and hoping to get clarification on the behaviour if post mask is not specified in the input to `dpdk-proc-info` tool.

Specifically, In PATCH v3 6/7, we see this:
+	/* If no port mask was specified, one will be provided */
+	if (enabled_port_mask == 0) {
+		RTE_ETH_FOREACH_DEV(i) {
+			enabled_port_mask |= 1u << i;

However, in PATCH v4 8/8, we see this:
+	/* If no port mask was specified, then show non-owned ports */
+	if (enabled_port_mask == 0) {
+		RTE_ETH_FOREACH_DEV(i)
+			enabled_port_mask = 1ul << i;
+	}

Was there any specific reason to show just the last non-owned port in case the port mask was not specified?
Should we show all non-owned ports in case the user doesn’t specify any port mask?

Regards,
Subendu.




[-- Attachment #2: Type: text/html, Size: 2119 bytes --]

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-04-24  5:34 [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports Subendu Santra
@ 2022-05-03  5:29 ` Subendu Santra
  2022-05-03  8:47 ` Thomas Monjalon
  1 sibling, 0 replies; 12+ messages in thread
From: Subendu Santra @ 2022-05-03  5:29 UTC (permalink / raw)
  To: stephen
  Cc: dev, hemant.agrawal, maryam.tahhan, reshma.pattan, Sriram Rajagopalan

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

Hi Stephen,

Could you please help us understand the rationale behind showing just the
last non-owned port in case the port mask was not specified?
I really appreciate your help in this regard.

Regards,
Subendu.



On Sun, Apr 24, 2022 at 11:04 AM Subendu Santra <subendu@arista.com> wrote:

> Hi Stephen,
>
> We were going through the patch set:
> https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/
> and hoping to get clarification on the behaviour if post mask is not
> specified in the input to `dpdk-proc-info` tool.
>
> Specifically, In PATCH v3 6/7, we see this:
>
> +	/* If no port mask was specified, one will be provided */
> +	if (enabled_port_mask == 0) {
> +		RTE_ETH_FOREACH_DEV(i) {
> +			enabled_port_mask |= 1u << i;
>
>
> However, in PATCH v4 8/8, we see this:
>
> +	/* If no port mask was specified, then show non-owned ports */
> +	if (enabled_port_mask == 0) {
> +		RTE_ETH_FOREACH_DEV(i)
> +			enabled_port_mask = 1ul << i;
> +	}
>
>
> Was there any specific reason to show just the last non-owned port in case
> the port mask was not specified?
> Should we show all non-owned ports in case the user doesn’t specify any
> port mask?
>
> Regards,
> Subendu.
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 2373 bytes --]

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-04-24  5:34 [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports Subendu Santra
  2022-05-03  5:29 ` Subendu Santra
@ 2022-05-03  8:47 ` Thomas Monjalon
  2022-05-04 17:48   ` Stephen Hemminger
  1 sibling, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2022-05-03  8:47 UTC (permalink / raw)
  To: Subendu Santra
  Cc: stephen, dev, hemant.agrawal, maryam.tahhan, reshma.pattan,
	Sriram Rajagopalan

24/04/2022 07:34, Subendu Santra:
> Hi Stephen,
> 
> We were going through the patch set: https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/ and hoping to get clarification on the behaviour if post mask is not specified in the input to `dpdk-proc-info` tool.
> 
> Specifically, In PATCH v3 6/7, we see this:
> +	/* If no port mask was specified, one will be provided */
> +	if (enabled_port_mask == 0) {
> +		RTE_ETH_FOREACH_DEV(i) {
> +			enabled_port_mask |= 1u << i;
> 
> However, in PATCH v4 8/8, we see this:
> +	/* If no port mask was specified, then show non-owned ports */
> +	if (enabled_port_mask == 0) {
> +		RTE_ETH_FOREACH_DEV(i)
> +			enabled_port_mask = 1ul << i;
> +	}
> 
> Was there any specific reason to show just the last non-owned port in case the port mask was not specified?
> Should we show all non-owned ports in case the user doesn’t specify any port mask?

It looks like a bug. It should be |=
Feel free to send a fix.



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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-05-03  8:47 ` Thomas Monjalon
@ 2022-05-04 17:48   ` Stephen Hemminger
  2022-05-10  9:09     ` Subendu Santra
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2022-05-04 17:48 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Subendu Santra, dev, hemant.agrawal, maryam.tahhan,
	reshma.pattan, Sriram Rajagopalan

On Tue, 03 May 2022 10:47:58 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 24/04/2022 07:34, Subendu Santra:
> > Hi Stephen,
> > 
> > We were going through the patch set: https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/ and hoping to get clarification on the behaviour if post mask is not specified in the input to `dpdk-proc-info` tool.
> > 
> > Specifically, In PATCH v3 6/7, we see this:
> > +	/* If no port mask was specified, one will be provided */
> > +	if (enabled_port_mask == 0) {
> > +		RTE_ETH_FOREACH_DEV(i) {
> > +			enabled_port_mask |= 1u << i;
> > 
> > However, in PATCH v4 8/8, we see this:
> > +	/* If no port mask was specified, then show non-owned ports */
> > +	if (enabled_port_mask == 0) {
> > +		RTE_ETH_FOREACH_DEV(i)
> > +			enabled_port_mask = 1ul << i;
> > +	}
> > 
> > Was there any specific reason to show just the last non-owned port in case the port mask was not specified?
> > Should we show all non-owned ports in case the user doesn’t specify any port mask?  
> 
> It looks like a bug. It should be |=
> Feel free to send a fix.
> 
> 

Agree. Thats a bug.

It would be good to have a "show all ports" flag to proc-info.
To show all ports including owned.

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-05-04 17:48   ` Stephen Hemminger
@ 2022-05-10  9:09     ` Subendu Santra
  2022-05-10 20:02       ` Stephen Hemminger
  0 siblings, 1 reply; 12+ messages in thread
From: Subendu Santra @ 2022-05-10  9:09 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Thomas Monjalon, dev, hemant.agrawal, maryam.tahhan,
	reshma.pattan, Sriram Rajagopalan

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

Hi Stephen, Thomas,

On a related note w.r.to commit 1dd6cffb6571f816d5a0d1fd620f43532240b40b
(app/procinfo: provide way to request info on owned ports), we see this
change:

-static uint32_t enabled_port_mask;
> +static unsigned long enabled_port_mask;


While this is ok for 64-bit machines, where unsigned long is 64-bit, on
32-bit machines unsigned long is 32-bits.
Should we change this to unsigned long long which is guaranteed to be
64-bits on both architectures?

Specifying a mask of 0xffffffffffffffff on 32-bit platforms results in
error:

> + sudo /usr/share/dpdk/tools/dpdk-procinfo -- --show-port -p
> 0xffffffffffffffff
> Invalid portmask '0xffffffffffffffff'


We have a script that runs periodically and it uses the dpdk-procinfo tool
to collect information about the ports.
It will be ideal to use the same portmask in the script irrespective of the
platform it runs on.

Kindly share your thoughts on this.

Regards,
Subendu.



On Wed, May 4, 2022 at 11:18 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Tue, 03 May 2022 10:47:58 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 24/04/2022 07:34, Subendu Santra:
> > > Hi Stephen,
> > >
> > > We were going through the patch set:
> https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/
> and hoping to get clarification on the behaviour if post mask is not
> specified in the input to `dpdk-proc-info` tool.
> > >
> > > Specifically, In PATCH v3 6/7, we see this:
> > > +   /* If no port mask was specified, one will be provided */
> > > +   if (enabled_port_mask == 0) {
> > > +           RTE_ETH_FOREACH_DEV(i) {
> > > +                   enabled_port_mask |= 1u << i;
> > >
> > > However, in PATCH v4 8/8, we see this:
> > > +   /* If no port mask was specified, then show non-owned ports */
> > > +   if (enabled_port_mask == 0) {
> > > +           RTE_ETH_FOREACH_DEV(i)
> > > +                   enabled_port_mask = 1ul << i;
> > > +   }
> > >
> > > Was there any specific reason to show just the last non-owned port in
> case the port mask was not specified?
> > > Should we show all non-owned ports in case the user doesn’t specify
> any port mask?
> >
> > It looks like a bug. It should be |=
> > Feel free to send a fix.
> >
> >
>
> Agree. Thats a bug.
>
> It would be good to have a "show all ports" flag to proc-info.
> To show all ports including owned.
>

[-- Attachment #2: Type: text/html, Size: 4213 bytes --]

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-05-10  9:09     ` Subendu Santra
@ 2022-05-10 20:02       ` Stephen Hemminger
  2022-05-11  7:36         ` Subendu Santra
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2022-05-10 20:02 UTC (permalink / raw)
  To: Subendu Santra
  Cc: Thomas Monjalon, dev, hemant.agrawal, maryam.tahhan,
	reshma.pattan, Sriram Rajagopalan

On Tue, 10 May 2022 14:39:05 +0530
Subendu Santra <subendu@arista.com> wrote:

> Hi Stephen, Thomas,
> 
> On a related note w.r.to commit 1dd6cffb6571f816d5a0d1fd620f43532240b40b
> (app/procinfo: provide way to request info on owned ports), we see this
> change:
> 
> -static uint32_t enabled_port_mask;
> > +static unsigned long enabled_port_mask;  
> 
> 
> While this is ok for 64-bit machines, where unsigned long is 64-bit, on
> 32-bit machines unsigned long is 32-bits.
> Should we change this to unsigned long long which is guaranteed to be
> 64-bits on both architectures?
> 
> Specifying a mask of 0xffffffffffffffff on 32-bit platforms results in
> error:
> 
> > + sudo /usr/share/dpdk/tools/dpdk-procinfo -- --show-port -p
> > 0xffffffffffffffff
> > Invalid portmask '0xffffffffffffffff'  
> 
> 
> We have a script that runs periodically and it uses the dpdk-procinfo tool
> to collect information about the ports.
> It will be ideal to use the same portmask in the script irrespective of the
> platform it runs on.
> 
> Kindly share your thoughts on this.
> 
> Regards,
> Subendu.
> 
> 
> 
> On Wed, May 4, 2022 at 11:18 PM Stephen Hemminger <
> stephen@networkplumber.org> wrote:  
> 
> > On Tue, 03 May 2022 10:47:58 +0200
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> >  
> > > 24/04/2022 07:34, Subendu Santra:  
> > > > Hi Stephen,
> > > >
> > > > We were going through the patch set:  
> > https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/
> > and hoping to get clarification on the behaviour if post mask is not
> > specified in the input to `dpdk-proc-info` tool.  
> > > >
> > > > Specifically, In PATCH v3 6/7, we see this:
> > > > +   /* If no port mask was specified, one will be provided */
> > > > +   if (enabled_port_mask == 0) {
> > > > +           RTE_ETH_FOREACH_DEV(i) {
> > > > +                   enabled_port_mask |= 1u << i;
> > > >
> > > > However, in PATCH v4 8/8, we see this:
> > > > +   /* If no port mask was specified, then show non-owned ports */
> > > > +   if (enabled_port_mask == 0) {
> > > > +           RTE_ETH_FOREACH_DEV(i)
> > > > +                   enabled_port_mask = 1ul << i;
> > > > +   }
> > > >
> > > > Was there any specific reason to show just the last non-owned port in  
> > case the port mask was not specified?  
> > > > Should we show all non-owned ports in case the user doesn’t specify  
> > any port mask?  
> > >
> > > It looks like a bug. It should be |=
> > > Feel free to send a fix.
> > >
> > >  
> >
> > Agree. Thats a bug.
> >
> > It would be good to have a "show all ports" flag to proc-info.
> > To show all ports including owned.
> >  

Using uint64_t is better, but eventually many DPDK utilities need to be
fixed to handle > 64 ports.

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2022-05-10 20:02       ` Stephen Hemminger
@ 2022-05-11  7:36         ` Subendu Santra
  0 siblings, 0 replies; 12+ messages in thread
From: Subendu Santra @ 2022-05-11  7:36 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Thomas Monjalon, dev, hemant.agrawal, maryam.tahhan,
	reshma.pattan, Sriram Rajagopalan

Agreed. I will change it to uint64_t and send it for review.
Thanks for your help.

Regards,
Subendu.


On Wed, May 11, 2022 at 1:32 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 10 May 2022 14:39:05 +0530
> Subendu Santra <subendu@arista.com> wrote:
>
> > Hi Stephen, Thomas,
> >
> > On a related note w.r.to commit 1dd6cffb6571f816d5a0d1fd620f43532240b40b
> > (app/procinfo: provide way to request info on owned ports), we see this
> > change:
> >
> > -static uint32_t enabled_port_mask;
> > > +static unsigned long enabled_port_mask;
> >
> >
> > While this is ok for 64-bit machines, where unsigned long is 64-bit, on
> > 32-bit machines unsigned long is 32-bits.
> > Should we change this to unsigned long long which is guaranteed to be
> > 64-bits on both architectures?
> >
> > Specifying a mask of 0xffffffffffffffff on 32-bit platforms results in
> > error:
> >
> > > + sudo /usr/share/dpdk/tools/dpdk-procinfo -- --show-port -p
> > > 0xffffffffffffffff
> > > Invalid portmask '0xffffffffffffffff'
> >
> >
> > We have a script that runs periodically and it uses the dpdk-procinfo tool
> > to collect information about the ports.
> > It will be ideal to use the same portmask in the script irrespective of the
> > platform it runs on.
> >
> > Kindly share your thoughts on this.
> >
> > Regards,
> > Subendu.
> >
> >
> >
> > On Wed, May 4, 2022 at 11:18 PM Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> >
> > > On Tue, 03 May 2022 10:47:58 +0200
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > > 24/04/2022 07:34, Subendu Santra:
> > > > > Hi Stephen,
> > > > >
> > > > > We were going through the patch set:
> > > https://inbox.dpdk.org/dev/20200715212228.28010-7-stephen@networkplumber.org/
> > > and hoping to get clarification on the behaviour if post mask is not
> > > specified in the input to `dpdk-proc-info` tool.
> > > > >
> > > > > Specifically, In PATCH v3 6/7, we see this:
> > > > > +   /* If no port mask was specified, one will be provided */
> > > > > +   if (enabled_port_mask == 0) {
> > > > > +           RTE_ETH_FOREACH_DEV(i) {
> > > > > +                   enabled_port_mask |= 1u << i;
> > > > >
> > > > > However, in PATCH v4 8/8, we see this:
> > > > > +   /* If no port mask was specified, then show non-owned ports */
> > > > > +   if (enabled_port_mask == 0) {
> > > > > +           RTE_ETH_FOREACH_DEV(i)
> > > > > +                   enabled_port_mask = 1ul << i;
> > > > > +   }
> > > > >
> > > > > Was there any specific reason to show just the last non-owned port in
> > > case the port mask was not specified?
> > > > > Should we show all non-owned ports in case the user doesn’t specify
> > > any port mask?
> > > >
> > > > It looks like a bug. It should be |=
> > > > Feel free to send a fix.
> > > >
> > > >
> > >
> > > Agree. Thats a bug.
> > >
> > > It would be good to have a "show all ports" flag to proc-info.
> > > To show all ports including owned.
> > >
>
> Using uint64_t is better, but eventually many DPDK utilities need to be
> fixed to handle > 64 ports.

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2020-07-21 17:08         ` Thomas Monjalon
@ 2020-07-21 17:37           ` Stephen Hemminger
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2020-07-21 17:37 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: maryam.tahhan, reshma.pattan, hemant.agrawal, dev

On Tue, 21 Jul 2020 19:08:57 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 21/07/2020 19:05, Stephen Hemminger:
> > On Fri, 17 Jul 2020 17:01:00 +0200
> > Thomas Monjalon <thomas@monjalon.net> wrote:  
> > > 15/07/2020 23:22, Stephen Hemminger:  
> > > > +	/* If no port mask was specified, one will be provided */    
> > > 
> > > Would be nice to help the user by printing a port mask
> > > of owned and unowned ports.  
> > 
> > Not needed, since each display already has the port #  
> 
> By default, owned ports are not displayed at all.
> How are we supposed to find them?

You have to pass it in portmask.
The alternative would be to show all ports and mark those that are owned.

The ownership model API does not support a way to ask for either
"tell me which other port owns port X" or "iterate over all the ports owned by Y".
The problem is that the owner id is generic and stored inside the PMD.
The root cause is that the original design of port ownership was targeting
a more general use case that was never used. I would have been happier with
a simpler parent port id index in ethdev instead.

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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2020-07-21 17:05       ` Stephen Hemminger
@ 2020-07-21 17:08         ` Thomas Monjalon
  2020-07-21 17:37           ` Stephen Hemminger
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2020-07-21 17:08 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: maryam.tahhan, reshma.pattan, hemant.agrawal, dev

21/07/2020 19:05, Stephen Hemminger:
> On Fri, 17 Jul 2020 17:01:00 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> > 15/07/2020 23:22, Stephen Hemminger:
> > > +	/* If no port mask was specified, one will be provided */  
> > 
> > Would be nice to help the user by printing a port mask
> > of owned and unowned ports.
> 
> Not needed, since each display already has the port #

By default, owned ports are not displayed at all.
How are we supposed to find them?

> > > +	if (enabled_port_mask == 0) {
> > > +		RTE_ETH_FOREACH_DEV(i) {
> > > +			enabled_port_mask |= 1u << i;
> > >  		}
> > >  	}
> > >  
> > > +	for (port_mask = enabled_port_mask; port_mask != 0;
> > > +	     port_mask &= ~(1u << i)) {  
> > 
> > Please would be good to help drunk or sleepy readers with few comments.
> ok?

Comments in the code I mean.

> > > +		/* ffs() first bit is 1 not 0 */
> > > +		i = ffs(port_mask) - 1;
> > > +
> > > +		if (i >= RTE_MAX_ETHPORTS)
> > > +			break;  
> > 
> > This check is already done in rte_eth_dev_is_valid_port().
> 
> Want to stop early if port is out of range,
> but continue if hits a port that is owned.

OK

> > > +
> > > +		if (!rte_eth_dev_is_valid_port(i))
> > > +			continue;  




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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2020-07-17 15:01     ` Thomas Monjalon
@ 2020-07-21 17:05       ` Stephen Hemminger
  2020-07-21 17:08         ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2020-07-21 17:05 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: maryam.tahhan, reshma.pattan, hemant.agrawal, dev

On Fri, 17 Jul 2020 17:01:00 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 15/07/2020 23:22, Stephen Hemminger:
> > --- a/app/proc-info/Makefile
> > +++ b/app/proc-info/Makefile
> > +CFLAGS += -DALLOW_EXPERIMENTAL_API  
> 
> not needed in app/
> 
> > +CFLAGS += -O3
> >  CFLAGS += $(WERROR_FLAGS)
> > +CFLAGS += -Wno-deprecated-declarations  
> 
> Which deprecated function is used?
> We must not use deprecated functions.

Fixing.

> 
> > +	/* If no port mask was specified, one will be provided */  
> 
> Would be nice to help the user by printing a port mask
> of owned and unowned ports.

Not needed, since each display already has the port #

> 
> > +	if (enabled_port_mask == 0) {
> > +		RTE_ETH_FOREACH_DEV(i) {
> > +			enabled_port_mask |= 1u << i;
> >  		}
> >  	}
> >  
> > +	for (port_mask = enabled_port_mask; port_mask != 0;
> > +	     port_mask &= ~(1u << i)) {  
> 
> Please would be good to help drunk or sleepy readers with few comments.
ok?

> 
> > +		/* ffs() first bit is 1 not 0 */
> > +		i = ffs(port_mask) - 1;
> > +
> > +		if (i >= RTE_MAX_ETHPORTS)
> > +			break;  
> 
> This check is already done in rte_eth_dev_is_valid_port().

Want to stop early if port is out of range,
but continue if hits a port that is owned.


> 
> > +
> > +		if (!rte_eth_dev_is_valid_port(i))
> > +			continue;  
> 
> 
> 


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

* Re: [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2020-07-15 21:22   ` [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports Stephen Hemminger
@ 2020-07-17 15:01     ` Thomas Monjalon
  2020-07-21 17:05       ` Stephen Hemminger
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2020-07-17 15:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: maryam.tahhan, reshma.pattan, hemant.agrawal, dev

15/07/2020 23:22, Stephen Hemminger:
> --- a/app/proc-info/Makefile
> +++ b/app/proc-info/Makefile
> +CFLAGS += -DALLOW_EXPERIMENTAL_API

not needed in app/

> +CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -Wno-deprecated-declarations

Which deprecated function is used?
We must not use deprecated functions.

> +	/* If no port mask was specified, one will be provided */

Would be nice to help the user by printing a port mask
of owned and unowned ports.

> +	if (enabled_port_mask == 0) {
> +		RTE_ETH_FOREACH_DEV(i) {
> +			enabled_port_mask |= 1u << i;
>  		}
>  	}
>  
> +	for (port_mask = enabled_port_mask; port_mask != 0;
> +	     port_mask &= ~(1u << i)) {

Please would be good to help drunk or sleepy readers with few comments.

> +		/* ffs() first bit is 1 not 0 */
> +		i = ffs(port_mask) - 1;
> +
> +		if (i >= RTE_MAX_ETHPORTS)
> +			break;

This check is already done in rte_eth_dev_is_valid_port().

> +
> +		if (!rte_eth_dev_is_valid_port(i))
> +			continue;




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

* [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports
  2020-07-15 21:22 ` [dpdk-dev] [PATCH v3 0/7] app/proc-info enhancments Stephen Hemminger
@ 2020-07-15 21:22   ` Stephen Hemminger
  2020-07-17 15:01     ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2020-07-15 21:22 UTC (permalink / raw)
  To: maryam.tahhan, reshma.pattan, hemant.agrawal; +Cc: dev, Stephen Hemminger

There are cases where a port maybe owned by another (failsafe, netvsc,
bond); but currently proc-info has no way to look at stats of those
ports. This patch provides way for the user to explicitly ask for these
ports.

If no portmask is given the output is unchanged; it only shows the
top level ports. If portmask requests a specific port it will be
shown even if owned.

The device owner is also a useful thing to show in port info.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 app/proc-info/Makefile |  3 ++
 app/proc-info/main.c   | 74 ++++++++++++++++++++++++++++--------------
 2 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/app/proc-info/Makefile b/app/proc-info/Makefile
index 214f3f54a1e9..f777e037861f 100644
--- a/app/proc-info/Makefile
+++ b/app/proc-info/Makefile
@@ -5,7 +5,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 APP = dpdk-procinfo
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -Wno-deprecated-declarations
 
 # all source are stored in SRCS-y
 
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 5000724fcdd1..14c04f8367f1 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -12,6 +12,7 @@
 #include <stdlib.h>
 #include <getopt.h>
 #include <unistd.h>
+#include <strings.h>
 
 #include <rte_eal.h>
 #include <rte_common.h>
@@ -676,19 +677,26 @@ eth_tx_queue_available(uint16_t port_id, uint16_t queue_id, uint16_t n)
 static void
 show_port(void)
 {
-	uint16_t i = 0;
-	int ret = 0, j, k;
+	uint32_t port_mask = enabled_port_mask;
+	int i, ret, j, k;
 
 	snprintf(bdr_str, MAX_STRING_LEN, " show - Port PMD ");
 	STATS_BDR_STR(10, bdr_str);
 
-	RTE_ETH_FOREACH_DEV(i) {
+	for (port_mask = enabled_port_mask; port_mask != 0;
+	     port_mask &= ~(1u << i)) {
 		uint16_t mtu = 0;
 		struct rte_eth_link link;
 		struct rte_eth_dev_info dev_info;
 		struct rte_eth_rss_conf rss_conf;
 		struct rte_eth_fc_conf fc_conf;
 		struct rte_ether_addr mac;
+		struct rte_eth_dev_owner owner;
+
+		i = ffs(port_mask) - 1;
+
+		if (!rte_eth_dev_is_valid_port(i))
+			continue;
 
 		memset(&rss_conf, 0, sizeof(rss_conf));
 
@@ -707,6 +715,11 @@ show_port(void)
 		       dev_info.driver_name, dev_info.device->name,
 		       rte_eth_dev_socket_id(i));
 
+		ret = rte_eth_dev_owner_get(i, &owner);
+		if (ret == 0 && owner.id != RTE_ETH_DEV_NO_OWNER)
+			printf("\t --  owner %#"PRIx64":%s\n",
+			       owner.id, owner.name);
+
 		ret = rte_eth_link_get(i, &link);
 		if (ret < 0) {
 			printf("Link get failed (port %u): %s\n",
@@ -1351,6 +1364,7 @@ main(int argc, char **argv)
 	char log_flag[] = "--log-level=6";
 	char *argp[argc + 4];
 	uint16_t nb_ports;
+	uint32_t port_mask;
 
 	/* preparse app arguments */
 	ret = proc_info_preparse_args(argc, argv);
@@ -1394,30 +1408,42 @@ main(int argc, char **argv)
 	if (nb_ports == 0)
 		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
 
-	/* If no port mask was specified*/
-	if (enabled_port_mask == 0)
-		enabled_port_mask = 0xffff;
-
-	RTE_ETH_FOREACH_DEV(i) {
-		if (enabled_port_mask & (1 << i)) {
-			if (enable_stats)
-				nic_stats_display(i);
-			else if (enable_xstats)
-				nic_xstats_display(i);
-			else if (reset_stats)
-				nic_stats_clear(i);
-			else if (reset_xstats)
-				nic_xstats_clear(i);
-			else if (enable_xstats_name)
-				nic_xstats_by_name_display(i, xstats_name);
-			else if (nb_xstats_ids > 0)
-				nic_xstats_by_ids_display(i, xstats_ids,
-						nb_xstats_ids);
-			else if (enable_metrics)
-				metrics_display(i);
+	/* If no port mask was specified, one will be provided */
+	if (enabled_port_mask == 0) {
+		RTE_ETH_FOREACH_DEV(i) {
+			enabled_port_mask |= 1u << i;
 		}
 	}
 
+	for (port_mask = enabled_port_mask; port_mask != 0;
+	     port_mask &= ~(1u << i)) {
+		/* ffs() first bit is 1 not 0 */
+		i = ffs(port_mask) - 1;
+
+		if (i >= RTE_MAX_ETHPORTS)
+			break;
+
+		if (!rte_eth_dev_is_valid_port(i))
+			continue;
+
+		if (enable_stats)
+			nic_stats_display(i);
+		else if (enable_xstats)
+			nic_xstats_display(i);
+		else if (reset_stats)
+			nic_stats_clear(i);
+		else if (reset_xstats)
+			nic_xstats_clear(i);
+		else if (enable_xstats_name)
+			nic_xstats_by_name_display(i, xstats_name);
+		else if (nb_xstats_ids > 0)
+			nic_xstats_by_ids_display(i, xstats_ids,
+						  nb_xstats_ids);
+		else if (enable_metrics)
+			metrics_display(i);
+
+	}
+
 	/* print port independent stats */
 	if (enable_metrics)
 		metrics_display(RTE_METRICS_GLOBAL);
-- 
2.27.0


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

end of thread, other threads:[~2022-05-11  7:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24  5:34 [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports Subendu Santra
2022-05-03  5:29 ` Subendu Santra
2022-05-03  8:47 ` Thomas Monjalon
2022-05-04 17:48   ` Stephen Hemminger
2022-05-10  9:09     ` Subendu Santra
2022-05-10 20:02       ` Stephen Hemminger
2022-05-11  7:36         ` Subendu Santra
  -- strict thread matches above, loose matches on Subject: below --
2020-05-06 19:37 [dpdk-dev] [PATCH 0/7] proc-info enhancements Stephen Hemminger
2020-07-15 21:22 ` [dpdk-dev] [PATCH v3 0/7] app/proc-info enhancments Stephen Hemminger
2020-07-15 21:22   ` [dpdk-dev] [PATCH v3 6/7] app/proc-info: provide way to request info on owned ports Stephen Hemminger
2020-07-17 15:01     ` Thomas Monjalon
2020-07-21 17:05       ` Stephen Hemminger
2020-07-21 17:08         ` Thomas Monjalon
2020-07-21 17:37           ` Stephen Hemminger

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