Re: [PATCH v2] ethdev: add Tx queue flow matching item

[Thomas Monjalon <thomas@monjalon.net>]

08/05/2023 15:49, kirankumark@marvell.com:
> From: Kiran Kumar K <kirankumark@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 <kirankumark@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).