diff --git a/examples/ethtool_coalesce.yml b/examples/ethtool_coalesce.yml new file mode 100644 index 0000000..d0e8948 --- /dev/null +++ b/examples/ethtool_coalesce.yml @@ -0,0 +1,38 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- hosts: all + tasks: + - include_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ network_interface_name1 }}" + state: up + type: ethernet + ip: + dhcp4: no + auto6: no + ethtool: + coalesce: + adaptive_rx: yes + adaptive_tx: no + pkt_rate_high: 128 + pkt_rate_low: 128 + rx_frames: 128 + rx_frames_high: 128 + rx_frames_irq: 128 + rx_frames_low: 128 + rx_usecs: 128 + rx_usecs_high: 128 + rx_usecs_irq: 128 + rx_usecs_low: 128 + sample_interval: 128 + stats_block_usecs: 128 + tx_frames: 128 + tx_frames_high: 128 + tx_frames_irq: 128 + tx_frames_low: 128 + tx_usecs: 128 + tx_usecs_high: 128 + tx_usecs_irq: 128 + tx_usecs_low: 128 diff --git a/library/network_connections.py b/library/network_connections.py index 8c11605..731346e 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -402,6 +402,24 @@ class IfcfgUtil: " ".join(configured_features), ) + ethtool_coalesce = connection["ethtool"]["coalesce"] + configured_coalesce = [] + for coalesce, setting in ethtool_coalesce.items(): + if setting is not None: + if isinstance(setting, bool): + setting = int(setting) + configured_coalesce.append( + "%s %s" % (coalesce.replace("_", "-"), setting) + ) + + if configured_coalesce: + if ethtool_options: + ethtool_options += " ; " + ethtool_options += "-C %s %s" % ( + connection["interface_name"], + " ".join(configured_coalesce), + ) + if ethtool_options: ifcfg["ETHTOOL_OPTS"] = ethtool_options @@ -907,6 +925,15 @@ class NMUtil: else: s_ethtool.set_feature(nm_feature, NM.Ternary.FALSE) + for coalesce, setting in connection["ethtool"]["coalesce"].items(): + nm_coalesce = nm_provider.get_nm_ethtool_coalesce(coalesce) + + if nm_coalesce: + if setting is None: + s_ethtool.option_set(nm_coalesce, None) + else: + s_ethtool.option_set_uint32(nm_coalesce, int(setting)) + if connection["mtu"]: if connection["type"] == "infiniband": s_infiniband = self.connection_ensure_setting(con, NM.SettingInfiniband) @@ -2006,9 +2033,9 @@ class Cmd_nm(Cmd): def _check_ethtool_setting_support(self, idx, connection): """Check if SettingEthtool support is needed and available - If any feature is specified, the SettingEthtool setting needs to be - available. Also NM needs to know about each specified setting. Do not - check if NM knows about any defaults. + If any ethtool setting is specified, the SettingEthtool + setting needs to be available. Also NM needs to know about each + specified setting. Do not check if NM knows about any defaults """ NM = Util.NM() @@ -2019,21 +2046,31 @@ class Cmd_nm(Cmd): if "ethtool" not in connection: return - ethtool_features = connection["ethtool"]["features"] - specified_features = dict( - [(k, v) for k, v in ethtool_features.items() if v is not None] - ) + ethtool_dict = { + "features": nm_provider.get_nm_ethtool_feature, + "coalesce": nm_provider.get_nm_ethtool_coalesce, + } - if specified_features and not hasattr(NM, "SettingEthtool"): - self.log_fatal(idx, "ethtool.features specified but not supported by NM") + for ethtool_key, nm_get_name_fcnt in ethtool_dict.items(): + ethtool_settings = connection["ethtool"][ethtool_key] + specified = dict( + [(k, v) for k, v in ethtool_settings.items() if v is not None] + ) - for feature, setting in specified_features.items(): - nm_feature = nm_provider.get_nm_ethtool_feature(feature) - if not nm_feature: + if specified and not hasattr(NM, "SettingEthtool"): self.log_fatal( - idx, "ethtool feature %s specified but not support by NM" % feature + idx, "ethtool.%s specified but not supported by NM", specified ) + for option, _ in specified.items(): + nm_name = nm_get_name_fcnt(option) + if not nm_name: + self.log_fatal( + idx, + "ethtool %s setting %s specified " + "but not supported by NM" % (ethtool_key, option), + ) + def run_action_absent(self, idx): seen = set() name = self.connections[idx]["name"] diff --git a/module_utils/network_lsr/argument_validator.py b/module_utils/network_lsr/argument_validator.py index 26ae253..ac80fe6 100644 --- a/module_utils/network_lsr/argument_validator.py +++ b/module_utils/network_lsr/argument_validator.py @@ -9,6 +9,8 @@ import socket from ansible.module_utils.network_lsr import MyError # noqa:E501 from ansible.module_utils.network_lsr.utils import Util # noqa:E501 +UINT32_MAX = 0xFFFFFFFF + class ArgUtil: @staticmethod @@ -584,7 +586,10 @@ class ArgValidator_DictEthtool(ArgValidatorDict): ArgValidatorDict.__init__( self, name="ethtool", - nested=[ArgValidator_DictEthtoolFeatures()], + nested=[ + ArgValidator_DictEthtoolFeatures(), + ArgValidator_DictEthtoolCoalesce(), + ], default_value=ArgValidator.MISSING, ) @@ -800,6 +805,87 @@ class ArgValidator_DictEthtoolFeatures(ArgValidatorDict): ) +class ArgValidator_DictEthtoolCoalesce(ArgValidatorDict): + def __init__(self): + ArgValidatorDict.__init__( + self, + name="coalesce", + nested=[ + ArgValidatorBool("adaptive_rx", default_value=None), + ArgValidatorBool("adaptive_tx", default_value=None), + ArgValidatorNum( + "pkt_rate_high", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "pkt_rate_low", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_frames", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_frames_high", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_frames_irq", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_frames_low", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_usecs", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_usecs_high", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_usecs_irq", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "rx_usecs_low", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "sample_interval", + val_min=0, + val_max=UINT32_MAX, + default_value=None, + ), + ArgValidatorNum( + "stats_block_usecs", + val_min=0, + val_max=UINT32_MAX, + default_value=None, + ), + ArgValidatorNum( + "tx_frames", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_frames_high", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_frames_irq", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_frames_low", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_usecs", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_usecs_high", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_usecs_irq", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ArgValidatorNum( + "tx_usecs_low", val_min=0, val_max=UINT32_MAX, default_value=None + ), + ], + ) + self.default_value = dict( + [(k, v.default_value) for k, v in self.nested.items()] + ) + + class ArgValidator_DictBond(ArgValidatorDict): VALID_MODES = [ diff --git a/module_utils/network_lsr/nm_provider.py b/module_utils/network_lsr/nm_provider.py index 9407e24..c75242a 100644 --- a/module_utils/network_lsr/nm_provider.py +++ b/module_utils/network_lsr/nm_provider.py @@ -5,6 +5,7 @@ from ansible.module_utils.network_lsr.utils import Util # noqa:E501 ETHTOOL_FEATURE_PREFIX = "ETHTOOL_OPTNAME_FEATURE_" +ETHTOOL_COALESCE_PREFIX = "ETHTOOL_OPTNAME_COALESCE_" def get_nm_ethtool_feature(name): @@ -21,3 +22,19 @@ def get_nm_ethtool_feature(name): feature = getattr(Util.NM(), name, None) return feature + + +def get_nm_ethtool_coalesce(name): + """ + Translate ethtool coalesce into Network Manager name + + :param name: Name of the coalesce + :type name: str + :returns: Name of the setting to be used by `NM.SettingEthtool.set_coalesce()` + :rtype: str + """ + + name = ETHTOOL_COALESCE_PREFIX + name.upper() + + coalesce = getattr(Util.NM(), name, None) + return coalesce diff --git a/tests/ensure_provider_tests.py b/tests/ensure_provider_tests.py index e4aee5f..2746bb8 100755 --- a/tests/ensure_provider_tests.py +++ b/tests/ensure_provider_tests.py @@ -68,6 +68,12 @@ NM_ONLY_TESTS = { MINIMUM_VERSION: "'1.20.0'", "comment": "# NetworKmanager 1.20.0 added support for forgetting profiles", }, + "playbooks/tests_ethtool_coalesce.yml": { + MINIMUM_VERSION: "'1.25.1'", + "comment": "# NetworkManager 1.25.1 introduced ethtool coalesce support", + }, + "playbooks/tests_802_1x_updated.yml": {}, + "playbooks/tests_802_1x.yml": {}, "playbooks/tests_reapply.yml": {}, # team interface is not supported on Fedora "playbooks/tests_team.yml": { diff --git a/tests/playbooks/manual_test_ethtool_coalesce.yml b/tests/playbooks/manual_test_ethtool_coalesce.yml new file mode 100644 index 0000000..8b2c456 --- /dev/null +++ b/tests/playbooks/manual_test_ethtool_coalesce.yml @@ -0,0 +1,115 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- hosts: all + vars: + interface: "{{ network_interface_name1 }}" + type: "{{ network_interface_type1 }}" + tasks: + - name: "INIT: Ethtool coalesce tests" + debug: + msg: "##################################################" + - include_tasks: tasks/show_interfaces.yml + - include_tasks: tasks/manage_test_interface.yml + vars: + state: present + - include_tasks: tasks/assert_device_present.yml + - name: Install ethtool (test dependency) + package: + name: ethtool + state: present + - block: + - name: >- + TEST: I can create a profile without changing the ethtool coalesce. + debug: + msg: "##################################################" + - name: Get current device coalesce + command: "ethtool --show-coalesce {{ interface }}" + register: original_ethtool_coalesce + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + state: up + type: ethernet + ip: + dhcp4: "no" + auto6: "no" + - name: Get current device coalesce + command: "ethtool --show-coalesce {{ interface }}" + register: ethtool_coalesce + - name: "ASSERT: The profile does not change the ethtool coalesce" + assert: + that: + - original_ethtool_coalesce.stdout == ethtool_coalesce.stdout + - name: >- + TEST: I can set rx-frames and adaptive-tx. + debug: + msg: "##################################################" + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + state: up + type: ethernet + ip: + dhcp4: "no" + auto6: "no" + ethtool: + coalesce: + rx_frames: 1 + tx_frames: 1 + - name: Get current device coalesce + command: "ethtool --show-coalesce {{ interface }}" + register: ethtool_coalesce + - name: + debug: + var: ethtool_coalesce.stdout_lines + - name: Assert device coalesce + assert: + that: + - >- + 'rx-frames: 1' in + ethtool_coalesce.stdout_lines + - >- + 'tx-frames: 1' in + ethtool_coalesce.stdout_lines + - name: "TEST: I can reset coalesce to their original value." + debug: + msg: "##################################################" + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + state: up + type: ethernet + ip: + dhcp4: "no" + auto6: "no" + - name: Get current device coalesce + command: "ethtool --show-coalesce {{ interface }}" + register: ethtool_coalesce + # Resetting the ethtools only works with NetworkManager + - name: "ASSERT: The profile does not change the ethtool coalesce" + assert: + that: + - original_ethtool_coalesce.stdout == ethtool_coalesce.stdout + when: + network_provider == 'nm' + always: + - block: + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + persistent_state: absent + state: down + ignore_errors: true + - include_tasks: tasks/manage_test_interface.yml + vars: + state: absent + tags: + - "tests::cleanup" diff --git a/tests/playbooks/tests_ethtool_coalesce.yml b/tests/playbooks/tests_ethtool_coalesce.yml new file mode 100644 index 0000000..62ff0e1 --- /dev/null +++ b/tests/playbooks/tests_ethtool_coalesce.yml @@ -0,0 +1,102 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- hosts: all + vars: + interface: testnic1 + type: veth + tasks: + - debug: + msg: "this is: playbooks/tests_ethtool_.coalesceyml" + tags: + - always + + - name: "INIT: Ethtool coalesce tests" + debug: + msg: "##################################################" + - include_tasks: tasks/show_interfaces.yml + - include_tasks: tasks/manage_test_interface.yml + vars: + state: present + - include_tasks: tasks/assert_device_present.yml + - name: Install ethtool (test dependency) + package: + name: ethtool + state: present + + - block: + - name: >- + TEST: I can create a profile without any coalescing option. + debug: + msg: "##################################################" + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + type: ethernet + autoconnect: no + ip: + dhcp4: no + auto6: no + - name: Get profile's coalescing options + command: nmcli -g ethtool.coalesce-rx-frames c show {{ interface }} + register: no_coalesce + - name: "ASSERT: The profile does not contain coalescing options" + assert: + that: no_coalesce.stdout == "" + - name: >- + TEST: I can set rx-frames. + debug: + msg: "##################################################" + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + type: ethernet + autoconnect: no + ip: + dhcp4: no + auto6: no + ethtool: + coalesce: + rx_frames: 128 + - name: Get profile's coalescing options + command: nmcli -g ethtool.coalesce-rx-frames c show {{ interface }} + register: with_coalesce + - name: Assert coalesce options set in profile + assert: + that: with_coalesce.stdout == '128' + - name: "TEST: I can clear coalescing options" + debug: + msg: "##################################################" + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + type: ethernet + autoconnect: no + ip: + dhcp4: no + auto6: no + - name: Get profile's coalescing options + command: nmcli -g ethtool.coalesce-rx-frames c show {{ interface }} + register: profile + - name: "ASSERT: The profile does reset coalescing options" + assert: + that: no_coalesce.stdout == "" + always: + - block: + - import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + persistent_state: absent + ignore_errors: true + - include_tasks: tasks/manage_test_interface.yml + vars: + state: absent + tags: + - "tests::cleanup" diff --git a/tests/tests_ethtool_coalesce_nm.yml b/tests/tests_ethtool_coalesce_nm.yml new file mode 100644 index 0000000..f38294a --- /dev/null +++ b/tests/tests_ethtool_coalesce_nm.yml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: BSD-3-Clause +# This file was generated by ensure_provider_tests.py +--- +# set network provider and gather facts +- hosts: all + name: Run playbook 'playbooks/tests_ethtool_coalesce.yml' with nm as provider + tasks: + - name: Set network provider to 'nm' + set_fact: + network_provider: nm + tags: + - always + + - block: + - name: Install NetworkManager + package: + name: NetworkManager + state: present + - name: Get NetworkManager version + command: rpm -q --qf "%{version}" NetworkManager + args: + warn: false + register: NetworkManager_version + when: true + when: + - ansible_distribution_major_version != '6' + tags: + - always + + +# workaround for: https://github.com/ansible/ansible/issues/27973 +# There is no way in Ansible to abort a playbook hosts with specific OS +# releases Therefore we include the playbook with the tests only if the hosts +# would support it. +# The test requires or should run with NetworkManager, therefore it cannot run +# on RHEL/CentOS 6 +- import_playbook: playbooks/tests_ethtool_coalesce.yml + when: + - ansible_distribution_major_version != '6' + + - NetworkManager_version.stdout is version('1.25.1', '>=') diff --git a/tests/unit/test_network_connections.py b/tests/unit/test_network_connections.py old mode 100755 new mode 100644 index a71fab0..698a85a --- a/tests/unit/test_network_connections.py +++ b/tests/unit/test_network_connections.py @@ -124,7 +124,36 @@ ETHTOOL_FEATURES_DEFAULTS = { "txvlan": None, } -ETHTOOL_DEFAULTS = {"features": ETHTOOL_FEATURES_DEFAULTS} + +ETHTOOL_COALESCE_DEFAULTS = { + "adaptive_rx": None, + "adaptive_tx": None, + "pkt_rate_high": None, + "pkt_rate_low": None, + "rx_frames": None, + "rx_frames_high": None, + "rx_frames_irq": None, + "rx_frames_low": None, + "rx_usecs": None, + "rx_usecs_high": None, + "rx_usecs_irq": None, + "rx_usecs_low": None, + "sample_interval": None, + "stats_block_usecs": None, + "tx_frames": None, + "tx_frames_high": None, + "tx_frames_irq": None, + "tx_frames_low": None, + "tx_usecs": None, + "tx_usecs_high": None, + "tx_usecs_irq": None, + "tx_usecs_low": None, +} + +ETHTOOL_DEFAULTS = { + "features": ETHTOOL_FEATURES_DEFAULTS, + "coalesce": ETHTOOL_COALESCE_DEFAULTS, +} ETHERNET_DEFAULTS = {"autoneg": None, "duplex": None, "speed": 0} @@ -2912,18 +2941,20 @@ class TestValidator(unittest.TestCase): }, ) - def _test_ethtool_changes(self, input_features, expected_features): + def _test_ethtool_changes(self, input_ethtool, expected_ethtool): """ When passing a dictionary 'input_features' with each feature and their value to change, and a dictionary 'expected_features' with the expected result in the configuration, the expected and resulting connection are created and validated. """ - custom_ethtool_features = copy.deepcopy(ETHTOOL_FEATURES_DEFAULTS) - custom_ethtool_features.update(expected_features) - expected_ethtool = {"features": custom_ethtool_features} + expected_ethtool_full = copy.deepcopy(ETHTOOL_DEFAULTS) + for key in list(expected_ethtool_full): + if key in expected_ethtool: + expected_ethtool_full[key].update(expected_ethtool[key]) + input_connection = { - "ethtool": {"features": input_features}, + "ethtool": input_ethtool, "name": "5", "persistent_state": "present", "type": "ethernet", @@ -2931,7 +2962,7 @@ class TestValidator(unittest.TestCase): expected_connection = { "actions": ["present"], - "ethtool": expected_ethtool, + "ethtool": expected_ethtool_full, "interface_name": "5", "persistent_state": "present", "state": None, @@ -2944,17 +2975,17 @@ class TestValidator(unittest.TestCase): When passing the name of an non-deprecated ethtool feature, their current version is updated. """ - input_features = {"tx_tcp_segmentation": "yes"} - expected_feature_changes = {"tx_tcp_segmentation": True} - self._test_ethtool_changes(input_features, expected_feature_changes) + input_ethtool = {"features": {"tx_tcp_segmentation": "yes"}} + expected_ethtool = {"features": {"tx_tcp_segmentation": True}} + self._test_ethtool_changes(input_ethtool, expected_ethtool) def test_set_deprecated_ethtool_feature(self): """ When passing a deprecated name, their current version is updated. """ - input_features = {"tx-tcp-segmentation": "yes"} - expected_feature_changes = {"tx_tcp_segmentation": True} - self._test_ethtool_changes(input_features, expected_feature_changes) + input_ethtool = {"features": {"esp-hw-offload": "yes"}} + expected_ethtool = {"features": {"esp_hw_offload": True}} + self._test_ethtool_changes(input_ethtool, expected_ethtool) def test_invalid_ethtool_settings(self): """ diff --git a/tests/unit/test_nm_provider.py b/tests/unit/test_nm_provider.py index c8fd4eb..ed8563f 100644 --- a/tests/unit/test_nm_provider.py +++ b/tests/unit/test_nm_provider.py @@ -29,3 +29,10 @@ def test_get_nm_ethtool_feature(): with mock.patch.object(nm_provider.Util, "NM") as nm_mock: nm_feature = nm_provider.get_nm_ethtool_feature("esp_hw_offload") assert nm_feature == nm_mock.return_value.ETHTOOL_OPTNAME_FEATURE_ESP_HW_OFFLOAD + + +def test_get_nm_ethtool_coalesce(): + """ Test get_nm_ethtool_coalesce() """ + with mock.patch.object(nm_provider.Util, "NM") as nm_mock: + nm_feature = nm_provider.get_nm_ethtool_coalesce("rx_frames") + assert nm_feature == nm_mock.return_value.ETHTOOL_OPTNAME_COALESCE_RX_FRAMES