RE: [PATCH v2] app/testpmd: fix lcore ID restriction

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

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 14 June 2024 17.27
> 
> On Fri, 14 Jun 2024 10:01:02 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
> > On 6/13/2024 8:13 PM, Stephen Hemminger wrote:
> > > On Thu, 13 Jun 2024 17:51:14 +0100
> > > Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> > >
> > >>> Hi Sivaprasad,
> > >>>
> > >>> Is this '(lcoreid_t)' cast required? Because of integer promotion
> I
> > >>> think result will be correct without casting.
> > >>>
> > >>> (And without integer promotion considered, casting needs to be
> done on
> > >>> one of the variables, not to the result, because result may be
> already
> > >>> cast down I think. Anyway this is not required for this case since
> > >>> variables are u16.)
> > >>>
> > >>
> > >> Why casing required (for record) is,
> > >> 'nb_txq' -> uint16_t, promoted to 'int'
> > >> 'nb_fwd_ports' -> uint16_t, promoted to 'int'
> > >> (nb_txq * nb_fwd_ports) -> result 'int'
> > >> nb_fwd_lcores  -> 'uint32_t'
> > >>
> > >> comparison between 'int' & 'uint32_t' gives warning. After some
> compiler
> > >> version it is smart enough to not give a warning, but casting is
> > >> required for old compilers.
> > >>
> > >> And back to my comment above, casting one of the parameter to
> > >> 'lcoreid_t' also works, as it forcing promotion to 'unsigned int'
> > >> instead. But logically casting looks odd, so keeping casting
> result.
> > >
> > > Where is the integer promotion happening?
> > >
> >
> > Raslan reported following compile error, this version of the patch has
> > the cast, but merged version, v3, doesn't.
> >
> >
> > ```
> > ../../root/dpdk/app/test-pmd/config.c: In function
> 'icmp_echo_config_setup':
> > ../../root/dpdk/app/test-pmd/config.c:5159:30: error: comparison
> between
> > signed and unsigned integer expressions [-Werror=sign-compare]
> >   if ((nb_txq * nb_fwd_ports) < nb_fwd_lcores)
> 
> That does look like a compiler bug. uint16 multiplied by uint16 should
> be uint16.

Not, C doesn't promote types like that. Arithmetic operations always promote smaller types to int or unsigned int. And since uint16 fits in int, uint16 is promoted to int (not unsigned int).

> Not sure why DPDK keeps using small types like uint16 so much, doesn't
> have any
> real benefit since all this is in registers.