ena: Batch RX statistics updates

[akiyano_amazon.com]

Summary

Move per-packet counter_enter/counter_exit pairs out of the RX
processing loop and batch them into a single update after the
loop completes.

Previously, each received packet triggered two separate
counter_enter/counter_exit blocks -- one for bytes and one for
packet count. This commit accumulates totals in local variables
and updates all four counters (ring and hw stats for both packets
and bytes) in a single counter_enter/counter_exit block after the
loop.

Also move the stats update to after the refill and LRO flush
so that the error path (goto update_stats) and the normal path
converge at the same label, avoiding code duplication.

Submitted by: David Arinzon <[e-mail]>
MFC after: 2 weeks
Sponsored by: Amazon, Inc.

[cperciva]

The commit message refers to critical sections, but in fact counter_{enter,exit} are no-ops everywhere except powerpc, and somehow I doubt ENA will ever run on powerpcs. So the win here isn't about critical sections; it's just about reducing the number of atomic operations.

Can you measure an improvement in benchmarks? It's clearly a good change to make; I'm just wondering if it's big enough to show up.

sys/dev/ena/ena_datapath.c

576

Why the different types? Both are being added to u64 counters, and both values will always fit into a 32-bit value -- the byte count is going to be at most 4MB aka 1024 4k descriptors right now, and even if we start using larger descriptors and larger rings it's hard to imagine it ever getting close to 4 GB.

619

BTW this will now return budget <= 0 instead of 0. I think this is fine since at this point budget is guaranteed to be positive?

[akiyano_amazon.com]

sys/dev/ena/ena_datapath.c

619

Correct, and thanks for checking. budget is guaranteed positive at that point: it starts at
ENA_RX_DESC_BUDGET (256), the loop is a do/while that only decrements budget at the bottom (
budget -= descs) and re-tests while (budget > 0), and the goto fires at the top of the loop body
before the decrement. So on entry to any iteration budget > 0, meaning budget <= 0 is false and we
return 0 — same as the previous explicit return (0). No behavior change on the error path.

[akiyano_amazon.com]

In D58240#1335929, @cperciva wrote:

The commit message refers to critical sections, but in fact counter_{enter,exit} are no-ops everywhere except powerpc, and somehow I doubt ENA will ever run on powerpcs. So the win here isn't about critical sections; it's just about reducing the number of atomic operations.

Can you measure an improvement in benchmarks? It's clearly a good change to make; I'm just wondering if it's big enough to show up.

You're right that the win isn't about critical sections — I've reworded the commit message.

On benchmarks: We don't expect this to be measurable. It's a few cycles per packet on a datapath that
isn't cycle-sensitive the way DPDK is, so it won't show up in throughput or latency. We're making the
change for the reduced per-packet work and the single, cleaner exit path — not for a measurable
number.