On Thu, 28 Sep 2023 16:39:06 +0300
Gregory Etelson <getel...@nvidia.com> wrote:
> +static __rte_always_inline
> +char *parse_hairpin_map_entry(char *input, char **next)
> +{
> + char *tail = strchr(input, ':');
> +
> + if (!tail)
> + return NULL;
> + tail[0] = '\0';
> + *next = tail + 1;
> + return input;
> +}
> +
There is no reason to mark this as inline. It is not in fast path.
Let compiler decide.
> + head = parse_hairpin_map_entry(next, &next);
> + if (!head)
> + goto err;
> + map->rx_port = atoi(head);
Use strtoul() to allow checking for invalid number.
Seems like the parsing here is following the parsing code pattern
that could use strtok_r.
> + /* Fail to setup rx queue, return */
> + if (port->port_status == RTE_PORT_HANDLING)
> + port->port_status = RTE_PORT_STOPPED;
> + else
> + fprintf(stderr,
> + "Port %d can not be set back to stopped\n", pi);
> + fprintf(stderr,
> + "Port %d failed to configure hairpin on rxq %u.\n"
> + "Peer port: %u peer txq: %u\n",
> + pi, qi, peer_tx_port, i);
Minor nit, port should be printed with %u since it is unsigned.
Lots of testpmd code gets this wrong, and compiler doesn't care.
> +/* Configure the Rx and Tx hairpin queues for the selected port. */
> +static int
> +setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi)
> +{
> + return !hairpin_multiport_mode ?
> + setup_legacy_hairpin_queus(pi, p_pi, cnt_pi) :
> + setup_mapped_harpin_queues(pi);
> +}
It is clearer to use an if statement in this case.
Also, better to write positive rather than negative logic.
if (hairpin_multiport_mode)
return setup_mapped_hairpin_queues(pi);
else
return setup_legacy_hairpin_queus(pi, p_pi, cnt_pi);
