ifcfg-rh: preserve an empty tc configuration

If the TC setting contains no qdiscs and filters, it is lost after a
write-read cycle. Fix this by adding a new property to indicate the
presence of the (empty) setting.

(cherry picked from commit 6a88d4e55c)
(cherry picked from commit acf0c4df2b)
(cherry picked from commit 4efcdf234d)

Makefile.am

@@ -3158,6 +3158,7 @@ EXTRA_DIST += \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy.cexpected \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write.cexpected \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-1 \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-2 \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-team-master-invalid \

libnm-core/nm-setting-tc-config.c

@@ -1798,8 +1798,11 @@ nm_setting_tc_config_class_init (NMSettingTCConfigClass *klass)
 	 **/
 	/* ---ifcfg-rh---
 	 * property: qdiscs
-	 * variable: QDISC1(+), QDISC2(+), ...
+	 * variable: QDISC1(+), QDISC2(+), ..., TC_COMMIT(+)
-	 * description: Queueing disciplines
+	 * description: Queueing disciplines to set on the interface. When no
+	 *  QDISC1, QDISC2, ..., FILTER1, FILTER2, ... keys are present,
+	 *  NetworkManager doesn't touch qdiscs and filters present on the
+	 *  interface, unless TC_COMMIT is set to 'yes'.
 	 * example: QDISC1=ingress, QDISC2="root handle 1234: fq_codel"
 	 * ---end---
 	 */
@@ -1831,8 +1834,11 @@ nm_setting_tc_config_class_init (NMSettingTCConfigClass *klass)
 	 **/
 	/* ---ifcfg-rh---
 	 * property: qdiscs
-	 * variable: FILTER1(+), FILTER2(+), ...
+	 * variable: FILTER1(+), FILTER2(+), ..., TC_COMMIT(+)
-	 * description: Traffic filters
+	 * description: Traffic filters to set on the interface. When no
+	 *  QDISC1, QDISC2, ..., FILTER1, FILTER2, ... keys are present,
+	 *  NetworkManager doesn't touch qdiscs and filters present on the
+	 *  interface, unless TC_COMMIT is set to 'yes'.
 	 * example: FILTER1="parent ffff: matchall action simple sdata Input", ...
 	 * ---end---
 	 */

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c

@@ -2539,7 +2539,8 @@ make_tc_setting (shvarFile *ifcfg)
 	}
 
 	if (   nm_setting_tc_config_get_num_qdiscs (s_tc) > 0
-	    || nm_setting_tc_config_get_num_tfilters (s_tc) > 0)
+	    || nm_setting_tc_config_get_num_tfilters (s_tc) > 0
+	    || svGetValueBoolean(ifcfg, "TC_COMMIT", FALSE))
 		return NM_SETTING (s_tc);
 
 	g_object_unref (s_tc);

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c

