Blame SOURCES/1011-don-t-touch-device-tc-config-by-default-rh1928078.patch

fd19ff
From 78e4c3d3d06b411a1bc9e60ee8bf0c460d4453b2 Mon Sep 17 00:00:00 2001
fd19ff
From: Beniamino Galvani <bgalvani@redhat.com>
fd19ff
Date: Tue, 25 May 2021 16:58:28 +0200
fd19ff
Subject: [PATCH 1/2] core,libnm: don't touch device TC configuration by
fd19ff
 default
fd19ff
fd19ff
NetworkManager supports a very limited set of qdiscs. If users want to
fd19ff
configure a unsupported qdisc, they need to do it outside of
fd19ff
NetworkManager using tc.
fd19ff
fd19ff
The problem is that NM also removes all qdiscs and filters during
fd19ff
activation if the connection doesn't contain a TC setting. Therefore,
fd19ff
setting TC configuration outside of NM is hard because users need to
fd19ff
do it *after* the connection is up (for example through a dispatcher
fd19ff
script).
fd19ff
fd19ff
Let NM consider the presence (or absence) of a TC setting in the
fd19ff
connection to determine whether NM should configure (or not) qdiscs
fd19ff
and filters on the interface. We already do something similar for
fd19ff
SR-IOV configuration.
fd19ff
fd19ff
Since new connections don't have the TC setting, the new behavior
fd19ff
(ignore existing configuration) will be the default. The impact of
fd19ff
this change in different scenarios is:
fd19ff
fd19ff
 - the user previously configured TC settings via NM. This continues
fd19ff
   to work as before;
fd19ff
fd19ff
 - the user didn't set any qdiscs or filters in the connection, and
fd19ff
   expected NM to clear them from the interface during activation.
fd19ff
   Here there is a change in behavior, but it seems unlikely that
fd19ff
   anybody relied on the old one;
fd19ff
fd19ff
 - the user didn't care about qdiscs and filters; NM removed all
fd19ff
   qdiscs upon activation, and so the default qdisc from kernel was
fd19ff
   used. After this change, NM will not touch qdiscs and the default
fd19ff
   qdisc will be used, as before;
fd19ff
fd19ff
 - the user set a different qdisc via tc and NM cleared it during
fd19ff
   activation. Now this will work as expected.
fd19ff
fd19ff
So, the new default behavior seems better than the previous one.
fd19ff
fd19ff
https://bugzilla.redhat.com/show_bug.cgi?id=1928078
fd19ff
(cherry picked from commit a48edd0410c878d65fc5adcd5192b116ab6f8afc)
fd19ff
(cherry picked from commit 2a8181bcd78d055b7cb9e6c0e026bc3b08231b5a)
fd19ff
---
fd19ff
 .../generate-docs-nm-settings-nmcli.xml.in    |  4 +--
fd19ff
 clients/common/settings-docs.h.in             |  4 +--
fd19ff
 libnm-core/nm-setting-tc-config.c             | 16 ++++++++++++
fd19ff
 src/core/devices/nm-device.c                  | 26 +++++++++----------
fd19ff
 4 files changed, 33 insertions(+), 17 deletions(-)
fd19ff
fd19ff
diff --git a/clients/cli/generate-docs-nm-settings-nmcli.xml.in b/clients/cli/generate-docs-nm-settings-nmcli.xml.in
fd19ff
index 1044ae0d38..0a75a0e681 100644
fd19ff
--- a/clients/cli/generate-docs-nm-settings-nmcli.xml.in
fd19ff
+++ b/clients/cli/generate-docs-nm-settings-nmcli.xml.in
fd19ff
@@ -914,9 +914,9 @@
fd19ff
     </setting>
fd19ff
     <setting name="tc" >
fd19ff
         
