muvarov 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:
That is good question. In general there should not be any garbage. But odp app
should dispatch event first. I think we can add timeout event to break loop
here instead of polling some global variable.
> muvarov wrote
> it's not blocking, it just polling function.
>> muvarov wrote
>> how about ODP_SCHED_WAIT and timeout event to break the loop and exit
>> application?
>>> bogdanPricope wrote
>>> 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_r158241210
updated_at 2017-12-21 10:19:03
