08/06/2023 21:59, Thomas Monjalon:
> 08/05/2023 15:49, kirankum...@marvell.com:
> > From: Kiran Kumar K <kirankum...@marvell.com>
> >
> > Adding support for Tx queue flow matching item.
> > This item is valid only for egress rules.
> > An example use case would be that application can
> > set different vlan insert rules with different PCP values
> > based on Tx queue number.
> >
> > Signed-off-by: Kiran Kumar K <kirankum...@marvell.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 28 +++++++++++++++++++++
> > doc/guides/prog_guide/rte_flow.rst | 7 ++++++
> > doc/guides/rel_notes/release_23_07.rst | 5 ++++
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++
> > lib/ethdev/rte_flow.c | 1 +
> > lib/ethdev/rte_flow.h | 26 +++++++++++++++++++
> > 6 files changed, 71 insertions(+)
>
> That's only 71 lines but I could make 10 comments.
> I'm fixing spacing, alignment, sorting, etc while pulling next-net,
> but it would have been more confortable if more attention was paid
> by the author and the reviewers.
>
>
> I have a question though (it could be fixed in a later patch):
>
> > +struct rte_flow_item_tx_queue {
> > + /** Tx queue number that packet is being transmitted */
> > + uint16_t tx_queue;
> > +};
>
> This field is used with host endianness, right?
>
> > +static const struct rte_flow_item_tx_queue rte_flow_item_tx_queue_mask = {
> > + .tx_queue = RTE_BE16(0xffff),
> > +};
>
> So why using RTE_BE16 to set a field which is not specifically big endian?
> (not talking about the fact that 0xffff is the same no matter the endianness,
> and not talking about the fact that it is better to use UINT16_MAX).
Repeating again:
This patch has bad quality, bad review,
I've noticed something wrong,
and there are 0 comments or reply!
Now David has sent a fix for this:
https://patches.dpdk.org/project/dpdk/patch/20230629135839.974700-1-david.march...@redhat.com/
Don't expect next patches to be merged quickly.
