[dpdk-dev] [RFC] ethdev: rte_eth_rx_burst() requirements for nb_pkts

[Morten Brørup]

Jeff and Ethernet API maintainers Thomas, Ferruh and Andrew,

I'm hijacking this patch thread to propose a small API modification that prevents unnecessarily performance degradations.

> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jeff Guo
> Sent: Thursday, August 27, 2020 9:55 AM
> 
> The limitation of burst size in vector rx was removed, since it should
> retrieve as much received packets as possible. And also the scattered
> receive path should use a wrapper function to achieve the goal of
> burst maximizing.
> 
> This patch set aims to maximize vector rx burst for for
> ixgbe/i40e/ice/iavf PMDs.
> 

Now I'm going to be pedantic and say that it still doesn't conform to the rte_eth_rx_burst() API, because the API does not specify any minimum requirement for nb_pkts.

In theory, that could also be fixed in the driver by calling the non-vector function from the vector functions if nb_pkts is too small for the vector implementation.

However, I think that calling rte_eth_rx_burst() with a small nb_pkts is silly and not in the spirit of DPDK, and introducing an additional comparison for a small nb_pkts in the driver vector functions would degrade their performance (only slightly, but anyway).

Instead, I propose that the rte_eth_rx_burst() API is updated with a minimum requirement for nb_pkts. This minimum requirement should be supported by all Ethernet drivers, instead of having minimum requirements for nb_pkts depending on driver and vector function.


I also have a small comment about the description for all the main rx functions:

+/**
+ * vPMD receive routine that reassembles scattered packets.
+ * Main receive routine that can handle arbitrary burst sizes
+ * Notice:
+ * - nb_pkts < RTE_I40E_DESCS_PER_LOOP, just return no packet
+ */

It says "can handle arbitrary burst sizes", but bears a notice that it really cannot. So please remove that line from all these functions.


Med venlig hilsen / kind regards
- Morten Brørup