Page MenuHomeFreeBSD

Update iflib to support more NIC designs
ClosedPublic

Authored by shurd on Aug 1 2016, 11:49 PM.

Details

Summary
  • Move group task queue into kern/subr_gtaskqueue.c
  • Change intr_enable to return an int so it can be detected if it's not implemented
  • Allow different TX/RX queues per set to be different sizes
  • Don't split up TX mbufs before transmit
  • Allow a completion queue for TX as well as RX
  • Pass the RX budget to isc_rxd_available() to allow an earlier return and avoid multiple calls
Test Plan

Tested with the bnxt driver, and a kernel-only make universe

Diff Detail

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

Event Timeline

shurd retitled this revision from to Update iflib to support more NIC designs.
shurd updated this object.
shurd edited the test plan for this revision. (Show Details)
shurd added reviewers: gallatin, glebius, rstone, sbruno, scottl.
sys/net/iflib.c
2260 ↗(On Diff #18951)

Why did you initialize this to mp? Its only used in a fairly rare case, and then it is initialized to the second mbuf in the chain

2482 ↗(On Diff #18951)

The way you're storing ntxd introduces a lot of indirection in a number of fairly hot code paths. I'm commenting here, but that applies to the same use in lots of places.

Can you elaborate a bit on why all this indirection is needed? Can't you just store ntxd in the txq and not have to go through several layers of pointers to get it?

4234 ↗(On Diff #18951)

Why use the long form unless you're also going to use the tcp_lro_queue_mbuf() routine?

And if you are going to use that, then 64 is a bit small. I like to use at least 1024, or the ring size, whichever is smaller.

sys/net/iflib.c
2260 ↗(On Diff #18951)

n is being initialized to avoid an unused variable warning when building NOINET kernels.

Use txq->ift_size where possible
instead of txq->ift_ctx->ifc_softc_ctx.isc_ntxd[txq->ift_br_offset]

sys/net/iflib.c
2482 ↗(On Diff #18951)

Yes, in fact it's already in txq->ift_size.

4234 ↗(On Diff #18951)

I'll see if kmacy@ comments on this before changing it.

  • More avoidance of ctx->ifc_softc_ctx.isc_ntxd[txq->ift_br_offset]
  • Use minimumin of 1024 or free list size for LRO init
sys/net/iflib.c
4234 ↗(On Diff #18951)

Now using the minimum of 1024 or the first rx free list size.

Are there further reservations to this change?

This is the API he has been working against for the bnxt driver so he needs this to go in before bnxt does.

In D7393#155251, @kmacy wrote:

Are there further reservations to this change?

This is the API he has been working against for the bnxt driver so he needs this to go in before bnxt does.

I was actually waiting to hear your feedback. If you've reviewed it & are OK with it, I'm OK with it. He's addressed my objections.

gallatin edited edge metadata.
This revision is now accepted and ready to land.Aug 11 2016, 12:43 PM
scottl edited edge metadata.
This revision was automatically updated to reflect the committed changes.