From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
Sent: Thursday, August 27, 2020 11:44 AM

On Thu, Aug 27, 2020 at 11:31:15AM +0200, Morten Brørup wrote:

quoted

quoted

From: Bruce Richardson [mailto:bruce.richardson@intel.com]
Sent: Thursday, August 27, 2020 11:10 AM

On Thu, Aug 27, 2020 at 10:40:11AM +0200, Morten Brørup wrote:

quoted

Jeff and Ethernet API maintainers Thomas, Ferruh and Andrew,

I'm hijacking this patch thread to propose a small API

modification

quoted

quoted

that prevents unnecessarily performance degradations.

quoted

quoted

From: dev [mailto:dev-bounces@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

quoted

quoted

retrieve as much received packets as possible. And also the

scattered

quoted

quoted

receive path should use a wrapper function to achieve the goal

of

quoted

quoted

quoted

quoted

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

quoted

quoted

the rte_eth_rx_burst() API, because the API does not specify any
minimum requirement for nb_pkts.

quoted

In theory, that could also be fixed in the driver by calling the

non-

quoted

quoted

vector function from the vector functions if nb_pkts is too small

for

quoted

quoted

the vector implementation.

quoted

However, I think that calling rte_eth_rx_burst() with a small

nb_pkts

quoted

quoted

is silly and not in the spirit of DPDK, and introducing an

additional

quoted

quoted

comparison for a small nb_pkts in the driver vector functions would
degrade their performance (only slightly, but anyway).

quoted

Actually, I'd like to see a confirmed measurement showing a

slowdown

quoted

quoted

before
we discard such an option. :-)

Good point!

quoted

While I agree that using small bursts is
not
keeping with the design approach of DPDK of using large bursts to
amortize
costs and allow prefetching, there are cases where a user/app may

want

quoted

quoted

a
small burst size, e.g. 4, for latency reasons, and we need a way to
support
that.

I assume that calling rte_eth_rx_burst() with nb_pkts=32 returns 4

packets if only 4 packets are available, so you would need to be
extremely latency sensitive to call it with a smaller nb_pkts. I guess
that high frequency trading is the only real life scenario here.

quoted

Yes, it really boils down to whether you are prepared to accept lower
max throughput or dropped packets in order to gain lower latency.

quoted

quoted

Since the path selection is dynamic, we need to either:
a) provide a way for the user to specify that they will use smaller
bursts
and so that vector functions should not be used
b) have the vector functions transparently fallback to the scalar

ones

quoted

quoted

if
used with smaller bursts

Of these, option b) is simpler, and should be low cost since any

check

quoted

quoted

is
just once per burst, and - assuming an app is written using the

same

quoted

quoted

request-size each time - should be entirely predictable after the

first

quoted

quoted

call.

Why does everyone assume that DPDK applications are so simple that

the branch predictor will cover the entire data path? I hear this
argument over and over again, and by principle I disagree with it!

quoted

Fair enough, that was an assumption on my part. Do you see in your apps
many cases where branches are getting mispredicted despite going the
same
way each time though the code?

We haven't looked deeply into this, but I don't think so.

My objection is of a more general nature. As a library, DPDK cannot assume that applications using it are simple, and - based on that assumption - take away resources that could have been available for the application.

The Intel general optimization guidelines specifies that code should be arranged to be consistent with the static branch prediction algorithm: make the fall-through code following a conditional branch be the likely target for a branch with a forward target, and make the fall-through code following a conditional branch be the unlikely target for a branch with a backward target.

It also says: Conditional branches that are never taken do not consume BTB resources.

Somehow this last detail is completely ignored by DPDK developers.

We put a lot of effort into conserving resources in most areas in DPDK, but when it comes to the branch prediction target buffer (BTB), we gladly organize code with branches turning the wrong way, thus unnecessarily consuming BTB entries. And the argument goes: The branch predictor will catch it after the first time.

quoted

How about c): add rte_eth_rx() and rte_eth_tx() functions for

receiving/transmitting a single packet. The ring library has such
functions.

quoted

Optimized single-packet functions might even perform better than

calling the burst functions with nb_pkts=1. Great for latency focused
applications. :-)

quoted

That is another option, yes.
A further option is to add to the vector code a one-off switch to check
first
time it's called that the request size is not lower than the min
supported
(again basing on the assumption that one is not going to be varying the
burst size asked - which may not be true in call cases but won't leave
us
any worse off than we are now!).

I certainly don't support this option. But it was worth mentioning.