Re: [dpdk-dev] [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter

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

On Sat, Sep 23, 2017 at 05:05:21PM +0530, Rao, Nikhil wrote:
Hi Nikhil,
> On 9/21/2017 9:13 PM, Pavan Nikhilesh Bhagavatula wrote:
> >Hi Nikhil,
> >
> >Few comments Inline
> >
> <snip>
>
> > + *  - 0: Success, statistics reset successfully.
> >
> >Invalid description.
>
> Thanks Pavan, for catching these, will fix.
>
> >
> >>+ *  - <0: Error code on failure, if the adapter doesn't use a rte_service
> >>+ * function, this function returns -ESRCH.
> >>+ */
> >>+int rte_event_eth_rx_adapter_service_id_get(uint8_t id, uint32_t *service_id);
> >>+
> >>+#ifdef __cplusplus
> >>+}
> >>+#endif
> >>+#endif	/* _RTE_EVENT_ETH_RX_ADAPTER_ */
> >>diff --git a/lib/librte_eventdev/rte_event_eth_rx_adapter.c b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> >>new file mode 100644
> >>index 000000000..d5b655dae
> >>--- /dev/null
> >>+++ b/lib/librte_eventdev/rte_event_eth_rx_adapter.c
> >>@@ -0,0 +1,1238 @@
> ><snip>
> >>+
> >>+static int
> >>+rx_adapter_ctrl(uint8_t id, int start)
> >>+{
> >>+	struct rte_event_eth_rx_adapter *rx_adapter;
> >>+	struct rte_eventdev *dev;
> >>+	struct eth_device_info *dev_info;
> >>+	uint32_t i;
> >>+	int use_service = 0;
> >>+	int stop = !start;
> >>+
> >>+	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
> >>+	rx_adapter = id_to_rx_adapter(id);
> >>+	if (!rx_adapter)
> >>+		return -EINVAL;
> >>+
> >>+	dev = &rte_eventdevs[rx_adapter->eventdev_id];
> >>+
> >>+	for (i = 0; i < rte_eth_dev_count(); i++) {
> >>+		dev_info = &rx_adapter->eth_devices[i];
> >>+		/* if start  check for num dev queues */
> >>+		if (start && !dev_info->nb_dev_queues)
> >>+			continue;
> >>+		/* if stop check if dev has been started */
> >>+		if (stop && !dev_info->dev_rx_started)
> >>+			continue;:1
> >>+		use_service |= !dev_info->internal_event_port;
> >>+		dev_info->dev_rx_started = start;
> >>+		if (!dev_info->internal_event_port)
> >>+			continue;
> >>+		start ? (*dev->dev_ops->eth_rx_adapter_start)(dev,
> >>+						&rte_eth_devices[i]) :
> >>+			(*dev->dev_ops->eth_rx_adapter_stop)(dev,
> >>+						&rte_eth_devices[i]);
> >>+	}
> >>+
> >>+	if (use_service)
> >
> >Here setting the service run state is not sufficient we need to enable the
> >service on a service core calling rte_service_start_with_defaults() should be
> >sufficient.
> >
>
> Yes it is necessary but insufficient.
>
> IMO,
> If the application is controlling core masks, the application flow at
> startup looks like:
>
> rte_event_eth_rx_adapter_create(id,..)
> ...
> rte_event_eth_rx_adapter_start(id)
> if (!rte_event_eth_rx_adapter_service_id_get(id, &service_id)) {
> 	rte_service_lcore_add(rx_lcore_id);
>         rte_service_map_lcore_set(service_id, rx_lcore_id, 1);
> 	rte_service_lcore_start(rx_lcore_id)
> }
>
> Since rte_service_start_with_defaults() is invoked before the adapter is
> created, how would it get assigned a core etc ?
>
I might have caused a bit of confusion, I meant to say that If we call
rte_service_start_with_defaults() when "use_service" is set while starting the
RX adapter it will assign the polling function to a service core (default 1:1
mapping).
But this might destroy the user configured service core mappings.
I think adding the above mentioned code snippet in the programmers guide is
self sufficient.

>
> Nikhil

Thanks,
Pavan
> >>+		rte_service_runstate_set(rx_adapter->service_id, start);
> >>+
> >>+	return 0;
> >>+}
> >>+
> ><snip>
> >
> >Regards,
> >Pavan
> >
>