Re: [PATCH v5 7/7] app/testpmd: add Host Shaper command

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

Since ethdev patch is factored out from the patch series the rest could
go to mlx5 maintainers.

On 6/7/22 15:59, Spike Du wrote:
> Add command line options to support host shaper configure.
> - Command syntax:
>    mlx5 set port <port_id> host_shaper avail_thresh_triggered <0|1> rate
> <rate_num>
> 
> - Example commands:
> To enable avail_thresh_triggered on port 1 and disable current host
> shaper:
> testpmd> mlx5 set port 1 host_shaper avail_thresh_triggered 1 rate 0
> 
> To disable avail_thresh_triggered and current host shaper on port 1:
> testpmd> mlx5 set port 1 host_shaper avail_thresh_triggered 0 rate 0
> 
> The rate unit is 100Mbps.
> To disable avail_thresh_triggered and configure a shaper of 5Gbps on
> port 1:
> testpmd> mlx5 set port 1 host_shaper avail_thresh_triggered 0 rate 50
> 
> Add sample code to handle rxq available descriptor threshold event, it
> delays a while so that rxq empties, then disables host shaper and
> rearms available descriptor threshold event.
> 
> Signed-off-by: Spike Du <spiked@nvidia.com>

[snip]

> +
> +static uint8_t host_shaper_avail_thresh_triggered[RTE_MAX_ETHPORTS];
> +#define SHAPER_DISABLE_DELAY_US 100000 /* 100ms */
> +
> +/**
> + * Disable the host shaper and re-arm available descriptor threshold event.
> + *
> + * @param[in] args
> + *   uint32_t integer combining port_id and rxq_id.
> + */
> +static void
> +mlx5_test_host_shaper_disable(void *args)
> +{
> +	uint32_t port_rxq_id = (uint32_t)(uintptr_t)args;
> +	uint16_t port_id = port_rxq_id & 0xffff;
> +	uint16_t qid = (port_rxq_id >> 16) & 0xffff;
> +	struct rte_eth_rxq_info qinfo;
> +
> +	printf("%s disable shaper\n", __func__);
> +	if (rte_eth_rx_queue_info_get(port_id, qid, &qinfo)) {
> +		printf("rx_queue_info_get returns error\n");
> +		return;
> +	}
> +	/* Rearm the available descriptor threshold event. */
> +	if (rte_eth_rx_avail_thresh_set(port_id, qid, qinfo.avail_thresh)) {
> +		printf("config avail_thresh returns error\n");
> +		return;
> +	}
> +	/* Only disable the shaper when avail_thresh_triggered is set. */
> +	if (host_shaper_avail_thresh_triggered[port_id] &&
> +	    rte_pmd_mlx5_host_shaper_config(port_id, 0, 0))
> +		printf("%s disable shaper returns error\n", __func__);
> +}
> +
> +void
> +mlx5_test_avail_thresh_event_handler(uint16_t port_id, uint16_t rxq_id)
> +{
> +	uint32_t port_rxq_id = port_id | (rxq_id << 16);

Nobody guarantees here that port_id refers to an mlx5 port.
It could be any port with avail_thres support.

> +
> +	rte_eal_alarm_set(SHAPER_DISABLE_DELAY_US,
> +			  mlx5_test_host_shaper_disable,
> +			  (void *)(uintptr_t)port_rxq_id);
> +	printf("%s port_id:%u rxq_id:%u\n", __func__, port_id, rxq_id);
> +}

[snip]

> +cmdline_parse_token_string_t cmd_port_host_shaper_mlx5 =
> +	TOKEN_STRING_INITIALIZER(struct cmd_port_host_shaper_result,
> +				mlx5, "mlx5");

I think it lucks 'static' keyword (many cases below).

[snip]