fd19ff
-                  description="Array of TC queueing disciplines." />
fd19ff
+                  description="Array of TC queueing disciplines. When the "tc" setting is present, qdiscs from this property are applied upon activation. If the property is empty, all qdiscs are removed and the device will only have the default qdisc assigned by kernel according to the "net.core.default_qdisc" sysctl. If the "tc" setting is not present, NetworkManager doesn't touch the qdiscs present on the interface." />
fd19ff
         
fd19ff
-                  description="Array of TC traffic filters." />
fd19ff
+                  description="Array of TC traffic filters. When the "tc" setting is present, filters from this property are applied upon activation. If the property is empty, NetworkManager removes all the filters. If the "tc" setting is not present, NetworkManager doesn't touch the filters present on the interface." />
fd19ff
     </setting>
fd19ff
     <setting name="team" >
fd19ff
         
fd19ff
diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in
fd19ff
index 2c275a99c8..7c3ff25fe3 100644
fd19ff
--- a/clients/common/settings-docs.h.in
fd19ff
+++ b/clients/common/settings-docs.h.in
fd19ff
@@ -342,8 +342,8 @@
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_SRIOV_AUTOPROBE_DRIVERS N_("Whether to autoprobe virtual functions by a compatible driver. If set to NM_TERNARY_TRUE (1), the kernel will try to bind VFs to a compatible driver and if this succeeds a new network interface will be instantiated for each VF. If set to NM_TERNARY_FALSE (0), VFs will not be claimed and no network interfaces will be created for them. When set to NM_TERNARY_DEFAULT (-1), the global default is used; in case the global default is unspecified it is assumed to be NM_TERNARY_TRUE (1).")
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_SRIOV_TOTAL_VFS N_("The total number of virtual functions to create. Note that when the sriov setting is present NetworkManager enforces the number of virtual functions on the interface (also when it is zero) during activation and resets it upon deactivation. To prevent any changes to SR-IOV parameters don't add a sriov setting to the connection.")
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_SRIOV_VFS N_("Array of virtual function descriptors. Each VF descriptor is a dictionary mapping attribute names to GVariant values. The 'index' entry is mandatory for each VF. When represented as string a VF is in the form: \"INDEX [ATTR=VALUE[ ATTR=VALUE]...]\". for example: \"2 mac=00:11:22:33:44:55 spoof-check=true\". Multiple VFs can be specified using a comma as separator. Currently, the following attributes are supported: mac, spoof-check, trust, min-tx-rate, max-tx-rate, vlans. The \"vlans\" attribute is represented as a semicolon-separated list of VLAN descriptors, where each descriptor has the form \"ID[.PRIORITY[.PROTO]]\". PROTO can be either 'q' for 802.1Q (the default) or 'ad' for 802.1ad.")
fd19ff
-#define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_QDISCS N_("Array of TC queueing disciplines.")
fd19ff
-#define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_TFILTERS N_("Array of TC traffic filters.")
fd19ff
+#define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_QDISCS N_("Array of TC queueing disciplines. When the \"tc\" setting is present, qdiscs from this property are applied upon activation. If the property is empty, all qdiscs are removed and the device will only have the default qdisc assigned by kernel according to the \"net.core.default_qdisc\" sysctl. If the \"tc\" setting is not present, NetworkManager doesn't touch the qdiscs present on the interface.")
fd19ff
+#define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_TFILTERS N_("Array of TC traffic filters. When the \"tc\" setting is present, filters from this property are applied upon activation. If the property is empty, NetworkManager removes all the filters. If the \"tc\" setting is not present, NetworkManager doesn't touch the filters present on the interface.")
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_TEAM_CONFIG N_("The JSON configuration for the team network interface.  The property should contain raw JSON configuration data suitable for teamd, because the value is passed directly to teamd. If not specified, the default configuration is used.  See man teamd.conf for the format details.")
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_TEAM_LINK_WATCHERS N_("Link watchers configuration for the connection: each link watcher is defined by a dictionary, whose keys depend upon the selected link watcher. Available link watchers are 'ethtool', 'nsna_ping' and 'arp_ping' and it is specified in the dictionary with the key 'name'. Available keys are:   ethtool: 'delay-up', 'delay-down', 'init-wait'; nsna_ping: 'init-wait', 'interval', 'missed-max', 'target-host'; arp_ping: all the ones in nsna_ping and 'source-host', 'validate-active', 'validate-inactive', 'send-always'. See teamd.conf man for more details.")
fd19ff
 #define DESCRIBE_DOC_NM_SETTING_TEAM_MCAST_REJOIN_COUNT N_("Corresponds to the teamd mcast_rejoin.count.")
