bogdanPricope replied on github web page: example/generator/odp_generator.c line 246 @@ -838,39 +892,55 @@ static int gen_recv_thread(void *arg) if (thr_args->stop) break; - /* Use schedule to get buf from any input queue */ - ev_cnt = odp_schedule_multi(NULL, ODP_SCHED_NO_WAIT, - events, burst_size); - if (ev_cnt == 0) - continue; - for (i = 0, pkt_cnt = 0; i < ev_cnt; i++) { - pkt = odp_packet_from_event(events[i]); - itf = &itfs[odp_pktio_index(odp_packet_input(pkt))]; - - if (odp_packet_has_ipv4(pkt)) { - if (itf->config.pktin.bit.ipv4_chksum) { - if (odp_packet_has_l3_error(pkt)) - printf("HW detected L3 error\n"); - } - } + pkt_cnt = odp_pktin_recv_tmo(pktin, pkts, burst_size, + ODP_PKTIN_NO_WAIT);
Comment: yep. My impression was that odp_pktin_recv() is blocking but it seems is no: in 'ping' mode we need to be able to stop the receive thread after a number of pings. > bogdanPricope wrote > This csum check is done with newer API in API-NEXT > (odp_packet_l3_chksum_status()). No sense to optimize this part for this > older implementation >> bogdanPricope wrote >> 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_r158211919 updated_at 2017-12-21 07:35:45