[PATCH] testpmd: add hairpin-map parameter
Etelson, Gregory
getelson at nvidia.com
Wed Oct 30 08:08:46 CET 2024
Hello Stephen,
>> +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.
>
There will be no parse_hairpin_map_entry() in the next patch version.
>
>> + 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.
I switched to sscanf().
>
>> + /* 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.
>
fixed.
>
>> +/* 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);
fixed.
I'll post fixes in the next patch version.
Regards,
Gregory
More information about the dev
mailing list