@@ -1023,6 +1023,7 @@ const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[] = {
 	_KEY_TYPE ("STABLE_ID",                                   NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
 	_KEY_TYPE ("STP",                                         NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
 	_KEY_TYPE ("SUBCHANNELS",                                 NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
+	_KEY_TYPE ("TC_COMMIT",                                   NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
 	_KEY_TYPE ("TEAM_CONFIG",                                 NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
 	_KEY_TYPE ("TEAM_MASTER",                                 NMS_IFCFG_KEY_TYPE_IS_PLAIN ),
 	_KEY_TYPE ("TEAM_MASTER_UUID",                            NMS_IFCFG_KEY_TYPE_IS_PLAIN ),

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h

@@ -33,7 +33,7 @@ typedef struct {
 	NMSIfcfgKeyTypeFlags key_flags;
 } NMSIfcfgKeyTypeInfo;
 
-extern const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[241];
+extern const NMSIfcfgKeyTypeInfo nms_ifcfg_well_known_keys[242];
 
 const NMSIfcfgKeyTypeInfo *nms_ifcfg_well_known_key_find_info (const char *key, gssize *out_idx);
 

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c

@@ -2360,46 +2360,46 @@ write_sriov_setting (NMConnection *connection, shvarFile *ifcfg)
 	}
 }
 
-static gboolean
+static void
-write_tc_setting (NMConnection *connection, shvarFile *ifcfg, GError **error)
+write_tc_setting (NMConnection *connection, shvarFile *ifcfg)
 {
 	NMSettingTCConfig *s_tc;
-	guint i, num, n;
+	guint num_qdiscs;
+	guint num_filters;
+	guint i;
+	guint n;
 	char tag[64];
 
 	s_tc = nm_connection_get_setting_tc_config (connection);
 	if (!s_tc)
-		return TRUE;
+		return;
 
-	num = nm_setting_tc_config_get_num_qdiscs (s_tc);
+	num_qdiscs = nm_setting_tc_config_get_num_qdiscs (s_tc);
-	for (n = 1, i = 0; i < num; i++) {
+	for (n = 1, i = 0; i < num_qdiscs; i++) {
 		NMTCQdisc *qdisc;
 		gs_free char *str = NULL;
 
 		qdisc = nm_setting_tc_config_get_qdisc (s_tc, i);
-		str = nm_utils_tc_qdisc_to_str (qdisc, error);
+		str = nm_utils_tc_qdisc_to_str (qdisc, NULL);
-		if (!str)
+		nm_assert (str);
-			return FALSE;
-
 		svSetValueStr (ifcfg, numbered_tag (tag, "QDISC", n), str);
 		n++;
 	}
 
-	num = nm_setting_tc_config_get_num_tfilters (s_tc);
+	num_filters = nm_setting_tc_config_get_num_tfilters (s_tc);
-	for (n = 1, i = 0; i < num; i++) {
+	for (n = 1, i = 0; i < num_filters; i++) {
 		NMTCTfilter *tfilter;
 		gs_free char *str = NULL;
 
 		tfilter = nm_setting_tc_config_get_tfilter (s_tc, i);
-		str = nm_utils_tc_tfilter_to_str (tfilter, error);
+		str = nm_utils_tc_tfilter_to_str (tfilter, NULL);
-		if (!str)
+		nm_assert (str);
-			return FALSE;
-
 		svSetValueStr (ifcfg, numbered_tag (tag, "FILTER", n), str);
 		n++;
 	}
 
-	return TRUE;
+	if (num_qdiscs == 0 && num_filters == 0)
+		svSetValueBoolean (ifcfg, "TC_COMMIT", TRUE);
 }
 
 static void
@@ -3225,9 +3225,7 @@ do_write_construct (NMConnection *connection,
 
 	write_sriov_setting (connection, ifcfg);
 
-	if (!write_tc_setting (connection, ifcfg, error))
+	write_tc_setting (connection, ifcfg);
-		return FALSE;
-
 
 	route_path_is_svformat = utils_has_route_file_new_syntax (route_path);
 

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected

@@ -0,0 +1,15 @@
+TYPE=Ethernet
+PROXY_METHOD=none
+BROWSER_ONLY=no
+TC_COMMIT=yes
+BOOTPROTO=none
+IPADDR=1.1.1.3
+PREFIX=24
+GATEWAY=1.1.1.1
+DEFROUTE=yes
+IPV4_FAILURE_FATAL=no
+IPV6INIT=no
+NAME="Test Write TC config"
+UUID=${UUID}
+DEVICE=eth0
+ONBOOT=yes

src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

@@ -10179,6 +10179,85 @@ test_tc_read (void)
 	g_object_unref (connection);
 }
 
+static void
+test_tc_write_empty (void)
+{
+	nmtst_auto_unlinkfile char *testfile = NULL;
+	gs_unref_object NMConnection *connection = NULL;
+	gs_unref_object NMConnection *reread = NULL;
+	NMSettingConnection *s_con;
+	NMSettingIPConfig *s_ip4;
+	NMSettingIPConfig *s_ip6;
+	NMSettingWired *s_wired;
+	NMSettingTCConfig *s_tc;
+	NMIPAddress *addr;
+	GError *error = NULL;
+
+	connection = nm_simple_connection_new ();
+
+	/* Connection setting */
+	s_con = (NMSettingConnection*) nm_setting_connection_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_con));
+
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_ID,
+	              "Test Write TC config",
+	              NM_SETTING_CONNECTION_UUID,
+	              nm_utils_uuid_generate_a (),
+	              NM_SETTING_CONNECTION_AUTOCONNECT,
+	              TRUE,
+	              NM_SETTING_CONNECTION_INTERFACE_NAME,
+	              "eth0",
+	              NM_SETTING_CONNECTION_TYPE,
+	              NM_SETTING_WIRED_SETTING_NAME,
+	              NULL);
+
+	/* Wired setting */
+	s_wired = (NMSettingWired*) nm_setting_wired_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_wired));
+
+	/* IP4 setting */
+	s_ip4 = (NMSettingIPConfig*) nm_setting_ip4_config_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_ip4));
+
+	g_object_set (s_ip4,
+	              NM_SETTING_IP_CONFIG_METHOD,
+	              NM_SETTING_IP4_CONFIG_METHOD_MANUAL,
+	              NM_SETTING_IP_CONFIG_GATEWAY,
+	              "1.1.1.1",
+	              NM_SETTING_IP_CONFIG_MAY_FAIL,
+	              TRUE,
+	              NULL);
+
+	addr = nm_ip_address_new (AF_INET, "1.1.1.3", 24, &error);
+	g_assert_no_error (error);
+	nm_setting_ip_config_add_address (s_ip4, addr);
+	nm_ip_address_unref (addr);
+
+	/* IP6 setting */
+	s_ip6 = (NMSettingIPConfig*) nm_setting_ip6_config_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_ip6));
+
+	g_object_set (s_ip6, NM_SETTING_IP_CONFIG_METHOD,
+	              NM_SETTING_IP6_CONFIG_METHOD_IGNORE, NULL);
+
+	/* TC setting */
+	s_tc = (NMSettingTCConfig*) nm_setting_tc_config_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_tc));
+
+	nm_connection_add_setting (connection, nm_setting_proxy_new ());
+
+	nmtst_assert_connection_verifies_without_normalization (connection);
+
+	_writer_new_connec_exp (connection,
+	                        TEST_SCRATCH_DIR,
+	                        TEST_IFCFG_DIR "/ifcfg-test-tc-write-empty.cexpected", &testfile);
+
+	reread = _connection_from_file (testfile, NULL, TYPE_BOND, NULL);
+
+	nmtst_assert_connection_equals (connection, FALSE, reread, FALSE);
+}
+
 static void
 test_tc_write (void)
 {
@@ -10727,6 +10806,7 @@ int main (int argc, char **argv)
 
 	g_test_add_func (TPATH "tc/read", test_tc_read);
 	g_test_add_func (TPATH "tc/write", test_tc_write);
+	g_test_add_func (TPATH "tc/write_empty", test_tc_write_empty);
 	g_test_add_func (TPATH "utils/test_well_known_keys", test_well_known_keys);
 	g_test_add_func (TPATH "utils/test_utils_has_route_file_new_syntax", test_utils_has_route_file_new_syntax);