Page MenuHomeFreeBSD

iflib: netmap: add per-tx-queue netmap support
ClosedPublic

Authored by vmaffione on Jun 13 2020, 7:59 AM.

Details

Summary

Add support for those cases where only a subset of the TX rings are opened, and where RX rings are not opened.
Rather than simply checking IFCAP_NETMAP, also use the netmap_kring_on() helper or the return value of netmap_tx_irq().
In the interrupt handler, in particular, there is no need to call isc_credits_update.

Test Plan

Tested with pkt-gen, bridge, vale-ctl on em devices within a VM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I suspect something went wrong with the email notification, so this is just a ping message to make sure reviewers are aware of this patch.

Can you add context please ? I upload patches using git diff -U9999999 to get the full file to show up.

sys/net/iflib.c
3763 ↗(On Diff #73549)

I have a concern is for performance in the netmap compiled in, but inactive case.

In this case, it seems that you've replaced a very cheap check of an ifp flag IFCAP_NETMAP (which is almost certain to be in cache) with a function call which then de-references ifp->if_netmap, which is likely to be cold if netmap is disabled.

Why did you need to remove the IFCAP_NETMAP check?

vmaffione edited the summary of this revision. (Show Details)

Be more cache-friendly by checking IFCAP_NETMAP before calling netmap_tx_irq.

vmaffione added inline comments.
sys/net/iflib.c
3763 ↗(On Diff #73549)

That's a very good point, thanks.
Yes, we can always check IFCAP_NETMAP before actually checking if the specific netmap ring is on (with netmap_tx_irq or netmap_kring_on).
I removed that check just to simplify the code, but I do agree that this causes unnecessary cache misses (and a function call) in the (normal) case where netmap is not in use.
On the contrary, there practically no impact on the case where netmap is in use.

Thank you for making this change!

This revision is now accepted and ready to land.Jun 24 2020, 10:47 PM
This revision was automatically updated to reflect the committed changes.
vmaffione marked an inline comment as done.