Add support for sysctl knobs to live tune the tx packet processing limits in igb and fix a wrap-around bug.
ClosedPublic
Actions

Authored by sbruno on Oct 30 2015, 5:27 PM.

Details

Reviewers

hiren
erj
gnn

Group Reviewers

Intel Networking

Commits

rS292670: Add support for sysctl knobs to live tune the tx packet processing limits

Summary

Prior this was using limit which was simply delcared but not initialized. With it being a u16, it would wrap around to 65535 after the first loop,
meaning the old limit was really 65536 packets and not truly unlimited.
With a 1G card, it shouldn't make much a difference, but initialize it
to -1 to be safe and more clear on the setting which matches the tool tip.

Add tx processing limit sysctl to igb(4)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

sbruno updated this revision to Diff 9828.Oct 30 2015, 5:27 PM

sbruno retitled this revision from to Add support for sysctl knobs to live tune the tx packet processing limits in igb. Prior this was using limit which was simply delcared but not initialized With it being a u16, it would wrap around to 65535 after the first loop, meaning the old....

sbruno updated this object.

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

sbruno added reviewers: hiren, erj.

Herald added a reviewer: Intel Networking. · View Herald TranscriptOct 30 2015, 5:27 PM

Herald added a subscriber: imp. · View Herald Transcript

sbruno edited edge metadata.Oct 30 2015, 5:27 PM

sbruno added a reviewer: gnn.

sbruno added subscribers: j-nitrology.com, kbowling.

hiren retitled this revision from Add support for sysctl knobs to live tune the tx packet processing limits in igb. Prior this was using limit which was simply delcared but not initialized With it being a u16, it would wrap around to 65535 after the first loop, meaning the old... to Add support for sysctl knobs to live tune the tx packet processing limits in igb and fix a wrap-around bug..Oct 30 2015, 5:31 PM

hiren updated this object.

j-nitrology.com updated this object.Oct 30 2015, 5:33 PM

Is there a way we can unify *_process_limit and *_processing_limit? I see both being used and that is confusing. I am not sure if its just the code and if user is presented with only one but in any case we should choose one and stick with it. BTW, this can be done as a followup/separate commit.

This change looks okay to me.

This revision is now accepted and ready to land.Oct 30 2015, 5:36 PM

In D4039#84487, @hiren wrote:

Is there a way we can unify *_process_limit and *_processing_limit? I see both being used and that is confusing. I am not sure if its just the code and if user is presented with only one but in any case we should choose one and stick with it. BTW, this can be done as a followup/separate commit.

This change looks okay to me.

So the process_limit is the global boot time tunable that becomes the default per adapter setting that can (now) be tuned live. That per adapter setting is called processing_limit. I agree though, we can probably give them some better naming. For it to make sense we'd probably want to change the sysctls to match, which I don't believe is safe:

hw.igb.tx_process_limit: -1
hw.igb.rx_process_limit: -1

dev.igb.1.tx_processing_limit: -1
dev.igb.1.rx_processing_limit: -1
dev.igb.0.tx_processing_limit: -1
dev.igb.0.rx_processing_limit: -1

Closed by commit rS292670: Add support for sysctl knobs to live tune the tx packet processing limits (authored by sbruno). · Explain WhyDec 23 2015, 9:54 PM

This revision was automatically updated to reflect the committed changes.