Skip to content

Commit

Permalink
Don't use '-A' in network.py on non-numa systems (BugFix) (#711)
Browse files Browse the repository at this point in the history
* Fix: In network.py, don't pass -A option to iperf3 if NUMA is unsupported

* add first tests for the network.py

---------

Co-authored-by: Maciej Kisielewski <maciej.kisielewski@canonical.com>
  • Loading branch information
rodwsmith and kissiel authored Jan 7, 2024
1 parent 67ab000 commit 61dfbe3
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
18 changes: 10 additions & 8 deletions providers/base/bin/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,15 @@ def find_numa(self, device):
with open(filename, "r") as file:
node_num = int(file.read())
except FileNotFoundError:
logging.warning("WARNING: Could not find the NUMA node "
"associated with {}!".format(device))
logging.warning("Setting the association to NUMA node 0, "
"which may not be optimal!")
node_num = 0
node_num = -1
# Some systems (that don't support NUMA?) produce a node_num of -1.
# Change this to 0, which seems to be correct....
# Later in the script, this will be interpreted to omit the -A option
# to iperf3, thus disabling NUMA features.
if node_num == -1:
node_num = 0
logging.info("NUMA node of {} is {}....".format(device, node_num))
logging.warning("WARNING: Could not find the NUMA node "
"associated with {}!".format(device))
else:
logging.info("NUMA node of {} is {}....".format(device, node_num))
return node_num

def extract_core_list(self, line):
Expand Down Expand Up @@ -214,6 +213,9 @@ def find_cores(self, numa_node):
"""Return a list of CPU cores tied to the specified NUMA node."""
numa_return = check_output("lscpu", universal_newlines=True,
stderr=STDOUT).split("\n")
# Note: If numa_node = -1, the below will never find a match, so
# core_list will remain empty, and later in the script, the -A option
# to iperf3 will be dropped.
expression = "NUMA node.*" + str(numa_node) + ".*CPU"

regex = re.compile(expression)
Expand Down
35 changes: 35 additions & 0 deletions providers/base/tests/test_network.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/usr/bin/env python3
# Copyright 2024 Canonical Ltd.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 3,
# as published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import unittest
from unittest.mock import patch, mock_open

from network import IPerfPerformanceTest

class IPerfPerfomanceTestTests(unittest.TestCase):

def test_find_numa_reports_node(self):
with patch('builtins.open', mock_open(read_data="1")) as mo:
returned = IPerfPerformanceTest.find_numa(None, "device")
self.assertEqual(returned, 1)

def test_find_numa_minus_one_from_sysfs(self):
with patch('builtins.open', mock_open(read_data="-1")) as mo:
returned = IPerfPerformanceTest.find_numa(None, "device")
self.assertEqual(returned, -1)
def test_find_numa_numa_node_not_found(self):
with patch('builtins.open', mock_open()) as mo:
mo.side_effect = FileNotFoundError
returned = IPerfPerformanceTest.find_numa(None, "device")
self.assertEqual(returned, -1)

0 comments on commit 61dfbe3

Please sign in to comment.