Page MenuHomeFreeBSD

em(4) Define new quirk for control of max DMA size
AbandonedPublic

Authored by sbruno on Feb 4 2016, 7:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Feb 5, 8:25 AM
Unknown Object (File)
Sat, Jan 25, 12:02 AM
Unknown Object (File)
Fri, Jan 24, 8:05 PM
Unknown Object (File)
Fri, Jan 24, 2:03 AM
Unknown Object (File)
Fri, Jan 24, 1:20 AM
Unknown Object (File)
Fri, Jan 24, 1:15 AM
Unknown Object (File)
Jan 5 2025, 12:53 PM
Unknown Object (File)
Nov 11 2024, 5:38 AM

Details

Reviewers
marius
Group Reviewers
Intel Networking
manpages
Summary

Add configuraable "quirk" to em(4) that lowers the maximum DMA
size to and from the card. Control the amount of mbufs that are
pushed down from the network stack into the card.

Add quirk for 82573E if detected.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2391
Build 2407: arc lint + arc unit

Event Timeline

sbruno retitled this revision from to em(4) Define new quirk for control of max DMA size.
sbruno updated this object.
sbruno edited the test plan for this revision. (Show Details)
sbruno added a reviewer: marius.
sys/dev/e1000/if_em.c
1895

I think this is a c99 variable length array, any reason to not leave it EM_MAX_SCATTER and let usages clamp it down?

sys/dev/e1000/if_em.c
1895

There's no reason to allocate the array to be bigger than we need. But I wonder if you're trying to tell me something else I don't understand.

Fixup SYSCTL declaration to point at correct variable and sysctl OID.

sys/dev/e1000/if_em.c
1895

You're changing to sizing the array at runtime with a variable rather than how it was with a constant, this is c99 magic and I wouldn't think it would be allowed in the kernel but somebody else will have to clarify.

sbruno edited edge metadata.

Address kevin's comments and don't do silly things with arrays.

Update diff to latest head update of e1000

Thanks Kevin, I've made this change after someone in IRC was able to dumb down the explanation and translate it to "bruno".

Okay, where's the equivalent of the "I wish you hadn't submitted this"-button in this thingy?

share/man/man4/em.4
263

That description is plain wrong; max_dma controls the maximum number of DMA segments used per TX mbuf. Accordingly, it translates to the maximum TSO segment count advertized and not the TSO size.

sys/dev/e1000/if_em.c
402

Likewise wrong; the maximum number of descriptors that may be pushed to the adapter is unchanged. Instead, em_max_dma is the maximum number of DMA segments per TX mbuf. That just indirectly relates to the number of descriptors _per_ packet sent. As such, {em_,}max_scatter also would be a better name than {em_,}max_dma.

679

That's total crap.

  1. em_max_dma is global to em(4) so if there's a 82573 in a system along with any number of other non-82573 MACs driven by em(4), the maximum number of DMA segments per TX mbuf is unnecessarily altered for these latter, too.
  1. According to erj@: "I think the maximum amount of DMA segments is 40 for most Intel products." If that's true, the default of EM_MAX_SCATTER should be 40 with a whitelist of MACs that actually can do 64, not the inverse. Same for EM_MAX_SCATTER in if_lem.h and IGB_MAX_SCATTER.
  1. Is this change actually known to solve anything? If it fixes the watchdog timeouts seen at link speeds below Gigabit Ethernet, r295133 should be reverted along with lowering the maximum to 40 segments.

This review is being abandoned in favor of D5238