Re: [PATCH] ethdev: fix Tx queue mask endianness

[David Marchand <david.marchand@redhat.com>]

On Fri, Jun 30, 2023 at 9:00 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Jun 29, 2023 at 6:14 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >
> > On 6/29/2023 4:42 PM, Thomas Monjalon wrote:
> > > 29/06/2023 17:40, David Marchand:
> > >> On Thu, Jun 29, 2023 at 5:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >>> 29/06/2023 15:58, David Marchand:
> > >>>> -     .tx_queue = RTE_BE16(0xffff),
> > >>>> +     .tx_queue = 0xffff,
> > >>>
> > >>> As I said in an earlier comment about the same issue,
> > >>> UINT16_MAX would be better.
> > >>
> > >> I don't mind updating (or maybe Ferruh can squash this directly ?) but
> > >> there are lots of uint16_t fields initialised with 0xffff in this same
> > >> file.
> > >
> > > It can be made in a separate patch for all occurences.
> > > First I would like to get some comments, what do you prefer
> > > between 0xffff and UINT16_MAX?
> > >
> >
> > Both works, no strong opinion, I am OK with 0xffff,
> >
> > The variable we are setting is '*_mask', and main point of the value
> > used is to have all bits set, and 0xff.. usage highlights it.
> >
> > Not for UINT16_MAX, but for wider variables, it is easier to make
> > mistake and put wrong number of 'f', using 'UINTxx_MAX' macro can
> > prevent this mistake, this is a benefit.
> >
> >
> > And I think consistency matters more, so if you prefer 'UINTxx_MAX',
> > lets stick to it.
> >
> > I can update above in next-net, but as far as I understand we can have a
> > patch to fix all occurrences.
>
> Given that we are considering unsigned integers, is there something
> wrong with using (typeof(var)) -1 ?

Or maybe get inspiration from what the Linux kernel does :-)
Like GENMASK().


-- 
David Marchand