Re: [PATCH v6 1/2] ethdev: add Tx queue mapping of aggregated ports

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 2/17/23 13:50, Jiawei Wang wrote:
> When multiple ports are aggregated into a single DPDK port,
> (example: Linux bonding, DPDK bonding, failsafe, etc.),
> we want to know which port use for Tx via a queue.
> 
> This patch introduces the new ethdev API
> rte_eth_dev_map_aggr_tx_affinity(), it's used to map a Tx queue
> with an aggregated port of the DPDK port (specified with port_id),
> The affinity is the number of the aggregated port.
> Value 0 means no affinity and traffic could be routed to any
> aggregated port, this is the default current behavior.
> 
> The maximum number of affinity is given by rte_eth_dev_count_aggr_ports().
> 
> Add the trace point for ethdev rte_eth_dev_count_aggr_ports()
> and rte_eth_dev_map_aggr_tx_affinity() functions.
> 
> Add the testpmd command line:
> testpmd> port config (port_id) txq (queue_id) affinity (value)
> 
> For example, there're two physical ports connected to
> a single DPDK port (port id 0), and affinity 1 stood for
> the first physical port and affinity 2 stood for the second
> physical port.
> Use the below commands to config tx phy affinity for per Tx Queue:
>          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 Tx Queue index 0 and Tx Queue index 1 with
> phy affinity 1, uses Tx Queue 0 or Tx Queue 1 send packets,
> these packets will be sent from the first physical port, and similar
> with the second physical port if sending packets with Tx Queue 2
> or Tx Queue 3.
> 
> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

[snip]

> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 6a550cfc83..b7fdc454a8 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1171,6 +1171,40 @@ typedef int (*eth_tx_descriptor_dump_t)(const struct rte_eth_dev *dev,
>   					uint16_t queue_id, uint16_t offset,
>   					uint16_t num, FILE *file);
>   
> +/**
> + * @internal
> + * Get the number of aggregated ports.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + *
> + * @return
> + *   Negative errno value on error, 0 or positive on success.
> + *
> + * @retval >=0
> + *   The number of aggregated port if success.
> + * @retval -ENOTSUP
> + *   Get aggregated ports API is not supported.
> + */
> +typedef int (*eth_count_aggr_ports_t)(uint16_t port_id);

Why does use port_id as the first parameter whereas all other
driver callbacks use 'struct rte_eth_dev *'?

> +
> +/**
> + * @internal
> + * Map a Tx queue with an aggregated port of the DPDK port.
> + *
> + * @param port_id
> + *   The identifier of the port used in rte_eth_tx_burst().
> + * @param tx_queue_id
> + *   The index of the transmit queue used in rte_eth_tx_burst().
> + * @param affinity
> + *   The number of the aggregated port.
> + *
> + * @return
> + *   Negative on error, 0 on success.
> + */
> +typedef int (*eth_map_aggr_tx_affinity_t)(uint16_t port_id, uint16_t tx_queue_id,
> +					  uint8_t affinity);

same herre

> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 055c46082b..1a63f9fb7a 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -6946,6 +6946,78 @@ rte_eth_buffer_split_get_supported_hdr_ptypes(uint16_t port_id, uint32_t *ptypes
>   	return j;
>   }
>   
> +int rte_eth_dev_count_aggr_ports(uint16_t port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (*dev->dev_ops->count_aggr_ports == NULL)

Is it OK that tracing is long in this case?

> +		return 0;
> +	ret = eth_err(port_id, (*dev->dev_ops->count_aggr_ports)(port_id));
> +
> +	rte_eth_trace_count_aggr_ports(port_id, ret);
> +
> +	return ret;
> +}
> +

[snip]