bogdanPricope replied on github web page: example/generator/odp_generator.c line 305 @@ -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); - if (odp_packet_has_udp(pkt)) { - if (itf->config.pktin.bit.udp_chksum) { - if (odp_packet_has_l4_error(pkt)) - printf("HW detected L4 error\n"); - } - } + if (pkt_cnt > 0) { + process_pkts(thr, thr_args, pkts, pkt_cnt); - /* Drop packets with errors */ - if (odp_unlikely(odp_packet_has_error(pkt))) { - odp_packet_free(pkt); - continue; - } - pkts[pkt_cnt++] = pkt; + odp_packet_free_multi(pkts, pkt_cnt); + } else if (pkt_cnt == 0) { + continue; + } else { + break; } + } + + return 0; +} - if (pkt_cnt) { - print_pkts(thr, thr_args, pkts, pkt_cnt); +/** + * Scheduler receive function + * + * @param arg thread arguments of type 'thread_args_t *' + */ +static int gen_recv_sched_thread(void *arg) +{ + int thr; + thread_args_t *thr_args; + odp_packet_t pkts[MAX_RX_BURST]; + odp_event_t events[MAX_RX_BURST]; + int pkt_cnt, burst_size, i; + + thr = odp_thread_id(); + thr_args = (thread_args_t *)arg; + burst_size = args->rx_burst_size; + + printf(" [%02i] created mode: RECEIVE SCHEDULER\n", thr); + odp_barrier_wait(&barrier); + + for (;;) { + if (thr_args->stop) + break; + + pkt_cnt = odp_schedule_multi(NULL, ODP_SCHED_NO_WAIT, + events, burst_size); + + if (pkt_cnt > 0) { + for (i = 0; i < pkt_cnt; i++) + pkts[i] = odp_packet_from_event(events[i]);
Comment: In general case is true, but in this particular case is not: we are not waiting other events / configuring other scheduled queues for ODP_EVENT_BUFFER, ODP_EVENT_TIMEOUT, ODP_EVENT_CRYPTO_COMPL or ODP_EVENT_IPSEC_RESULT. I hope ODP is not throwing garbage events in default scheduler group... or? > bogdanPricope wrote > As mentioned before, we need to be able to stop receive side in 'ping' mode > (not waiting infinitely). If spinning affects performance, we can put a 1 s > wait.... (don't know if spinning or extra timer is worse) >> bogdanPricope wrote >> 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_r158214384 updated_at 2017-12-21 07:54:55