Re: [RFC 2/5] ethdev: introduce the affinity field in Tx queue API

[Thomas Monjalon <thomas@monjalon.net>]

21/12/2022 11:29, Jiawei Wang:
> For the multiple hardware ports connect to a single DPDK port (mhpsdp),
> the previous patch introduces the new rte flow item to match the port
> affinity of the received packets.
> 
> This patch adds the tx_affinity setting in Tx queue API, the affinity value
> reflects packets be sent to which hardware port.

I think "affinity" means we would like packet to be sent
on a specific hardware port, but it is not mandatory.
Is it the meaning you want? Or should it be a mandatory port?

> Adds the new tx_affinity field into the padding hole of rte_eth_txconf
> structure, the size of rte_eth_txconf keeps the same. Adds a suppress
> type for structure change in the ABI check file.
> 
> This patch adds the testpmd command line:
> testpmd> port config (port_id) txq (queue_id) affinity (value)
> 
> For example, there're two hardware ports connects to a single DPDK

connects -> connected

> port (port id 0), and affinity 1 stood for hard port 1 and affinity
> 2 stood for hardware port 2, used the below command to config
> tx affinity for each TxQ:
> 	port config 0 txq 0 affinity 1
> 	port config 0 txq 1 affinity 1
> 	port config 0 txq 2 affinity 2
> 	port config 0 txq 3 affinity 2
> 
> These commands config the TxQ index 0 and TxQ index 1 with affinity 1,
> uses TxQ 0 or TxQ 1 send packets, these packets will be sent from the
> hardware port 1, and similar with hardware port 2 if sending packets
> with TxQ 2 or TxQ 3.

[...]
> @@ -212,6 +212,10 @@ API Changes
> +* ethdev: added a new field:
> +
> +  - Tx affinity per-queue ``rte_eth_txconf.tx_affinity``

Adding a new field is not an API change because
existing applications don't need to update their code
if they don't care this new field.
I think you can remove this note.

> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1138,6 +1138,7 @@ struct rte_eth_txconf {
>  				      less free descriptors than this value. */
>  
>  	uint8_t tx_deferred_start; /**< Do not start queue with rte_eth_dev_start(). */
> +	uint8_t tx_affinity; /**< Drives the setting of affinity per-queue. */

Why "Drives"? It is the setting, right?
rte_eth_txconf is per-queue so no need to repeat.
I think a good comment here would be to mention it is a physical port index for mhpsdp.
Another good comment would be to specify how ports are numbered.