fd19ff
diff --git a/libnm-core/nm-setting-tc-config.c b/libnm-core/nm-setting-tc-config.c
fd19ff
index 33df6d34e5..2fad98b1e8 100644
fd19ff
--- a/libnm-core/nm-setting-tc-config.c
fd19ff
+++ b/libnm-core/nm-setting-tc-config.c
fd19ff
@@ -1810,6 +1810,15 @@ nm_setting_tc_config_class_init(NMSettingTCConfigClass *klass)
fd19ff
      * NMSettingTCConfig:qdiscs: (type GPtrArray(NMTCQdisc))
fd19ff
      *
fd19ff
      * Array of TC queueing disciplines.
fd19ff
+     *
fd19ff
+     * When the #NMSettingTCConfig setting is present, qdiscs from this
fd19ff
+     * property are applied upon activation. If the property is empty,
fd19ff
+     * all qdiscs are removed and the device will only
fd19ff
+     * have the default qdisc assigned by kernel according to the
fd19ff
+     * "net.core.default_qdisc" sysctl.
fd19ff
+     *
fd19ff
+     * If the #NMSettingTCConfig setting is not present, NetworkManager
fd19ff
+     * doesn't touch the qdiscs present on the interface.
fd19ff
      **/
fd19ff
     /* ---ifcfg-rh---
fd19ff
      * property: qdiscs
fd19ff
@@ -1834,6 +1843,13 @@ nm_setting_tc_config_class_init(NMSettingTCConfigClass *klass)
fd19ff
      * NMSettingTCConfig:tfilters: (type GPtrArray(NMTCTfilter))
fd19ff
      *
fd19ff
      * Array of TC traffic filters.
fd19ff
+     *
fd19ff
+     * When the #NMSettingTCConfig setting is present, filters from this
fd19ff
+     * property are applied upon activation. If the property is empty,
fd19ff
+     * NetworkManager removes all the filters.
fd19ff
+     *
fd19ff
+     * If the #NMSettingTCConfig setting is not present, NetworkManager
fd19ff
+     * doesn't touch the filters present on the interface.
fd19ff
      **/
