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

[Stephen Hemminger]

On Wed, 30 Oct 2024 09:37:24 +0200
Gregory Etelson <getel...@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;