nm ethtool: Use only original desire to update settings

We only create/update ethtool settings base on user's original desire
state.

Signed-off-by: Gris Ge <fge@redhat.com>

libnmstate/ifaces/__init__.py

@@ -17,12 +17,14 @@
 # along with this program. If not, see <https://www.gnu.org/licenses/>.
 #
 
-from .ifaces import Ifaces
 from .base_iface import BaseIface
-from .linux_bridge_port_vlan import NmstateLinuxBridgePortVlan
+from .ethtool import IfaceEthtool
+from .ifaces import Ifaces
 from .linux_bridge_port_vlan import KernelBridgePortVlans
+from .linux_bridge_port_vlan import NmstateLinuxBridgePortVlan
 
-Ifaces
 BaseIface
-NmstateLinuxBridgePortVlan
+IfaceEthtool
+Ifaces
 KernelBridgePortVlans
+NmstateLinuxBridgePortVlan

libnmstate/nm/connection.py

@@ -22,6 +22,8 @@
 import uuid
 
 from libnmstate.error import NmstatePluginError
+from libnmstate.ifaces import IfaceEthtool
+from libnmstate.schema import Ethtool
 from libnmstate.schema import Interface
 from libnmstate.schema import InterfaceType
 from libnmstate.schema import LinuxBridge as LB
@@ -234,8 +236,15 @@ def create_new_nm_simple_conn(iface, nm_profile):
     if iface.ieee_802_1x_conf:
         settings.append(create_802_1x_setting(iface.ieee_802_1x_conf))
 
-    if iface.ethtool:
-        setting = create_ethtool_setting(iface.ethtool, nm_profile)
+    if Ethtool.CONFIG_SUBTREE in iface.original_dict and iface.is_desired:
+        iface_ethtool = IfaceEthtool(
+            iface.original_dict[Ethtool.CONFIG_SUBTREE]
+        )
+        iface_ethtool.canonicalize(iface.original_dict[Ethtool.CONFIG_SUBTREE])
+        setting = create_ethtool_setting(
+            iface_ethtool,
+            nm_profile,
+        )
         if setting:
             settings.append(setting)
 

libnmstate/nm/ethtool.py

@@ -37,26 +37,14 @@ def create_ethtool_setting(iface_ethtool, base_con_profile):
         nm_setting = NM.SettingEthtool.new()
 
     if iface_ethtool.pause and hasattr(NM, "ETHTOOL_OPTNAME_PAUSE_AUTONEG"):
-        if iface_ethtool.pause.autoneg is not None:
-            nm_setting.option_set(
-                # pylint: disable=no-member
-                NM.ETHTOOL_OPTNAME_PAUSE_AUTONEG,
-                # pylint: enable=no-member
-                GLib.Variant.new_boolean(iface_ethtool.pause.autoneg),
-            )
-        if iface_ethtool.pause.rx is not None:
-            nm_setting.option_set(
-                # pylint: disable=no-member
-                NM.ETHTOOL_OPTNAME_PAUSE_RX,
-                # pylint: enable=no-member
-                GLib.Variant.new_boolean(iface_ethtool.pause.rx),
-            )
-        if iface_ethtool.pause.tx is not None:
-            nm_setting.option_set(
-                # pylint: disable=no-member
-                NM.ETHTOOL_OPTNAME_PAUSE_TX,
-                # pylint: enable=no-member
-                GLib.Variant.new_boolean(iface_ethtool.pause.tx),
+        if iface_ethtool.pause.autoneg is True:
+            nm_set_pause(nm_setting, True, None, None)
+        elif iface_ethtool.pause.autoneg is False:
+            nm_set_pause(
+                nm_setting,
+                False,
+                iface_ethtool.pause.rx,
+                iface_ethtool.pause.tx,
             )
 
     if iface_ethtool.feature:
@@ -94,3 +82,22 @@ def nm_set_feature(nm_setting, kernel_feature_name, value):
             f"Ethtool feature {kernel_feature_name} is invalid "
             "or not supported by current NetworkManager"
         )
+
+
+def nm_set_pause(nm_setting, autoneg, rx, tx):
+    rx_value = None if rx is None else GLib.Variant.new_boolean(rx)
+    tx_value = None if tx is None else GLib.Variant.new_boolean(tx)
+    # pylint: disable=no-member
+    nm_setting.option_set(
+        NM.ETHTOOL_OPTNAME_PAUSE_AUTONEG,
+        GLib.Variant.new_boolean(autoneg),
+    )
+    nm_setting.option_set(
+        NM.ETHTOOL_OPTNAME_PAUSE_RX,
+        rx_value,
+    )
+    nm_setting.option_set(
+        NM.ETHTOOL_OPTNAME_PAUSE_TX,
+        tx_value,
+    )
+    # pylint: enable=no-member

tests/lib/nm/ethtool_test.py

@@ -32,7 +32,7 @@ def nm_mock():
         yield m
 
 
-def test_create_setting_pause(nm_mock):
+def test_create_setting_pause_autoneg(nm_mock):
     iface_ethtool = IfaceEthtool(
         {
             Ethtool.Pause.CONFIG_SUBTREE: {
@@ -52,6 +52,33 @@ def test_create_setting_pause(nm_mock):
                 nm_mock.ETHTOOL_OPTNAME_PAUSE_AUTONEG,
                 GLib.Variant.new_boolean(True),
             ),
+            mock.call(nm_mock.ETHTOOL_OPTNAME_PAUSE_RX, None),
+            mock.call(nm_mock.ETHTOOL_OPTNAME_PAUSE_TX, None),
+        ],
+        any_order=False,
+    )
+
+
+def test_create_setting_pause_autoneg_off(nm_mock):
+    iface_ethtool = IfaceEthtool(
+        {
+            Ethtool.Pause.CONFIG_SUBTREE: {
+                Ethtool.Pause.AUTO_NEGOTIATION: False,
+                Ethtool.Pause.RX: True,
+                Ethtool.Pause.TX: True,
+            }
+        }
+    )
+    nm_ethtool_setting_mock = nm_mock.SettingEthtool.new.return_value
+
+    nm_ethtool.create_ethtool_setting(iface_ethtool, base_con_profile=None)
+
+    nm_ethtool_setting_mock.option_set.assert_has_calls(
+        [
+            mock.call(
+                nm_mock.ETHTOOL_OPTNAME_PAUSE_AUTONEG,
+                GLib.Variant.new_boolean(False),
+            ),
             mock.call(
                 nm_mock.ETHTOOL_OPTNAME_PAUSE_RX,
                 GLib.Variant.new_boolean(True),