fd19ff
     /* ---ifcfg-rh---
fd19ff
      * property: qdiscs
fd19ff
diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
fd19ff
index 7c2a6d3250..901d309c59 100644
fd19ff
--- a/src/core/devices/nm-device.c
fd19ff
+++ b/src/core/devices/nm-device.c
fd19ff
@@ -8361,26 +8361,23 @@ _routing_rules_sync(NMDevice *self, NMTernary set_mode)
fd19ff
 static gboolean
fd19ff
 tc_commit(NMDevice *self)
fd19ff
 {
fd19ff
-    NMConnection *    connection          = NULL;
fd19ff
     gs_unref_ptrarray GPtrArray *qdiscs   = NULL;
fd19ff
     gs_unref_ptrarray GPtrArray *tfilters = NULL;
fd19ff
-    NMSettingTCConfig *          s_tc     = NULL;
fd19ff
+    NMSettingTCConfig *          s_tc;
fd19ff
     NMPlatform *                 platform;
fd19ff
     int                          ip_ifindex;
fd19ff
 
fd19ff
-    platform   = nm_device_get_platform(self);
fd19ff
-    connection = nm_device_get_applied_connection(self);
fd19ff
-    if (connection)
fd19ff
-        s_tc = nm_connection_get_setting_tc_config(connection);
fd19ff
+    s_tc = nm_device_get_applied_setting(self, NM_TYPE_SETTING_TC_CONFIG);
fd19ff
+    if (!s_tc)
fd19ff
+        return TRUE;
fd19ff
 
fd19ff
     ip_ifindex = nm_device_get_ip_ifindex(self);
fd19ff
     if (!ip_ifindex)
fd19ff
-        return s_tc == NULL;
fd19ff
+        return FALSE;
fd19ff
 
fd19ff
-    if (s_tc) {
fd19ff
-        qdiscs   = nm_utils_qdiscs_from_tc_setting(platform, s_tc, ip_ifindex);
fd19ff
-        tfilters = nm_utils_tfilters_from_tc_setting(platform, s_tc, ip_ifindex);
fd19ff
-    }
fd19ff
+    platform = nm_device_get_platform(self);
fd19ff
+    qdiscs   = nm_utils_qdiscs_from_tc_setting(platform, s_tc, ip_ifindex);
fd19ff
+    tfilters = nm_utils_tfilters_from_tc_setting(platform, s_tc, ip_ifindex);
fd19ff
 
fd19ff
     if (!nm_platform_qdisc_sync(platform, ip_ifindex, qdiscs))
fd19ff
         return FALSE;
fd19ff
@@ -15900,9 +15897,12 @@ nm_device_cleanup(NMDevice *self, NMDeviceStateReason reason, CleanupType cleanu
fd19ff
 
fd19ff
             nm_platform_ip_route_flush(platform, AF_UNSPEC, ifindex);
fd19ff
             nm_platform_ip_address_flush(platform, AF_UNSPEC, ifindex);
fd19ff
-            nm_platform_tfilter_sync(platform, ifindex, NULL);
fd19ff
-            nm_platform_qdisc_sync(platform, ifindex, NULL);
fd19ff
             set_ipv6_token(self, iid, "::");
fd19ff
+
fd19ff
+            if (nm_device_get_applied_setting(self, NM_TYPE_SETTING_TC_CONFIG)) {
fd19ff
+                nm_platform_tfilter_sync(platform, ifindex, NULL);
fd19ff
+                nm_platform_qdisc_sync(platform, ifindex, NULL);
fd19ff
+            }
fd19ff
         }
fd19ff
     }
fd19ff
 
fd19ff
-- 
fd19ff
2.31.1
fd19ff
fd19ff
From 450b6aab2ba4221a08e8ce529ec7c16dfb97458b Mon Sep 17 00:00:00 2001
fd19ff
From: Beniamino Galvani <bgalvani@redhat.com>
fd19ff
Date: Tue, 25 May 2021 18:00:27 +0200
fd19ff
Subject: [PATCH 2/2] ifcfg-rh: preserve an empty tc configuration
fd19ff
fd19ff
If the TC setting contains no qdiscs and filters, it is lost after a
fd19ff
write-read cycle. Fix this by adding a new property to indicate the
fd19ff
presence of the (empty) setting.
fd19ff
fd19ff
(cherry picked from commit 6a88d4e55cf031da2b5a8458d21487a011357da4)
fd19ff
(cherry picked from commit acf0c4df2b0fb0dc332aa929131953390998828f)
fd19ff
---
fd19ff
 Makefile.am                                   |  1 +
fd19ff
 libnm-core/nm-setting-tc-config.c             | 14 +++-
fd19ff
 .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c    |  3 +-
fd19ff
 .../plugins/ifcfg-rh/nms-ifcfg-rh-utils.c     |  1 +
fd19ff
 .../plugins/ifcfg-rh/nms-ifcfg-rh-utils.h     |  2 +-
fd19ff
 .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c    | 38 +++++----
fd19ff
 .../ifcfg-test-tc-write-empty.cexpected       | 15 ++++
fd19ff
 .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c    | 80 +++++++++++++++++++
fd19ff
 8 files changed, 128 insertions(+), 26 deletions(-)
fd19ff
 create mode 100644 src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected
fd19ff
fd19ff
diff --git a/Makefile.am b/Makefile.am
fd19ff
index 9279672c1f..c8e417729b 100644
fd19ff
--- a/Makefile.am
fd19ff
+++ b/Makefile.am
fd19ff
@@ -3300,6 +3300,7 @@ EXTRA_DIST += \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy.cexpected \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write.cexpected \
fd19ff
+	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1 \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2 \
fd19ff
 	src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid \
fd19ff
diff --git a/libnm-core/nm-setting-tc-config.c b/libnm-core/nm-setting-tc-config.c
fd19ff
index 2fad98b1e8..31e829c1d2 100644
fd19ff
--- a/libnm-core/nm-setting-tc-config.c
fd19ff
+++ b/libnm-core/nm-setting-tc-config.c
fd19ff
@@ -1822,8 +1822,11 @@ nm_setting_tc_config_class_init(NMSettingTCConfigClass *klass)
fd19ff
      **/
