Matias Elo(matiaselo) replied on github web page:

platform/linux-generic/pktio/dpdk.c
line 70
@@ -1372,10 +1382,10 @@ static int dpdk_recv(pktio_entry_t *pktio_entry, int 
index,
        if (!pkt_dpdk->lockless_rx)
                odp_ticketlock_lock(&pkt_dpdk->rx_lock[index]);
        /**
-        * ixgbe_pmd has a minimum supported RX burst size ('min_rx_burst'). If
-        * 'num' < 'min_rx_burst', 'min_rx_burst' is used as rte_eth_rx_burst()
-        * argument and the possibly received extra packets are cached for the
-        * next dpdk_recv_queue() call to use.
+        * ixgbe and i40e drivers have a minimum supported RX burst size


Comment:
@nagarahalli I don't see a new capability parameter being very useful as these 
two dpdk drivers are likely the only devices which would use it. At least we 
haven't come across any other network devices which have this kind of 
limitation.

I ran a quick test and the two additional if statements caused by rx caching 
have a minuscule  performance impact.


> nagarahalli wrote
> @matiaselo Agree on the simple applications. But for real applications or for 
> performance benchmarking, this adds additional cycles. If this is made as 
> part of capability, application can decide the burst size upfront.


>> Matias Elo(matiaselo) wrote:
>> @lumag That would be the best solution but I don't see it happening anytime 
>> soon. The number of rx queues returned by rte_eth_dev_info_get() is in 
>> principle correct. Here the limiting factor is RSS capability and as far as 
>> I can see there is no way to query this information.


>>> Matias Elo(matiaselo) wrote:
>>> I'm not sure I got your point but anyway dev_info is type 'struct 
>>> rte_eth_dev_info' and comes from dpdk, so nothing can be added there.
>>> 
>>> As @lumag noted this fix is required because of a dpdk defect. With ixgbe 
>>> drivers rte_eth_dev_info_get() returns a max_rx_queues value which is not 
>>> valid (at least when using RSS). This is the only driver a have observer 
>>> this issue with.
>>> 
>>> The number 16 is explained in the comment line (1139) just above.


>>>> Matias Elo(matiaselo) wrote:
>>>> @Bill-Fischofer-Linaro Yes, the application has to request at least 
>>>> 'min_rx_burst' packets but it may receive less.


>>>>> Matias Elo(matiaselo) wrote:
>>>>> @nagarahalli This dpdk driver "feature" can be completely hidden from odp 
>>>>> applications (and it already is), so I don't support adding a new device 
>>>>> capability parameter. This would make writing simple applications more 
>>>>> complex. E.g. an application wouldn't be able to receive one packet at a 
>>>>> time anymore.


>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> Again, shouldn't this sort of configuration be a field in the `dev_info` 
>>>>>> struct rather than having to be special-cased for each driver? Also, 
>>>>>> must 16 be a "magic number" here?


>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>> It looks like this should be fixed inside DPDK rather than here.


>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> Presumably these drivers have a way of flushing short bursts? If the 
>>>>>>>> `min_rx_burst` is 4 and the other side sends an odd number of packets, 
>>>>>>>> presumably the receiver application isn't left hanging forever waiting 
>>>>>>>> for the rest of a burst that will never arrive?


>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> Wouldn't it be simpler (and more extensible) to have `min_rx_burst` 
>>>>>>>>> as a field in the `dev_info` struct? 


>>>>>>>>>> nagarahalli wrote
>>>>>>>>>> 'min_rx_burst' should be added to the capability as it is a 
>>>>>>>>>> restriction from the device. The application should adjust the 'num' 
>>>>>>>>>> according to the capability it reads from the pkt I/O.


https://github.com/Linaro/odp/pull/287#discussion_r150198861
updated_at 2017-11-10 10:19:17

Reply via email to