Add driver backpressure
Needs ReviewPublic
Actions

Authored by kmacy on Nov 27 2015, 1:26 AM.

Details

Reviewers

scottl
gallatin
hiren
lstewart
sbruno
rstone
shurd
erj

Group Reviewers

Intel Networking
transport
iflib

Summary

Rather than treating a transient txq overrun as a network loss event schedule another call to tcp_output when the software ring has partially drained.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kmacy updated this revision to Diff 10536.Nov 27 2015, 1:26 AM

kmacy retitled this revision from to Add driver backpressure.

kmacy updated this object.

kmacy edited the test plan for this revision. (Show Details)

kmacy added reviewers: scottl, lstewart, hiren, rstone, gallatin.

kmacy set the repository for this revision to rS FreeBSD src repository - subversion.

kmacy added a project: transport.

kmacy added a subscriber: benno.

Herald added reviewers: sbruno, Intel Networking. · View Herald TranscriptNov 27 2015, 1:26 AM

Herald added a subscriber: imp. · View Herald Transcript

kmacy edited edge metadata.Nov 27 2015, 1:37 AM

kmacy added a subscriber: gnn.

@kmacy I don't think iflib is part of -head yet. I see this patch includes references to sys/net/iflib* which also don't exist in -head yet. Can this patch be applied cleanly to -head? if not, I'd be great to have that done.

hiren added subscribers: j-nitrology.com, kbowling.Nov 27 2015, 7:47 AM

In D4295#90451, @hiren wrote:

@kmacy I don't think iflib is part of -head yet. I see this patch includes references to sys/net/iflib* which also don't exist in -head yet. Can this patch be applied cleanly to -head? if not, I'd be great to have that done.

It's not part of HEAD yet. The objective here is to elucidate the mechanism. I haven't been able to test it under real load yet. Do you really want to run with scissors? :D

I'm adding a "looks good to me" here for the Intel Reviewers as this adds functionality in the stack and the modification to the driver is required.

This revision is now accepted and ready to land.Dec 1 2015, 9:11 PM

In D4295#90577, @kmacy wrote:

In D4295#90451, @hiren wrote:

@kmacy I don't think iflib is part of -head yet. I see this patch includes references to sys/net/iflib* which also don't exist in -head yet. Can this patch be applied cleanly to -head? if not, I'd be great to have that done.

It's not part of HEAD yet. The objective here is to elucidate the mechanism. I haven't been able to test it under real load yet. Do you really want to run with scissors? :D

Heh, as we talked on the transport call, we can reproduce this problem and would love to see how the patch is handling it. :-)

I'd let others chime-in wrt design and what not.

Does ixgbe compile with the first parenthesis missing?

In D4295#127591, @cramerj_intel.com wrote:

Does ixgbe compile with the first parenthesis missing?

Odd. I'll update the patch. Thanks.

-M

erj resigned from this revision.Oct 3 2018, 5:22 PM

Herald added a reviewer: shurd. · View Herald TranscriptOct 3 2018, 5:22 PM

Herald added a reviewer: transport. · View Herald Transcript

Herald added a subscriber: ae. · View Herald Transcript

This revision now requires review to proceed.Oct 3 2018, 5:22 PM

zlei added a subscriber: zlei.Tue, Oct 22, 1:53 AM

Herald added a reviewer: iflib. · View Herald TranscriptTue, Oct 22, 1:53 AM

Herald added subscribers: glebius, melifaro. · View Herald Transcript

cc added a reviewer: cc.Tue, Oct 22, 8:04 PM

cc removed a reviewer: cc.

cc added a subscriber: cc.

Why is this re-surfacing?

I think the ratelimit code has a better solution, since TCP can query how full the nic queue is and avoid ENOBUFs entirely by avoiding sending on a full queue. The problem with doing this in general is that, outside of ratelimit, the TCP stack has no way to determine what NIC queue a packet will be sent on.

In D4295#1077477, @gallatin wrote:

Why is this re-surfacing?

I think the ratelimit code has a better solution, since TCP can query how full the nic queue is and avoid ENOBUFs entirely by avoiding sending on a full queue. The problem with doing this in general is that, outside of ratelimit, the TCP stack has no way to determine what NIC queue a packet will be sent on.

Hi Drew, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280386