Re: [dpdk-dev] [PATCH v6 08/10] examples/l2fwd-event: add eventdev main loop

["Varghese, Vipin" <vipin.varghese@intel.com>]

HI Pavan,

snipped
> Add event dev main loop based on enabled l2fwd options and eventdev
> capabilities.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> +	if (rsrc->event_mode) {
> +		port_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> +		port_conf.rx_adv_conf.rss_conf.rss_key = NULL;
> +		port_conf.rx_adv_conf.rss_conf.rss_hf = ETH_RSS_IP;
> +	}
Question, is RSS hash configured for generating flow id for Eventdev? As my understanding. RSS for single RX port-queue pair does not require the same. 
snipped
> +		if (is_master && timer_period > 0) {
> +			cur_tsc = rte_rdtsc();
> +			diff_tsc = cur_tsc - prev_tsc;
> +
> +			/* advance the timer */
> +			timer_tsc += diff_tsc;
> +
> +			/* if timer has reached its timeout */
> +			if (unlikely(timer_tsc >= timer_period)) {
> +				print_stats(rsrc);
> +				/* reset the timer */
> +				timer_tsc = 0;
> +			}
> +			prev_tsc = cur_tsc;
> +		}
Is it possible to move the print_stats to service core, as 'CALL_MASTER' is enabled in remote_launch making this a potential worker?

> +
> +		/* Read packet from eventdev */
> +		if (!rte_event_dequeue_burst(event_d_id, port_id, &ev, 1, 0))
> +			continue;
Is not this unlikely `nb_burst == 0`

> +
> +		l2fwd_event_fwd(rsrc, &ev, tx_q_id, timer_period, flags);
> +
> +		if (flags & L2FWD_EVENT_TX_ENQ) {
> +			while (rte_event_enqueue_burst(event_d_id, port_id,
> +						       &ev, 1) &&
> +					!rsrc->force_quit)
> +				;
Can we place a `continue` as we are not expecting ` L2FWD_EVENT_TX_DIRECT`?
> +		}

> +
> +		if (flags & L2FWD_EVENT_TX_DIRECT) {
> +			while
> (!rte_event_eth_tx_adapter_enqueue(event_d_id,
> +								port_id,
> +								&ev, 1, 0) &&
> +					!rsrc->force_quit)
> +				;
> +		}
> +	}
snipped
> +
> +	while (!rsrc->force_quit) {
> +		/* if timer is enabled */
> +		if (is_master && timer_period > 0) {
> +			cur_tsc = rte_rdtsc();
> +			diff_tsc = cur_tsc - prev_tsc;
> +
> +			/* advance the timer */
> +			timer_tsc += diff_tsc;
> +
> +			/* if timer has reached its timeout */
> +			if (unlikely(timer_tsc >= timer_period)) {
> +				print_stats(rsrc);
> +				/* reset the timer */
> +				timer_tsc = 0;
> +			}
> +			prev_tsc = cur_tsc;
> +		}
Can we move `print_stats` logic to service core?

> +
> +		/* Read packet from eventdev */
> +		nb_rx = rte_event_dequeue_burst(event_d_id, port_id, ev,
> +						deq_len, 0);
> +		if (nb_rx == 0)
Can we use `unlikely`?
> +			continue;
> +
> +		for (i = 0; i < nb_rx; i++) {
> +			l2fwd_event_fwd(rsrc, &ev[i], tx_q_id, timer_period,
> +					flags);
> +		}
> +
> +		if (flags & L2FWD_EVENT_TX_ENQ) {
> +			nb_tx = rte_event_enqueue_burst(event_d_id, port_id,
> +							ev, nb_rx);
> +			while (nb_tx < nb_rx && !rsrc->force_quit)
> +				nb_tx +=
> rte_event_enqueue_burst(event_d_id,
> +						port_id, ev + nb_tx,
> +						nb_rx - nb_tx);
Can we use `continue` as we do not transmit from the same worker int his case?
> +		}
> +
snipped