Page MenuHomeFreeBSD

e1000: Fix and clean up usage of DMA and TSO segments
ClosedPublic

Authored by marius on Feb 9 2016, 11:17 PM.

Details

Summary
  • At Intel it is believed that most of their products support "only" 40 DMA segments so lower {EM,IGB}_MAX_SCATTER accordingly. Actually, 40 is more than plenty to handle full size TSO packets so it doesn't make sense to further distinguish between MAC variants that really can do 64 DMA segments. Moreover, capping at 40 DMA segments limits the stack usage of {em,igb}_xmit() that - given the rare use of more than these - previously hardly was justifiable, while still being sufficient to avoid the problems seen with em(4) and EM_MAX_SCATTER set to 32.
  • In igb(4), pass the actually supported TSO parameters up the stack. Previously, the defaults set in if_attach_internal() were applied, i. e. a maximum of 35 TSO segments, which made supporting more than these in the driver pointless. However, this might explain why no problems were seen with IGB_MAX_SCATTER at 64.
  • In em(4), take the 5 m_pullup(9) invocations performed by em_xmit() in the TSO case into account when reporting TSO parameters upwards. In the worst case, each of these calls will add another mbuf and, thus, the requirement for an additional DMA segment. So for best performance, it doesn't make sense to advertize a maximum of TSO segments that typically will require defragmentation in em_xmit(). Again, this leaves enough room to handle full size TSO packets.
  • Drop TSO macros from if_lem.h given that corresponding MACS don't support TSO in the first place.

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

marius updated this revision to Diff 13164.Feb 9 2016, 11:17 PM
marius retitled this revision from to e1000: Fix and clean up usage of DMA and TSO segments.
marius updated this object.
marius edited the test plan for this revision. (Show Details)
marius added a reviewer: erj.
marius set the repository for this revision to rS FreeBSD src repository.
sbruno accepted this revision.Feb 16 2016, 12:40 PM
sbruno edited edge metadata.
This revision is now accepted and ready to land.Feb 16 2016, 12:40 PM
erj added inline comments.Feb 19 2016, 11:21 PM
sys/dev/e1000/if_igb.c
3145 ↗(On Diff #13164)

You don't want to set this value to IGB_MAX_SCATTER - 5, like in em?

marius added inline comments.Feb 19 2016, 11:50 PM
sys/dev/e1000/if_igb.c
3145 ↗(On Diff #13164)

No, because unlike em_xmit(), igb_xmit() doesn't need to do m_pullup(9)'s adding to the count of segments required (the reverse, i. e. the em(4) case, is explained in the summary).
In principle, IGB_MAX_SCATTER could be lowered to 35, though, given there's no actual need for that extra 5 segments.

erj accepted this revision.Feb 23 2016, 12:49 AM
erj edited edge metadata.
jeffrey.e.pieper_intel.com edited edge metadata.

I'm not seeing any issues with the testing I'm doing, so looks good to me.

This revision was automatically updated to reflect the committed changes.