Re: [dpdk-dev] [PATCH v2 10/11] examples/l3fwd: add graceful teardown for eventdevice

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

>-----Original Message-----
>From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>Sent: Monday, January 6, 2020 5:43 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob Kollanukkaran <jerinj@marvell.com>; Kovacevic, Marko
><marko.kovacevic@intel.com>; Ori Kam <orika@mellanox.com>;
>Richardson, Bruce <bruce.richardson@intel.com>; Nicolau, Radu
><radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>;
>Kantecki, Tomasz <tomasz.kantecki@intel.com>; Sunil Kumar Kori
><skori@marvell.com>
>Cc: dev@dpdk.org
>Subject: [EXT] RE: [dpdk-dev] [PATCH v2 10/11] examples/l3fwd: add
>graceful teardown for eventdevice
>
>External Email
>
>----------------------------------------------------------------------
>
>> >> Add graceful teardown that addresses both event mode and poll
>> >mode.
>> >>
>> >> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> >> ---
>> >>  examples/l3fwd/main.c | 49
>++++++++++++++++++++++++++++++-
>> >------------
>> >>  1 file changed, 34 insertions(+), 15 deletions(-)
>> >>
>> >> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
>> >> index 0ae64dd41..68998f42c 100644
>> >> --- a/examples/l3fwd/main.c
>> >> +++ b/examples/l3fwd/main.c
>> >> @@ -920,7 +920,7 @@ main(int argc, char **argv)
>> >>  	struct lcore_conf *qconf;
>> >>  	struct rte_eth_dev_info dev_info;
>> >>  	struct rte_eth_txconf *txconf;
>> >> -	int ret;
>> >> +	int i, ret;
>> >>  	unsigned nb_ports;
>> >>  	uint16_t queueid, portid;
>> >>  	unsigned lcore_id;
>> >> @@ -1195,27 +1195,46 @@ main(int argc, char **argv)
>> >>  		}
>> >>  	}
>> >>
>> >> -
>> >>  	check_all_ports_link_status(enabled_port_mask);
>> >>
>> >>  	ret = 0;
>> >>  	/* launch per-lcore init on every lcore */
>> >>  	rte_eal_mp_remote_launch(l3fwd_lkp.main_loop, NULL,
>> >CALL_MASTER);
>> >> -	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
>> >> -		if (rte_eal_wait_lcore(lcore_id) < 0) {
>> >> -			ret = -1;
>> >> -			break;
>> >> +	if (evt_rsrc->enabled) {
>> >> +		for (i = 0; i < evt_rsrc->rx_adptr.nb_rx_adptr; i++)
>> >> +			rte_event_eth_rx_adapter_stop(
>> >> +					evt_rsrc->rx_adptr.rx_adptr[i]);
>> >> +		for (i = 0; i < evt_rsrc->tx_adptr.nb_tx_adptr; i++)
>> >> +			rte_event_eth_tx_adapter_stop(
>> >> +					evt_rsrc->tx_adptr.tx_adptr[i]);
>> >> +
>> >> +		RTE_ETH_FOREACH_DEV(portid) {
>> >> +			if ((enabled_port_mask & (1 << portid)) == 0)
>> >> +				continue;
>> >> +			rte_eth_dev_stop(portid);
>> >>  		}
>> >> -	}
>> >>
>> >> -	/* stop ports */
>> >> -	RTE_ETH_FOREACH_DEV(portid) {
>> >> -		if ((enabled_port_mask & (1 << portid)) == 0)
>> >> -			continue;
>> >> -		printf("Closing port %d...", portid);
>> >> -		rte_eth_dev_stop(portid);
>> >> -		rte_eth_dev_close(portid);
>> >> -		printf(" Done\n");
>> >
>> >Why to stop ports *before* making sure all lcores are stopped?
>> >Shouldn't that peace of code be identical for both poll and event
>mode?
>> >Something like:
>> >rte_eal_mp_wait_lcore();
>> >
>> >                RTE_ETH_FOREACH_DEV(portid) {
>> >                        if ((enabled_port_mask & (1 << portid)) == 0)
>> >                                continue;
>> >                        rte_eth_dev_stop(portid);
>> >                        rte_eth_dev_close(portid);
>> >                }
>> >?
>> >
>>
>> Event dev spec requires stopping producers before consumers else
>we might run into
>> deadlock in some cases.
>
>Ok... but for TX path wouldn't core be a producer?

Not in all cases, true in case of SW event device and implementation defined in HW event devices.

Also both the cases of Tx poll on force_quit in case Tx path fails so that cores can exit.

                if (flags & L3FWD_EVENT_TX_ENQ) {
                        ev.queue_id = tx_q_id;
                        ev.op = RTE_EVENT_OP_FORWARD;
                        while (rte_event_enqueue_burst(event_d_id, event_p_id,
                                                &ev, 1) && !force_quit)
                                ;
                }

                if (flags & L3FWD_EVENT_TX_DIRECT) {
                        rte_event_eth_tx_adapter_txq_set(mbuf, 0);
                        while (!rte_event_eth_tx_adapter_enqueue(event_d_id,
                                                event_p_id, &ev, 1, 0) &&
                                        !force_quit)
                                ;
                }

>Also for that wouldn't rte_event_eth_(rx|tx)_adapter_stop(0 be
>enough?

In case of HW event devices the above might be nop as control lies with driver/net.
And since anyway we need to stop ethernet device we might as well do it here.

>I am not familiar with event-dev spec at all, so forgive for possibly dumb
>questions 😉

😊

>
>>
>> >> +		rte_eal_mp_wait_lcore();
>> >> +		RTE_ETH_FOREACH_DEV(portid) {
>> >> +			if ((enabled_port_mask & (1 << portid)) == 0)
>> >> +				continue;
>> >> +			rte_eth_dev_close(portid);
>> >> +		}
>> >> +
>> >> +		rte_event_dev_stop(evt_rsrc->event_d_id);
>> >> +		rte_event_dev_close(evt_rsrc->event_d_id);
>> >> +
>> >> +	} else {
>> >> +		rte_eal_mp_wait_lcore();
>> >> +
>> >> +		RTE_ETH_FOREACH_DEV(portid) {
>> >> +			if ((enabled_port_mask & (1 << portid)) == 0)
>> >> +				continue;
>> >> +			printf("Closing port %d...", portid);
>> >> +			rte_eth_dev_stop(portid);
>> >> +			rte_eth_dev_close(portid);
>> >> +			printf(" Done\n");
>> >> +		}
>> >>  	}
>> >>  	printf("Bye...\n");
>> >>
>> >> --
>> >> 2.17.1