iflib: Do not ring the transmit doorbell when nothing is pending

[rcm]

Summary

For a lightly used ring iflib_txd_db_check() may defer zero descriptors,
so its "pending >= limit" test is true even when nothing has been queued
since the last doorbell. iflib_txq_drain() calls it before, inside and
after its loop, so a sender that drains its own packet wrote the tail
register three times per packet, twice with the value the hardware
already had.

The log of 81be655266fa ("iflib: ensure that tx interrupts enabled and
cleanups") calls skipping the doorbell when db_pending is zero "an
obvious missing optimization"; the comparison against a limit of zero
defeated it. vmx(4) and mgb(4) have dropped such repeated requests in
the driver since 2019. Return early when nothing is pending.

MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

Test Plan

On a four-core router with Intel I226-V ports (igc) and four queues,
forwarding 64-byte UDP packets goes from 0.95 to 2.5 Mpps in one
direction and from 1.5 to 4.2 Mpps in both.

[kbowling]

I'm about to leave on a business trip so I can't give a proper affirmative review, but one thing jumps out without digging into anything.. the multi-line comment and another control flow make it somewhat unpleasing to me. I would investigate a couple ways of writing this, either build the ring decision up with single statement ifs (and line comments as needed), or use de morgans to move true or false around so there are early exits and only one ring decision (perhaps de-indented.. whatever ends up feeling clean after playing with it.

[rcm]

In D60290#1384499, @kbowling wrote:

I'm about to leave on a business trip so I can't give a proper affirmative review, but one thing jumps out without digging into anything.. the multi-line comment and another control flow make it somewhat unpleasing to me. I would investigate a couple ways of writing this, either build the ring decision up with single statement ifs (and line comments as needed), or use de morgans to move true or false around so there are early exits and only one ring decision (perhaps de-indented.. whatever ends up feeling clean after playing with it.

I tried both and went with the De Morgan form: two early exits, then the doorbell write as the de-indented tail of the function.

[gallatin]

sys/net/iflib.c

3464

If I'm reading things correctly, the only functional change is this line. That seems reasonable to me.

The next hunk just seems to invert the logic so as to un-indent the flush. If that's true, thats probably a worthy change, but I think it should be submitted separately.

[rcm]

sys/net/iflib.c

3464

Yes that is correct. I can split these two changes

[rcm]

The diff is now just the functional change, the early return in iflib_txd_db_check() when ift_db_pending is 0. The inversion will be posted separately as a no-functional-change follow-up.