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
> >
>
