bogdanPricope replied on github web page:

example/generator/odp_generator.c
line 87
@@ -563,44 +572,68 @@ static int create_pktio(const char *dev, odp_pool_t pool,
                return -1;
        }
 
-       if (num_rx_queues > capa.max_input_queues)
-               num_rx_queues = capa.max_input_queues;
+       if (num_rx_queues) {
+               pktin_mode = ODP_PKTIO_OP_MT_UNSAFE;
+               if (num_rx_queues > capa.max_input_queues) {
+                       num_rx_queues = capa.max_input_queues;
+                       pktin_mode = ODP_PKTIO_OP_MT;
+                       EXAMPLE_DBG("Warning: Force RX multithread safe mode "
+                                   "(slower)on %s\n",  dev);
+               }
 
-       odp_pktin_queue_param_init(&pktin_param);
-       pktin_param.num_queues = num_rx_queues;
-       pktin_param.queue_param.sched.sync = ODP_SCHED_SYNC_ATOMIC;
+               odp_pktin_queue_param_init(&pktin_param);
+               pktin_param.num_queues = num_rx_queues;
+               pktin_param.op_mode = pktin_mode;
+               if (sched)
+                       pktin_param.queue_param.sched.sync =
+                               ODP_SCHED_SYNC_ATOMIC;


Comment:
Yes, this can be part of another PR.

> bogdanPricope wrote
>  * @return Number of events outputted (0 ... num)


>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>> The new `odp_event_filter_packet()` API would be useful here.


>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> Why `ODP_SCHED_NO_WAIT` vs. `ODP_SCHED_WAIT` here? You're just spinning if 
>>> no packets are available so why not let the scheduler do the waiting?


>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> Agree with @muvarov, this could use some comments to explain why these 
>>>> calls are being used. You'd expect a dedicated RX thread to simply wait 
>>>> for packet input.


>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> Checksum errors will result in `odp_packet_has_error()` being set as 
>>>>> well, so these checks can be done only if the summary packet error 
>>>>> predicate is set, avoiding unnecessary checks for known good packets.


>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> Might be good to have options for controlling the queue sync type here 
>>>>>> as `ODP_SCHED_SYNC_PARALLEL` should result in highest throughput, and 
>>>>>> `ODP_SCHED_SYNC_ORDERED` would be useful in testing performance of 
>>>>>> scheduler implementations (in theory should be better than 
>>>>>> `ODP_SCHED_SYNC_ATOMIC`).
>>>>>> 
>>>>>> Something to explore in another PR


>>>>>>> muvarov wrote
>>>>>>> ok


>>>>>>>> muvarov wrote
>>>>>>>> and why odp_pktin_recv_tmo() and not odp_pktin_recv() ?


>>>>>>>>> muvarov wrote
>>>>>>>>> why not ODP_PKTIN_WAIT?


>>>>>>>>>> muvarov wrote
>>>>>>>>>> not all events are packets.


>>>>>>>>>>> muvarov wrote
>>>>>>>>>>> ```
>>>>>>>>>>>  * @return Next highest priority event
>>>>>>>>>>>  * @retval ODP_EVENT_INVALID on timeout and no events available
>>>>>>>>>>> ```


>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>> just separate rx function for scheduler and on thread start you 
>>>>>>>>>>>> just select scheduler or direct.


>>>>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>>>>> This will complicate this already over-complicated code: we may 
>>>>>>>>>>>>> need to decide between ultimate performance and feature richness. 


>>>>>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>>>>>> No -  we need to print csum errors first.
>>>>>>>>>>>>>> This part was significantly changed in api-next (csum checks use 
>>>>>>>>>>>>>> different/ new API) and it makes no sense to optimize it for the 
>>>>>>>>>>>>>> old (master) code. After integration in api-next, this part will 
>>>>>>>>>>>>>> be  reworked  to use less parser flags (reduce parsing level).
>>>>>>>>>>>>>> For example, removing L4 parsing and locating interface is 
>>>>>>>>>>>>>> bringing an extra 1 mpps.


>>>>>>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>>>>>>> '-r' may work.


>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>> Having an option to use direct mode seems reasonable, but 
>>>>>>>>>>>>>>>> shouldn't we retain schedule mode (perhaps as a command line 
>>>>>>>>>>>>>>>> switch)? This would provide an easy means of testing scheduler 
>>>>>>>>>>>>>>>> efficiency as it is tuned. At least in some environments we'd 
>>>>>>>>>>>>>>>> like schedule mode to show better performance than direct.


>>>>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>>>>> that has to be the first check.


>>>>>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>>>>>> -r ?


https://github.com/Linaro/odp/pull/343#discussion_r158210664
updated_at 2017-12-21 07:55:32

Reply via email to