From 80e7022fa35881c2030ef23e6833fac1cfb0913d Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 25 May 2021 18:00:27 +0200 Subject: [PATCH] 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 6a88d4e55cf031da2b5a8458d21487a011357da4) (cherry picked from commit acf0c4df2b0fb0dc332aa929131953390998828f) (cherry picked from commit 4efcdf234d05d386314d370822f5e783071b5765) (cherry picked from commit d3ca2ed1fcbafbf05135b7f6c14bae5e090c26e1) (cherry picked from commit c826be4f7692e9a0efaf3626a0411fb7a94058dd) --- Makefile.am | 1 + libnm-core/nm-setting-tc-config.c | 14 +++- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 3 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 43 +++++----- .../ifcfg-test-tc-write-empty.cexpected | 15 ++++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 80 +++++++++++++++++++ 6 files changed, 131 insertions(+), 25 deletions(-) create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected diff --git a/Makefile.am b/Makefile.am index 1bfe2459f4..f957816141 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3045,6 +3045,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 \ diff --git a/libnm-core/nm-setting-tc-config.c b/libnm-core/nm-setting-tc-config.c index 71ab62a61b..93987edf16 100644 --- a/libnm-core/nm-setting-tc-config.c +++ b/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(+), ... - * description: Queueing disciplines + * variable: QDISC1(+), QDISC2(+), ..., TC_COMMIT(+) + * 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(+), ... - * description: Traffic filters + * variable: FILTER1(+), FILTER2(+), ..., TC_COMMIT(+) + * 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--- */ diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 14d94db803..3e221cae31 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -2431,7 +2431,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); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 017938ef12..ca222f08ce 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -2332,48 +2332,52 @@ write_sriov_setting (NMConnection *connection, shvarFile *ifcfg) } } -static gboolean -write_tc_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) +static void +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]; svUnsetAll (ifcfg, SV_KEY_TYPE_TC); s_tc = nm_connection_get_setting_tc_config (connection); - if (!s_tc) - return TRUE; + if (!s_tc) { + svUnsetValue (ifcfg, "TC_COMMIT"); + return; + } - num = nm_setting_tc_config_get_num_qdiscs (s_tc); - for (n = 1, i = 0; i < num; i++) { + num_qdiscs = nm_setting_tc_config_get_num_qdiscs (s_tc); + 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); - if (!str) - return FALSE; - + str = nm_utils_tc_qdisc_to_str (qdisc, NULL); + nm_assert (str); svSetValueStr (ifcfg, numbered_tag (tag, "QDISC", n), str); n++; } - num = nm_setting_tc_config_get_num_tfilters (s_tc); - for (n = 1, i = 0; i < num; i++) { + num_filters = nm_setting_tc_config_get_num_tfilters (s_tc); + 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); - if (!str) - return FALSE; - + str = nm_utils_tc_tfilter_to_str (tfilter, NULL); + nm_assert (str); svSetValueStr (ifcfg, numbered_tag (tag, "FILTER", n), str); n++; } - return TRUE; + if (num_qdiscs == 0 && num_filters == 0) + svSetValueBoolean (ifcfg, "TC_COMMIT", TRUE); + else + svUnsetValue (ifcfg, "TC_COMMIT"); } static gboolean @@ -3242,8 +3246,7 @@ do_write_construct (NMConnection *connection, write_sriov_setting (connection, ifcfg); - if (!write_tc_setting (connection, ifcfg, error)) - return FALSE; + write_tc_setting (connection, ifcfg); svUnsetValue (ifcfg, "DHCP_HOSTNAME"); svUnsetValue (ifcfg, "DHCP_FQDN"); diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-tc-write-empty.cexpected new file mode 100644 index 0000000000..4df768b463 --- /dev/null +++ b/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 diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index a782ccc621..442a7ddb30 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -10123,6 +10123,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) { @@ -10505,6 +10584,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); return g_test_run (); }