muvarov replied on github web page:
platform/linux-generic/odp_packet_io.c
line 22
@@ -1577,10 +1577,27 @@ int odp_pktin_recv_tmo(odp_pktin_queue_t queue,
odp_packet_t packets[], int num,
odp_time_t t1, t2;
struct timespec ts;
int started = 0;
+ pktio_entry_t *entry;
ts.tv_sec = 0;
ts.tv_nsec = SLEEP_NSEC;
+ entry = get_pktio_entry(queue.pktio);
+ if (entry == NULL) {
+ ODP_DBG("pktio entry %d does not exist\n", queue.pktio);
+ return -1;
+ }
+
+ if (entry->s.ops->recv_tmo) {
+ uint64_t usecs = (wait * SLEEP_NSEC + 999) / 1000;
+
+ if (wait == ODP_PKTIN_WAIT)
+ usecs = ODP_PKTIN_WAIT;
+
+ return entry->s.ops->recv_tmo(entry, queue.index, packets, num,
+ usecs);
Comment:
just pass nsec to drivers or you will have double calculation like in netmap
code.
> Juha-Matti Tilli(jmtilli) wrote:
> Are you entirely sure about this? This is a function pointer, and if two
> pktio devices have the same function pointer for recv_mq_tmo(), the other
> function pointers including the recv() function pointer should be identical
> as well. We do pass the device (entry[i]) to the function pointed to by
> recv_mq_tmo as well, so it should be able to update the correct counters. The
> ops structure is const, after all...
>
> So, when receiving packets from a particular device using netmap, we do call
> netmap_recv() and pass to it the pktio_entry[i] and also index[i] for the
> queue index. This is the same that the odp_pktin_recv_mq_tmo() function
> already did. It calls odp_pktin_recv() in a loop, and the latter function
> calls entry->s.ops->recv(entry, queue.index, packets, num); which to me seems
> to be exactly the same thing -- you pass exactly the same entry and the same
> queue.index. Same function, same arguments, I can't see how the behavior
> would be different...
>
> Now, if somebody opposes the .recv_mq_tmo function pointer due to added code
> line count, I might perhaps agree. But it seems to be the only way the DPDK
> support for genuine sleep could be added, so I see some value in it.
>> Juha-Matti Tilli(jmtilli) wrote:
>> Several options: (1) I could implement a binary search algorithm that
>> incrementally checks various usec values and uses odp_pktin_wait_time() to
>> check which one of these trial attempts result in the same sleep wait value,
>> but that would be slow; (2) I could implement odp_pktin_wait_time_reverse()
>> that does the conversion in the other direction, but that would be an API
>> change; (3) I could just do the conversion and pass usec values instead of
>> sleep wait values. I carefully considered each and selected (3), but do you
>> think (2) or (1) would be better?
>>> Juha-Matti Tilli(jmtilli) wrote:
>>> Will fix.
>>>> Juha-Matti Tilli(jmtilli) wrote:
>>>> I disagree. SLEEP_NSEC is a private #define in odp_packet_io.c so what the
>>>> wait parameter actually means cannot be interpreted in the drivers.
>>>> (Believe me, I tried it!) Thus, the conversion from the sleep wait
>>>> parameter to microseconds is needed.
>>>>> Matias Elo(matiaselo) wrote:
>>>>> As I noted in the previous comment, I think this check isn't enough. The
>>>>> input queues would have to be from the same pktio device for this to work.
>>>>>> Matias Elo(matiaselo) wrote:
>>>>>> Internal functions should not use odp_ prefix.
>>>>>>> Matias Elo(matiaselo) wrote:
>>>>>>> You should pass the `wait` parameter to `entry->s.ops->recv_tmo()` and
>>>>>>> not do any conversions here.
>>>>>>>> Matias Elo(matiaselo) wrote:
>>>>>>>> ODP_TIME_SEC_IN_NS would work here.
https://github.com/Linaro/odp/pull/341#discussion_r157292206
updated_at 2017-12-15 20:27:24
