On 10/30/2019 11:53 PM, Ori Kam wrote:
> This commit introduce the hairpin queues to the testpmd.
> the hairpin queue is configured using --hairpinq=<n>
> the hairpin queue adds n queue objects for both the total number
> of TX queues and RX queues.
> The connection between the queues are 1 to 1, first Rx hairpin queue
> will be connected to the first Tx hairpin queue
>
> Signed-off-by: Ori Kam <or...@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> ---
> app/test-pmd/parameters.c | 28 ++++++++++++
> app/test-pmd/testpmd.c | 109
> +++++++++++++++++++++++++++++++++++++++++++++-
> app/test-pmd/testpmd.h | 3 ++
New parameter should be documented in 'doc/guides/testpmd_app_ug/run_app.rst',
can you please describe befiefly how it works, what the mapping will like by
default etc..
<...>
> @@ -2028,6 +2076,11 @@ struct extmem_param {
> queueid_t qi;
> struct rte_port *port;
> struct rte_ether_addr mac_addr;
> + struct rte_eth_hairpin_conf hairpin_conf = {
> + .peer_count = 1,
> + };
> + int i;
> + struct rte_eth_hairpin_cap cap;
>
> if (port_id_is_invalid(pid, ENABLED_WARN))
> return 0;
> @@ -2060,9 +2113,16 @@ struct extmem_param {
> configure_rxtx_dump_callbacks(0);
> printf("Configuring Port %d (socket %u)\n", pi,
> port->socket_id);
> + if (nb_hairpinq > 0 &&
> + rte_eth_dev_hairpin_capability_get(pi, &cap)) {
> + printf("Port %d doesn't support hairpin "
> + "queues\n", pi);
> + return -1;
> + }
> /* configure port */
> - diag = rte_eth_dev_configure(pi, nb_rxq, nb_txq,
> - &(port->dev_conf));
> + diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
> + nb_txq + nb_hairpinq,
> + &(port->dev_conf));
> if (diag != 0) {
> if (rte_atomic16_cmpset(&(port->port_status),
> RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
> @@ -2155,6 +2215,51 @@ struct extmem_param {
> port->need_reconfig_queues = 1;
> return -1;
> }
> + /* setup hairpin queues */
> + i = 0;
> + for (qi = nb_txq; qi < nb_hairpinq + nb_txq; qi++) {
> + hairpin_conf.peers[0].port = pi;
> + hairpin_conf.peers[0].queue = i + nb_rxq;
> + diag = rte_eth_tx_hairpin_queue_setup
> + (pi, qi, nb_txd, &hairpin_conf);
> + i++;
> + if (diag == 0)
> + continue;
> +
> + /* Fail to setup rx queue, return */
> + if (rte_atomic16_cmpset(&(port->port_status),
> + RTE_PORT_HANDLING,
> + RTE_PORT_STOPPED) == 0)
> + printf("Port %d can not be set back "
> + "to stopped\n", pi);
> + printf("Fail to configure port %d hairpin "
> + "queues\n", pi);
> + /* try to reconfigure queues next time */
> + port->need_reconfig_queues = 1;
> + return -1;
> + }
> + i = 0;
> + for (qi = nb_rxq; qi < nb_hairpinq + nb_rxq; qi++) {
> + hairpin_conf.peers[0].port = pi;
> + hairpin_conf.peers[0].queue = i + nb_txq;
> + diag = rte_eth_rx_hairpin_queue_setup
> + (pi, qi, nb_rxd, &hairpin_conf);
> + i++;
> + if (diag == 0)
> + continue;
> +
> + /* Fail to setup rx queue, return */
> + if (rte_atomic16_cmpset(&(port->port_status),
> + RTE_PORT_HANDLING,
> + RTE_PORT_STOPPED) == 0)
> + printf("Port %d can not be set back "
> + "to stopped\n", pi);
> + printf("Fail to configure port %d hairpin "
> + "queues\n", pi);
> + /* try to reconfigure queues next time */
> + port->need_reconfig_queues = 1;
> + return -1;
> + }
'start_port()' function is already huge, what do you think moving hairpin
related setup into a specific function and call it when "nb_hairpinq > 0" only?
This makes the hairpin related config more clear and 'start_port()' function
simpler.
I think all hairpin related operations can be extracted, like capability check,
'rte_eth_dev_configure' & hairpin queue setup.
