Re: [PATCH v6] testpmd: add hairpin map parameter

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 30 Oct 2024 09:37:24 +0200
Gregory Etelson <getelson@nvidia.com> wrote:

> +static int
> +port_config_hairpin_rxq(portid_t pi, uint16_t peer_tx_port,
> +			queueid_t rxq_head, queueid_t txq_head,
> +			uint16_t qcount, uint32_t manual_bind)
> +{
> +	int diag;
> +	queueid_t i, qi;
> +	uint32_t tx_explicit = !!(hairpin_mode & 0x10);
> +	uint32_t force_mem = !!(hairpin_mode & HAIRPIN_MODE_RX_FORCE_MEMORY);
> +	uint32_t locked_mem = !!(hairpin_mode & HAIRPIN_MODE_RX_LOCKED_MEMORY);
> +	uint32_t rte_mem = !!(hairpin_mode & HAIRPIN_MODE_RX_RTE_MEMORY);
> +	struct rte_port *port = &ports[pi];
> +	struct rte_eth_hairpin_conf hairpin_conf = {
> +		.peer_count = 1,
> +	};
> +
> +	for (qi = rxq_head, i = 0; qi < rxq_head + qcount; qi++) {
> +		hairpin_conf.peers[0].port = peer_tx_port;
> +		hairpin_conf.peers[0].queue = i + txq_head;
> +		hairpin_conf.manual_bind = manual_bind;
> +		hairpin_conf.tx_explicit = tx_explicit;
> +		hairpin_conf.force_memory = force_mem;
> +		hairpin_conf.use_locked_device_memory = locked_mem;
> +		hairpin_conf.use_rte_memory = rte_mem;

Did you consider that most of the hairpin_conf values could just be set directly,
avoiding intermediate variables.


> +		diag = rte_eth_rx_hairpin_queue_setup
> +			(pi, qi, nb_rxd, &hairpin_conf);

Unnecessary line break.

Suggestion:

static int
port_config_hairpin_rxq(portid_t pi, uint16_t peer_tx_port,
			queueid_t rxq_head, queueid_t txq_head,
			uint16_t qcount, uint32_t manual_bind)
{
	int diag;
	queueid_t i, qi;
	struct rte_port *port = &ports[pi];
	struct rte_eth_hairpin_conf hairpin_conf = {
		.peer_count = 1,
		.manual_bind = manual_bind,
		.tx_explicit = !!(hairpin_mode & 0x10),
		.force_memory = !!(hairpin_mode & HAIRPIN_MODE_RX_FORCE_MEMORY),
		.use_locked_device_memory = !!(hairpin_mode & HAIRPIN_MODE_RX_LOCKED_MEMORY),
		.use_rte_memory = !!(hairpin_mode & HAIRPIN_MODE_RX_RTE_MEMORY),
	};

	for (qi = rxq_head, i = 0; qi < rxq_head + qcount; qi++) {
		hairpin_conf.peers[0].port = peer_tx_port;
		hairpin_conf.peers[0].queue = i + txq_head;

		diag = rte_eth_rx_hairpin_queue_setup(pi, qi, nb_rxd, &hairpin_conf);
		i++;
		if (diag == 0)
			continue;