improve CPU cores affinity to NUMA nodes v9.4 #12369
Conversation
Split the code into multiple functions for easier readability.
Part of Ticket 2321 work to remove unnecessary lists from the config file. Ticket: 2321
Provide backward compatibility with the previous configuration format to allow smooth transition to the new format. The commit adds docs about the new format and the introduced changes.
Using the new configuration format, it is now possible to set CPU affinity settings per interface. The threading.autopin option has been added to automatically use CPUs from the same NUMA node as the interface. The autopin option requires hwloc-devel / hwloc-dev to be installed and --enable-hwloc flag in configure script. Ticket: 7036
lukashino
requested review from
jasonish,
jufajardini,
victorjulien and
a team
as code owners
|
Reposting my comment from #12359 @ct0br0 commented:
It depends. By default if you have e.g.
No, this PR doesn't add extra expression logic like you suggest. In DPDK it is implemented to exclude management threads but in other capture modes, I didn't feel comfortable making the change.
Well, I think what you are suggesting here is to be more expressive with the CPU lists. This would allow a user to not have the knowledge of the system so much - on the level of individual CPU cores. This work has not been focused on this and view it as a follow-up.
@jlucovsky commented:
It is possible, I guess I wanted to keep fewer structures around. I can rework it if it is the preferred approach. Edit:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12369 +/- ##
==========================================
- Coverage 82.54% 82.45% -0.09%
==========================================
Files 912 913 +1
Lines 258028 258296 +268
==========================================
- Hits 212988 212985 -3
- Misses 45040 45311 +271
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 24141 |
| @@ -1777,6 +1777,7 @@ spm-algo: auto | |||
| # Suricata is multi-threaded. Here the threading can be influenced. | |||
| threading: | |||
| set-cpu-affinity: no | |||
| autopin: no | |||
There was a problem hiding this comment.
Is autopin or pinning common language for this? I just ask, cause its new to Suricata, and the option is called cpu-affinity, not cpu-pinning.
There was a problem hiding this comment.
Good point though cpu pinning is often used. It's relatively interchangeable term though somewhere I found that it actually means strict configuration of the thread to the given CPU core whereas affinity is rather the highly preferred variant. And CPU pinning is what we actually do in Suri.
There was a problem hiding this comment.
Is there a way to have autopin not use a group of cores?
Linux likes to use the first core of each socket and some chips have compute-complex (AMD).
It might be helpful to better support these cases with an "exclude" list, e.g.,
auto-pin-exclude: [0-3,22-25] # Don't use first compute complex on each 22-core socket
| CPU cores on the same NUMA nodes as the network capture interface have | ||
| reduced memory access latency and increased the performance of Suricata. | ||
| This is enabled by setting the `autopin` option to `yes` in the threading | ||
| section. This option is available for worker-cpu-set and receive-cpu-set. |
There was a problem hiding this comment.
nit: backticks
Can autopin: yes be used on a single-socket CPU? Is there a warning message? I know it has no impact when there is 1 numa node.
There was a problem hiding this comment.
yes it can be used but there is no warning, should there be? As you mention, it has no effect in there but I think it makes the configuration more portable across different systems so I would not issue one. But it of course can be added np.
|
Re to Jeff:
Definitely possible although I view this as an additional feature. But just to make sure we are on the same page - autopin tries to select the "best" CPU cores from available worker-cpu-set or (interface-specific-cpu-set if available) so the user solution to this would be to restrict the list of allowed CPUs to use so your |
|
I really like this, so some questions:
2.1. Would it create 6 workers? 2.2 What happens if the |
The user can change it and, actually, atm the user should specify it because I think no inheritance is in place and the interface-specific settings are initialized to the default values.
No, when the interface is initialized, it tries to look up the interface-specific node. If it is not found, then it falls back to worker-cpu-set. So worker-cpu-set is not even looked at when interface-specific node is found and is therefore ignored.
Not created.
Correct.
Just for the record - I am assuming you mean |
|
Next #12656 |
Followup of #12359
Redmine ticket:
https://redmine.openinfosecfoundation.org/issues/7036
https://redmine.openinfosecfoundation.org/issues/2321
This work allows more precise thread assignment and it is done either
This works with AF-PACKET and DPDK (and should with other capture modes as well). The primary target is workers runmode but it also works with autofp receive threads.
To try this PR:
Describe changes:
v9.4:
v9:
v8:
v7:
v6
On YAML changes:
CPU affinity section in
suricata.yamlhas been changed to (seesuricata.yaml.infor the full picture):Below is the reasoning for it.
When starting this work I wanted to follow the same structure as CPU set nodes that is:
converted to JSON it looks like:
{ "threading": { "cpu-affinity": [ { "worker-cpu-set": { "cpu": [ "all" ], "mode": "exclusive" } } ] } }So worker-cpu-set is an object by itself, it holds the key that is named "worker-cpu-set" and only that contains its children properties (CPU, prio, etc.).
But when adding the interface-specific node I found out that having the interface names directly as the keys for the object storing the properties (CPU, prio, etc.) can change its name to something else, in this case to
net-bonding0.So adding it as a value:
yields an invalid YAML construct.
Therefore, for interface-specific CPU affinity settings, I ended up putting it on the same level as the interface name itself. This follows the same structure as is established in e.g. capture-mode interface definition (dpdk.interfaces, af-packet).
Note: In the matter of this I transferred the base *-cpu-set nodes from the list items as part of the Ticket 2321.
The CPU affinity interface-specific design I ended up with is:
JSON visualization of this:
{ "threading": { "cpu-affinity": { "worker-cpu-set": { "cpu": [ "all" ], "mode": "exclusive", "interface-specific-cpu-set": [ { "interface": "net_bonding0", "cpu": [ "all" ] }, { "interface": "net_bonding1", "cpu": [ "all" ] } ] } } } }This leaves the base {worker,receive,management}-cpu-set nodes in the original setting and as we know it. The internal code can handle this because the names of these are known beforehand and can "step down" to the children level to inspect cpu-affinity properties. The dynamic part - interface-specific-cpu-set contains all properties on the same level.
Other unsuccessful YAML designs
Specifying the NUMA affinity in the individual capture modes
Example
But management / other threads would still be required to configure elsewhere. I liked the proposed approach more because all affinity-related settings are centralized.
Not having a list nodes in the YAML
YAML library changes net_bonding to net-bonding when YAML is loaded - so this is a nogo.
Have interface-related properties as children
This is a disallowed construct in YAML, so another nogo. A similar pattern is currently with *-cpu-set nodes but the different here is that the parent list item has a value (in this case net_bonding0). That is what is causing problems. Interface related therefore need to right under "interface".
PRO TIP: A good way to visualize different YAML structures is by using YAML to JSON converters. Currently it creates object like:
[ { "interface": "3b:00.0", "mode": "exclusive", } ]