fd19ff
     /* ---ifcfg-rh---
fd19ff
      * property: qdiscs
fd19ff
-     * variable: QDISC1(+), QDISC2(+), ...
fd19ff
-     * description: Queueing disciplines
fd19ff
+     * variable: QDISC1(+), QDISC2(+), ..., TC_COMMIT(+)
fd19ff
+     * description: Queueing disciplines to set on the interface. When no
fd19ff
+     *  QDISC1, QDISC2, ..., FILTER1, FILTER2, ... keys are present,
fd19ff
+     *  NetworkManager doesn't touch qdiscs and filters present on the
fd19ff
+     *  interface, unless TC_COMMIT is set to 'yes'.
fd19ff
      * example: QDISC1=ingress, QDISC2="root handle 1234: fq_codel"
fd19ff
      * ---end---
fd19ff
      */
fd19ff
@@ -1853,8 +1856,11 @@ nm_setting_tc_config_class_init(NMSettingTCConfigClass *klass)
fd19ff
      **/
fd19ff
     /* ---ifcfg-rh---
fd19ff
      * property: qdiscs
fd19ff
-     * variable: FILTER1(+), FILTER2(+), ...
fd19ff
-     * description: Traffic filters
fd19ff
+     * variable: FILTER1(+), FILTER2(+), ..., TC_COMMIT(+)
fd19ff
+     * description: Traffic filters to set on the interface. When no
fd19ff
+     *  QDISC1, QDISC2, ..., FILTER1, FILTER2, ... keys are present,
fd19ff
+     *  NetworkManager doesn't touch qdiscs and filters present on the
fd19ff
+     *  interface, unless TC_COMMIT is set to 'yes'.
fd19ff
      * example: FILTER1="parent ffff: matchall action simple sdata Input", ...
fd19ff
      * ---end---
fd19ff
      */
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
fd19ff
index 209957d9b8..a42c418884 100644
fd19ff
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
fd19ff
@@ -2707,7 +2707,8 @@ make_tc_setting(shvarFile *ifcfg)
fd19ff
     }
fd19ff
 
fd19ff
     if (nm_setting_tc_config_get_num_qdiscs(s_tc) > 0
fd19ff
-        || nm_setting_tc_config_get_num_tfilters(s_tc) > 0)
fd19ff
+        || nm_setting_tc_config_get_num_tfilters(s_tc) > 0
fd19ff
+        || svGetValueBoolean(ifcfg, "TC_COMMIT", FALSE))
fd19ff
         return NM_SETTING(s_tc);
fd19ff
 
