Juha-Matti Tilli(jmtilli) replied on github web page:
platform/linux-generic/odp_packet_io.c
line 104
@@ -1626,6 +1683,68 @@ int odp_pktin_recv_mq_tmo(const odp_pktin_queue_t
queues[], unsigned num_q,
odp_time_t t1, t2;
struct timespec ts;
int started = 0;
+ pktio_entry_t *entry[num_q];
+ int index[num_q];
+ fd_set readfds;
+ int maxfd = -1;
+ uint64_t usecs = (wait * SLEEP_NSEC + 999) / 1000;
+ int (*impl)(int num_q, pktio_entry_t *entry[], int index[],
+ odp_packet_t packets[], int num, unsigned *from,
+ uint64_t wait_usecs) = NULL;
+ int impl_set = 0;
+
+ if (wait == ODP_PKTIN_WAIT)
+ usecs = ODP_PKTIN_WAIT;
+
+ for (i = 0; i < num_q; i++) {
+ entry[i] = get_pktio_entry(queues[i].pktio);
+ index[i] = queues[i].index;
+ if (entry[i] == NULL) {
+ ODP_DBG("pktio entry %d does not exist\n",
+ queues[i].pktio);
+ return -1;
+ }
+ if (!impl_set) {
+ impl = entry[i]->s.ops->recv_mq_tmo;
+ impl_set = 1;
+ } else {
+ if (impl != entry[i]->s.ops->recv_mq_tmo) {
Comment:
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_r157253391
updated_at 2017-12-15 17:14:33
