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 <step...@networkplumber.org> wrote: > > On Tue, 10 May 2022 14:39:05 +0530 > Subendu Santra <sube...@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 < > > step...@networkplumber.org> wrote: > > > > > On Tue, 03 May 2022 10:47:58 +0200 > > > Thomas Monjalon <tho...@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-step...@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.