fd19ff
     g_object_unref(s_tc);
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
fd19ff
index 8da5de473b..ada1942acb 100644
fd19ff
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
fd19ff
@@ -1026,6 +1026,7 @@ const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[] = {
fd19ff
     _KEY_TYPE("STABLE_ID", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
     _KEY_TYPE("STP", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
     _KEY_TYPE("SUBCHANNELS", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
+    _KEY_TYPE("TC_COMMIT", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
     _KEY_TYPE("TEAM_CONFIG", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
     _KEY_TYPE("TEAM_MASTER", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
     _KEY_TYPE("TEAM_MASTER_UUID", NMS_IFCFG_KEY_TYPE_IS_PLAIN),
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
fd19ff
index 36ec922514..04a1b63d3e 100644
fd19ff
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
fd19ff
@@ -33,7 +33,7 @@ typedef struct {
fd19ff
     NMSIfcfgKeyTypeFlags key_flags;
fd19ff
 } NMSIfcfgKeyTypeInfo;
fd19ff
 
fd19ff
-extern const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[247];
fd19ff
+extern const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[248];
fd19ff
 
fd19ff
 const NMSIfcfgKeyTypeInfo *nms_ifcfg_well_known_key_find_info(const char *key, gssize *out_idx);
fd19ff
 
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
fd19ff
index a968fce0ba..65bacb293a 100644
fd19ff
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
fd19ff
@@ -2511,46 +2511,46 @@ write_sriov_setting(NMConnection *connection, shvarFile *ifcfg)
fd19ff
     }
fd19ff
 }
fd19ff
 
fd19ff
-static gboolean
fd19ff
-write_tc_setting(NMConnection *connection, shvarFile *ifcfg, GError **error)
fd19ff
+static void
fd19ff
+write_tc_setting(NMConnection *connection, shvarFile *ifcfg)
fd19ff
 {
fd19ff
     NMSettingTCConfig *s_tc;
fd19ff
-    guint              i, num, n;
fd19ff
+    guint              num_qdiscs;
fd19ff
+    guint              num_filters;
fd19ff
+    guint              i;
fd19ff
+    guint              n;
fd19ff
     char               tag[64];
fd19ff
 
fd19ff
     s_tc = nm_connection_get_setting_tc_config(connection);
fd19ff
     if (!s_tc)
fd19ff
-        return TRUE;
fd19ff
+        return;
fd19ff
 
fd19ff
-    num = nm_setting_tc_config_get_num_qdiscs(s_tc);
fd19ff
-    for (n = 1, i = 0; i < num; i++) {
fd19ff
+    num_qdiscs = nm_setting_tc_config_get_num_qdiscs(s_tc);
fd19ff
+    for (n = 1, i = 0; i < num_qdiscs; i++) {
fd19ff
         NMTCQdisc *   qdisc;
fd19ff
         gs_free char *str = NULL;
fd19ff
 
fd19ff
         qdisc = nm_setting_tc_config_get_qdisc(s_tc, i);
fd19ff
-        str   = nm_utils_tc_qdisc_to_str(qdisc, error);
fd19ff
-        if (!str)
fd19ff
-            return FALSE;
fd19ff
-
fd19ff
+        str   = nm_utils_tc_qdisc_to_str(qdisc, NULL);
fd19ff
+        nm_assert(str);
fd19ff
         svSetValueStr(ifcfg, numbered_tag(tag, "QDISC", n), str);
fd19ff
         n++;
fd19ff
     }
fd19ff
 
fd19ff
-    num = nm_setting_tc_config_get_num_tfilters(s_tc);
fd19ff
-    for (n = 1, i = 0; i < num; i++) {
fd19ff
+    num_filters = nm_setting_tc_config_get_num_tfilters(s_tc);
fd19ff
+    for (n = 1, i = 0; i < num_filters; i++) {
fd19ff
         NMTCTfilter * tfilter;
fd19ff
         gs_free char *str = NULL;
fd19ff
 
fd19ff
         tfilter = nm_setting_tc_config_get_tfilter(s_tc, i);
fd19ff
-        str     = nm_utils_tc_tfilter_to_str(tfilter, error);
fd19ff
-        if (!str)
fd19ff
-            return FALSE;
fd19ff
-
fd19ff
+        str     = nm_utils_tc_tfilter_to_str(tfilter, NULL);
fd19ff
+        nm_assert(str);
fd19ff
         svSetValueStr(ifcfg, numbered_tag(tag, "FILTER", n), str);
fd19ff
         n++;
fd19ff
     }
fd19ff
 
fd19ff
-    return TRUE;
fd19ff
+    if (num_qdiscs == 0 && num_filters == 0)
fd19ff
+        svSetValueBoolean(ifcfg, "TC_COMMIT", TRUE);
fd19ff
 }
fd19ff
 
fd19ff
 static void
fd19ff
@@ -3373,9 +3373,7 @@ do_write_construct(NMConnection *                  connection,
fd19ff
     write_match_setting(connection, ifcfg);
fd19ff
     write_hostname_setting(connection, ifcfg);
fd19ff
     write_sriov_setting(connection, ifcfg);
fd19ff
-
fd19ff
-    if (!write_tc_setting(connection, ifcfg, error))
fd19ff
-        return FALSE;
fd19ff
+    write_tc_setting(connection, ifcfg);
fd19ff
 
fd19ff
     route_path_is_svformat = utils_has_route_file_new_syntax(route_path);
fd19ff
 
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected
fd19ff
new file mode 100644
fd19ff
index 0000000000..4df768b463
fd19ff
--- /dev/null
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected
fd19ff
@@ -0,0 +1,15 @@
fd19ff
+TYPE=Ethernet
fd19ff
+PROXY_METHOD=none
fd19ff
+BROWSER_ONLY=no
fd19ff
+TC_COMMIT=yes
fd19ff
+BOOTPROTO=none
fd19ff
+IPADDR=1.1.1.3
fd19ff
+PREFIX=24
fd19ff
+GATEWAY=1.1.1.1
fd19ff
+DEFROUTE=yes
fd19ff
+IPV4_FAILURE_FATAL=no
fd19ff
+IPV6INIT=no
fd19ff
+NAME="Test Write TC config"
fd19ff
+UUID=${UUID}
fd19ff
+DEVICE=eth0
fd19ff
+ONBOOT=yes
fd19ff
diff --git a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
fd19ff
index 59127d0103..9d9ed62653 100644
fd19ff
--- a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
fd19ff
+++ b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
fd19ff
@@ -11108,6 +11108,85 @@ test_tc_read(void)
fd19ff
     g_object_unref(connection);
fd19ff
 }
fd19ff
 
fd19ff
+static void
fd19ff
+test_tc_write_empty(void)
fd19ff
+{
fd19ff
+    nmtst_auto_unlinkfile char *testfile     = NULL;
fd19ff
+    gs_unref_object NMConnection *connection = NULL;
fd19ff
+    gs_unref_object NMConnection *reread     = NULL;
fd19ff
+    NMSettingConnection *         s_con;
fd19ff
+    NMSettingIPConfig *           s_ip4;
fd19ff
+    NMSettingIPConfig *           s_ip6;
fd19ff
+    NMSettingWired *              s_wired;
fd19ff
+    NMSettingTCConfig *           s_tc;
fd19ff
+    NMIPAddress *                 addr;
fd19ff
+    GError *                      error = NULL;
fd19ff
+
fd19ff
+    connection = nm_simple_connection_new();
fd19ff
+
fd19ff
+    /* Connection setting */
fd19ff
+    s_con = (NMSettingConnection *) nm_setting_connection_new();
fd19ff
+    nm_connection_add_setting(connection, NM_SETTING(s_con));
fd19ff
+
fd19ff
+    g_object_set(s_con,
fd19ff
+                 NM_SETTING_CONNECTION_ID,
fd19ff
+                 "Test Write TC config",
fd19ff
+                 NM_SETTING_CONNECTION_UUID,
fd19ff
+                 nm_utils_uuid_generate_a(),
fd19ff
+                 NM_SETTING_CONNECTION_AUTOCONNECT,
fd19ff
+                 TRUE,
fd19ff
+                 NM_SETTING_CONNECTION_INTERFACE_NAME,
fd19ff
+                 "eth0",
fd19ff
+                 NM_SETTING_CONNECTION_TYPE,
fd19ff
+                 NM_SETTING_WIRED_SETTING_NAME,
fd19ff
+                 NULL);
fd19ff
+
fd19ff
+    /* Wired setting */
fd19ff
+    s_wired = (NMSettingWired *) nm_setting_wired_new();
fd19ff
+    nm_connection_add_setting(connection, NM_SETTING(s_wired));
fd19ff
+
fd19ff
+    /* IP4 setting */
fd19ff
+    s_ip4 = (NMSettingIPConfig *) nm_setting_ip4_config_new();
fd19ff
+    nm_connection_add_setting(connection, NM_SETTING(s_ip4));
fd19ff
+
fd19ff
+    g_object_set(s_ip4,
fd19ff
+                 NM_SETTING_IP_CONFIG_METHOD,
fd19ff
+                 NM_SETTING_IP4_CONFIG_METHOD_MANUAL,
fd19ff
+                 NM_SETTING_IP_CONFIG_GATEWAY,
fd19ff
+                 "1.1.1.1",
fd19ff
+                 NM_SETTING_IP_CONFIG_MAY_FAIL,
fd19ff
+                 TRUE,
fd19ff
+                 NULL);
fd19ff
+
fd19ff
+    addr = nm_ip_address_new(AF_INET, "1.1.1.3", 24, &error);
fd19ff
+    g_assert_no_error(error);
fd19ff
+    nm_setting_ip_config_add_address(s_ip4, addr);
fd19ff
+    nm_ip_address_unref(addr);
fd19ff
+
fd19ff
+    /* IP6 setting */
fd19ff
+    s_ip6 = (NMSettingIPConfig *) nm_setting_ip6_config_new();
fd19ff
+    nm_connection_add_setting(connection, NM_SETTING(s_ip6));
fd19ff
+
fd19ff
+    g_object_set(s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE, NULL);
fd19ff
+
fd19ff
+    /* TC setting */
fd19ff
+    s_tc = (NMSettingTCConfig *) nm_setting_tc_config_new();
fd19ff
+    nm_connection_add_setting(connection, NM_SETTING(s_tc));
fd19ff
+
fd19ff
+    nm_connection_add_setting(connection, nm_setting_proxy_new());
fd19ff
+
fd19ff
+    nmtst_assert_connection_verifies_without_normalization(connection);
fd19ff
+
fd19ff
+    _writer_new_connec_exp(connection,
fd19ff
+                           TEST_SCRATCH_DIR,
fd19ff
+                           TEST_IFCFG_DIR "/ifcfg-test-tc-write-empty.cexpected",
fd19ff
+                           &testfile);
fd19ff
+
fd19ff
+    reread = _connection_from_file(testfile, NULL, TYPE_BOND, NULL);
fd19ff
+
fd19ff
+    nmtst_assert_connection_equals(connection, FALSE, reread, FALSE);
fd19ff
+}
fd19ff
+
fd19ff
 static void
fd19ff
 test_tc_write(void)
fd19ff
 {
fd19ff
@@ -11848,6 +11927,7 @@ main(int argc, char **argv)
fd19ff
 
fd19ff
     g_test_add_func(TPATH "tc/read", test_tc_read);
fd19ff
     g_test_add_func(TPATH "tc/write", test_tc_write);
fd19ff
+    g_test_add_func(TPATH "tc/write_empty", test_tc_write_empty);
fd19ff
     g_test_add_func(TPATH "utils/test_well_known_keys", test_well_known_keys);
fd19ff
     g_test_add_func(TPATH "utils/test_utils_has_route_file_new_syntax",
fd19ff
                     test_utils_has_route_file_new_syntax);
fd19ff
-- 
fd19ff
2.31.1
fd19ff