Re: [PATCH v1 5/5] examples/l3fwd: enable direct rearm mode

[Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>]

20/04/2022 09:16, Feifei Wang пишет:
> Enable direct rearm mode. The mapping is decided in the data plane based
> on the first packet received.
> 
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>   examples/l3fwd/l3fwd_lpm.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> index bec22c44cd..38ffdf4636 100644
> --- a/examples/l3fwd/l3fwd_lpm.c
> +++ b/examples/l3fwd/l3fwd_lpm.c
> @@ -147,7 +147,7 @@ lpm_main_loop(__rte_unused void *dummy)
>   	unsigned lcore_id;
>   	uint64_t prev_tsc, diff_tsc, cur_tsc;
>   	int i, nb_rx;
> -	uint16_t portid;
> +	uint16_t portid, tx_portid;
>   	uint8_t queueid;
>   	struct lcore_conf *qconf;
>   	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
> @@ -158,6 +158,8 @@ lpm_main_loop(__rte_unused void *dummy)
>   
>   	const uint16_t n_rx_q = qconf->n_rx_queue;
>   	const uint16_t n_tx_p = qconf->n_tx_port;
> +	int direct_rearm_map[n_rx_q];
> +
>   	if (n_rx_q == 0) {
>   		RTE_LOG(INFO, L3FWD, "lcore %u has nothing to do\n", lcore_id);
>   		return 0;
> @@ -169,6 +171,7 @@ lpm_main_loop(__rte_unused void *dummy)
>   
>   		portid = qconf->rx_queue_list[i].port_id;
>   		queueid = qconf->rx_queue_list[i].queue_id;
> +		direct_rearm_map[i] = 0;
>   		RTE_LOG(INFO, L3FWD,
>   			" -- lcoreid=%u portid=%u rxqueueid=%hhu\n",
>   			lcore_id, portid, queueid);
> @@ -209,6 +212,17 @@ lpm_main_loop(__rte_unused void *dummy)
>   			if (nb_rx == 0)
>   				continue;
>   
> +			/* Determine the direct rearm mapping based on the first
> +			 * packet received on the rx queue
> +			 */
> +			if (direct_rearm_map[i] == 0) {
> +				tx_portid = lpm_get_dst_port(qconf, pkts_burst[0],
> +							portid);
> +				rte_eth_direct_rxrearm_map(portid, queueid,
> +								tx_portid, queueid);
> +				direct_rearm_map[i] = 1;
> +			}
> +

That just doesn't look right to me: why to make decision based on the 
first packet?
What would happen if second and all other packets have to be routed
to different ports?
In fact, this direct-rearm mode seems suitable only for hard-coded
one to one mapped forwarding (examples/l2fwd, testpmd).
For l3fwd it can be used safely only when we have one port in use.
Also I think it should be selected at init-time and
it shouldn't be on by default.
To summarize, my opinion:
special cmd-line parameter to enable it.
allowable only when we run l3fwd over one port.


>   #if defined RTE_ARCH_X86 || defined __ARM_NEON \
>   			 || defined RTE_ARCH_PPC_64
>   			l3fwd_lpm_send_packets(nb_rx, pkts_burst,