Page MenuHomeFreeBSD

Select lacp egress ports based on NUMA domain
ClosedPublic

Authored by gallatin on Apr 25 2019, 9:07 PM.

Details

Summary

This change creates an array of port maps indexed by numa domain for lacp port selection. If we have lacp interfaces in more than one domain, then we select the egress port by indexing into the numa port maps and picking a port on the appropriate numa domain.

This is behavior is controlled by the new ifconfig use_numa flag and net.link.lagg.use_numa sysctl/tunable (both modeled after the existing use_flowid), which default to enabled.

This is the first in a series of patches to use the numa information recently added to ifnet, mbuf and inpcb structs.

Test Plan

On a 4-node NUMA machine, attach NICs to domains 1 and 3 and use them in lacp. Use a dtrace script like this:
fbt::mlx5e_xmit:entry
/ args[1]->m_pkthdr.numa_domain == 3 /
{

dom = args[0]->if_numa_domain;
@dom = lquantize(dom, 0, 3, 1);

}

And observe that when use_numa is disabled, we see packets intended for domain 3 split evenly across the NICs on domain 1 and 3:

dtrace: script '/d/port.d' matched 1 probe
^C

value  ------------- Distribution ------------- count
    0 |                                         0
    1 |@@@@@@@@@@@@@@@@@@@@                     191481
    2 |                                         0
 >= 3 |@@@@@@@@@@@@@@@@@@@@                     183655

After enabling use_numa, we see these packets going out only domain 3

c368.sjc002.dev# /tmp/ifconfig lagg0 use_numa
c368.sjc002.dev# dtrace -s /d/port.d
dtrace: script '/d/port.d' matched 1 probe
^C

value  ------------- Distribution ------------- count
    2 |                                         0
 >= 3 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 297132

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

gallatin created this revision.Apr 25 2019, 9:07 PM
markj added inline comments.Apr 26 2019, 7:37 AM
sbin/ifconfig/ifconfig.8
2501 ↗(On Diff #56667)

Style: new sentences should start on new lines.

Also, .Xr NUMA 4 ?

gallatin updated this revision to Diff 56706.Apr 26 2019, 1:58 PM

Fix style issues pointed out by markj in the ifconfig man page change, and add cross-references to the numa(4) man page, as he suggests.

gallatin marked an inline comment as done.Apr 26 2019, 1:58 PM
hselasky added inline comments.Apr 26 2019, 7:39 PM
sys/net/ieee8023ad_lacp.c
1086 ↗(On Diff #56706)

off by one??

if (domain >= MAXMEMDOM)

gallatin updated this revision to Diff 56731.Apr 26 2019, 9:29 PM

Fix off-by-one error pointed out by hselasky

gallatin marked an inline comment as done.Apr 26 2019, 9:30 PM
gallatin added inline comments.
sys/net/ieee8023ad_lacp.c
1086 ↗(On Diff #56706)

Thanks for catching that!

hselasky accepted this revision.Apr 27 2019, 10:16 AM
This revision is now accepted and ready to land.Apr 27 2019, 10:16 AM
scottl accepted this revision.Apr 30 2019, 6:53 AM
markj accepted this revision.May 2 2019, 5:23 PM
bz requested changes to this revision.May 2 2019, 5:51 PM
bz added inline comments.
sbin/ifconfig/ifconfig.8
31 ↗(On Diff #56731)

Please don't forget to update .Dd on the commit day

2504 ↗(On Diff #56731)

Also lines should be wrapped properly at 72 or something; someone from manpages can help with the exact details.

sys/net/ieee8023ad_lacp.c
865 ↗(On Diff #56731)

if you write it as

} else
#endif
{
   ..
}

you can save the extra #ifdef NUMA for the }

sys/net/if_lagg.c
271 ↗(On Diff #56731)

Why do you set arg2 to 1 despite having arg1 not a NULL ptr?

This revision now requires changes to proceed.May 2 2019, 5:51 PM
gallatin marked an inline comment as done.May 2 2019, 6:19 PM
gallatin added inline comments.
sbin/ifconfig/ifconfig.8
2504 ↗(On Diff #56731)

Thanks for noticing. I managed to back out a patch that fixed this when I fixed something else. Sigh. I cannot wait until we can use a sane review system with pull requests.

gallatin updated this revision to Diff 56987.May 2 2019, 11:38 PM
  • Restored the fixes to the manpage formatting and xref's requested by markj (as noticed by bz)
  • Updated the date on the manpage as requested by bz (speculative date for now)
  • Changed ifdef / { matching as requested by bz
  • Changed net.link.lagg.default_use_numa init as requested by bz (i think)
  • Fixed printing of USE_NUMA flag in ifconfig, as noticed by me.
gallatin marked 3 inline comments as done.May 2 2019, 11:40 PM
gallatin added inline comments.
sys/net/if_lagg.c
271 ↗(On Diff #56731)

I'm afraid I cannot parse what you're asking for. Can you please elaborate? I think I may have fixed something, but I'm just guessing at this point.

bz accepted this revision.May 3 2019, 7:15 AM
This revision is now accepted and ready to land.May 3 2019, 7:15 AM
markj accepted this revision.May 3 2019, 1:10 PM
hselasky accepted this revision.May 3 2019, 1:53 PM
This revision was automatically updated to reflect the committed changes.