From 59ead7095202fa4df5e33d3d78469a4c80471cd3 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Sat, 11 Jan 2020 14:11:58 +0100 Subject: [PATCH 1/3] initrd/ibft-reader: don't set con.interface-name in iBFT connections If an argument in form ip=eth0:ibft is specified, we'd first create a wired connection with con.interface-name and then proceed completing it from the iBFT block. At that point we also add the MAC address, so the interface-name is no longer necessary.. Worse even, for VLAN connections, it results in an attempt to create a VLAN with the same name as the parent wired device. Ooops. Let's just drop it. MAC address is guarranteed to be there and does the right thing for both plain wired devices as well as VLANs. --- src/initrd/nmi-ibft-reader.c | 1 + src/initrd/tests/test-cmdline-reader.c | 24 ++++++++++++++++++++++++ src/initrd/tests/test-ibft-reader.c | 3 +++ 3 files changed, 28 insertions(+) diff --git a/src/initrd/nmi-ibft-reader.c b/src/initrd/nmi-ibft-reader.c index ffce98fc40..47b90ebfdb 100644 --- a/src/initrd/nmi-ibft-reader.c +++ b/src/initrd/nmi-ibft-reader.c @@ -296,6 +296,7 @@ connection_setting_add (GHashTable *nic, NM_SETTING_CONNECTION_TYPE, type, NM_SETTING_CONNECTION_UUID, uuid, NM_SETTING_CONNECTION_ID, id, + NM_SETTING_CONNECTION_INTERFACE_NAME, NULL, NULL); g_free (uuid); diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index 1d4bb9a6e8..5ea5e105c0 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -767,6 +767,27 @@ test_team (void) g_assert_cmpint (nm_setting_connection_get_multi_connect (s_con), ==, NM_CONNECTION_MULTI_CONNECT_SINGLE); } +static void +test_ibft_ip_dev (void) +{ + const char *const*ARGV = NM_MAKE_STRV ("ip=eth0:ibft"); + gs_unref_hashtable GHashTable *connections = NULL; + NMSettingConnection *s_con; + NMConnection *connection; + + connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV); + g_assert (connections); + g_assert_cmpint (g_hash_table_size (connections), ==, 1); + + connection = g_hash_table_lookup (connections, "eth0"); + g_assert (connection); + + s_con = nm_connection_get_setting_connection (connection); + g_assert (s_con); + g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_VLAN_SETTING_NAME); + g_assert_cmpstr (nm_setting_connection_get_interface_name (s_con), ==, NULL); +} + static void test_ibft (void) { @@ -782,11 +803,13 @@ test_ibft (void) g_assert (connection); nmtst_assert_connection_verifies_without_normalization (connection); g_assert_cmpstr (nm_connection_get_id (connection), ==, "iBFT VLAN Connection 0"); + g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, NULL); connection = g_hash_table_lookup (connections, "ibft2"); g_assert (connection); nmtst_assert_connection_verifies_without_normalization (connection); g_assert_cmpstr (nm_connection_get_id (connection), ==, "iBFT Connection 2"); + g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, NULL); } static void @@ -1045,6 +1068,7 @@ int main (int argc, char **argv) g_test_add_func ("/initrd/cmdline/team", test_team); g_test_add_func ("/initrd/cmdline/bridge", test_bridge); g_test_add_func ("/initrd/cmdline/bridge/default", test_bridge_default); + g_test_add_func ("/initrd/cmdline/ibft/ip_dev", test_ibft_ip_dev); g_test_add_func ("/initrd/cmdline/ibft", test_ibft); g_test_add_func ("/initrd/cmdline/ignore_extra", test_ignore_extra); g_test_add_func ("/initrd/cmdline/rd_znet", test_rd_znet); diff --git a/src/initrd/tests/test-ibft-reader.c b/src/initrd/tests/test-ibft-reader.c index 932c1a48d3..f7709543f1 100644 --- a/src/initrd/tests/test-ibft-reader.c +++ b/src/initrd/tests/test-ibft-reader.c @@ -65,6 +65,7 @@ test_read_ibft_dhcp (void) g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_WIRED_SETTING_NAME); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "iBFT Connection 1"); + g_assert_cmpstr (nm_setting_connection_get_interface_name (s_con), ==, NULL); g_assert_cmpint (nm_setting_connection_get_timestamp (s_con), ==, 0); g_assert (nm_setting_connection_get_autoconnect (s_con)); @@ -109,6 +110,7 @@ test_read_ibft_static (void) g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_WIRED_SETTING_NAME); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "iBFT Connection 0"); + g_assert_cmpstr (nm_setting_connection_get_interface_name (s_con), ==, NULL); g_assert_cmpint (nm_setting_connection_get_timestamp (s_con), ==, 0); g_assert (nm_setting_connection_get_autoconnect (s_con)); @@ -178,6 +180,7 @@ test_read_ibft_vlan (void) s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_VLAN_SETTING_NAME); + g_assert_cmpstr (nm_setting_connection_get_interface_name (s_con), ==, NULL); /* ===== WIRED SETTING ===== */ s_wired = nm_connection_get_setting_wired (connection); From 39e1e723de016ca6953c9fb3b5762eb93cc13dbe Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Sat, 11 Jan 2020 14:13:19 +0100 Subject: [PATCH 2/3] initrd/cmdline: obey rd.iscsi.ibft Do process the connections from the iBFT block if the rd.iscsi.ibft or rd.iscsi.ibft=1 argument is present. This is supposed to fix what was originally reported by Kairui Song here: https://github.com/dracutdevs/dracut/pull/697 --- src/initrd/nmi-cmdline-reader.c | 86 ++++++++++++++------------ src/initrd/tests/test-cmdline-reader.c | 20 +++++- 2 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index c32ef1cfea..619133ca20 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -194,6 +194,48 @@ _base_setting_set (NMConnection *connection, const char *property, const char *v g_type_class_unref (object_class); } +static void +read_all_connections_from_fw (GHashTable *connections, const char *sysfs_dir) +{ + gs_unref_hashtable GHashTable *ibft = NULL; + NMConnection *connection; + GHashTableIter iter; + const char *mac; + GHashTable *nic; + const char *index; + GError *error = NULL; + + ibft = nmi_ibft_read (sysfs_dir); + + g_hash_table_iter_init (&iter, ibft); + while (g_hash_table_iter_next (&iter, (gpointer)&mac, (gpointer)&nic)) { + connection = nm_simple_connection_new (); + + index = g_hash_table_lookup (nic, "index"); + if (!index) { + _LOGW (LOGD_CORE, "Ignoring an iBFT entry without an index"); + continue; + } + + if (!nmi_ibft_update_connection_from_nic (connection, nic, &error)) { + _LOGW (LOGD_CORE, "Unable to merge iBFT configuration: %s", error->message); + g_error_free (error); + } + + g_hash_table_insert (connections, + g_strdup_printf ("ibft%s", index), + connection); + } + + connection = nmi_dt_reader_parse (sysfs_dir); + if (connection) { + g_hash_table_insert (connections, + g_strdup ("ofw"), + connection); + } +} + + static void parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument) { @@ -258,44 +300,7 @@ parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument) if (ifname == NULL && ( g_strcmp0 (kind, "fw") == 0 || g_strcmp0 (kind, "ibft") == 0)) { - GHashTableIter iter; - const char *mac; - GHashTable *nic; - const char *index; - - /* This is the ip=ibft case. Just take all we got from iBFT - * and don't process anything else, since there's no ifname - * specified to apply it to. */ - if (!ibft) - ibft = nmi_ibft_read (sysfs_dir); - - g_hash_table_iter_init (&iter, ibft); - while (g_hash_table_iter_next (&iter, (gpointer)&mac, (gpointer)&nic)) { - connection = nm_simple_connection_new (); - - index = g_hash_table_lookup (nic, "index"); - if (!index) { - _LOGW (LOGD_CORE, "Ignoring an iBFT entry without an index"); - continue; - } - - if (!nmi_ibft_update_connection_from_nic (connection, nic, &error)) { - _LOGW (LOGD_CORE, "Unable to merge iBFT configuration: %s", error->message); - g_error_free (error); - } - - g_hash_table_insert (connections, - g_strdup_printf ("ibft%s", index), - connection); - } - - connection = nmi_dt_reader_parse (sysfs_dir); - if (connection) { - g_hash_table_insert (connections, - g_strdup ("ofw"), - connection); - } - + read_all_connections_from_fw (connections, sysfs_dir); return; } @@ -421,8 +426,7 @@ parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument) if (mac) { g_strchomp (mac); mac_up = g_ascii_strup (mac, -1); - if (!ibft) - ibft = nmi_ibft_read (sysfs_dir); + ibft = nmi_ibft_read (sysfs_dir); nic = g_hash_table_lookup (ibft, mac_up); if (!nic) _LOGW (LOGD_CORE, "No iBFT NIC for %s (%s)", ifname, mac_up); @@ -838,6 +842,8 @@ nmi_cmdline_reader_parse (const char *sysfs_dir, const char *const*argv) parse_nameserver (connections, argument); else if (strcmp (tag, "rd.peerdns") == 0) parse_rd_peerdns (connections, argument); + else if (strcmp (tag, "rd.iscsi.ibft") == 0 && _nm_utils_ascii_str_to_bool (argument, TRUE)) + read_all_connections_from_fw (connections, sysfs_dir); else if (strcmp (tag, "rd.bootif") == 0) ignore_bootif = !_nm_utils_ascii_str_to_bool (argument, TRUE); else if (strcmp (tag, "rd.neednet") == 0) diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index 5ea5e105c0..dbfe5cc9b7 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -789,10 +789,9 @@ test_ibft_ip_dev (void) } static void -test_ibft (void) +_test_ibft_ip (const char *const*ARGV) { gs_unref_hashtable GHashTable *connections = NULL; - const char *const*ARGV = NM_MAKE_STRV ("ip=ibft"); NMConnection *connection; connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV); @@ -812,6 +811,20 @@ test_ibft (void) g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, NULL); } +static void +test_ibft_ip (void) +{ + const char *const*ARGV = NM_MAKE_STRV ("ip=ibft"); + _test_ibft_ip (ARGV); +} + +static void +test_ibft_rd_iscsi_ibft (void) +{ + const char *const*ARGV = NM_MAKE_STRV ("rd.iscsi.ibft"); + _test_ibft_ip (ARGV); +} + static void test_ignore_extra (void) { @@ -1069,7 +1082,8 @@ int main (int argc, char **argv) g_test_add_func ("/initrd/cmdline/bridge", test_bridge); g_test_add_func ("/initrd/cmdline/bridge/default", test_bridge_default); g_test_add_func ("/initrd/cmdline/ibft/ip_dev", test_ibft_ip_dev); - g_test_add_func ("/initrd/cmdline/ibft", test_ibft); + g_test_add_func ("/initrd/cmdline/ibft/ip", test_ibft_ip); + g_test_add_func ("/initrd/cmdline/ibft/rd_iscsi_ibft", test_ibft_rd_iscsi_ibft); g_test_add_func ("/initrd/cmdline/ignore_extra", test_ignore_extra); g_test_add_func ("/initrd/cmdline/rd_znet", test_rd_znet); g_test_add_func ("/initrd/cmdline/rd_znet/legacy", test_rd_znet_legacy); From 9f95b797f100fd8862123ba71fc9fc30c26e1d7b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Jan 2020 16:39:42 +0100 Subject: [PATCH 3/3] initrd/cmdline: minor style cleanups --- src/initrd/nmi-cmdline-reader.c | 3 +-- src/initrd/tests/test-cmdline-reader.c | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index 619133ca20..2cb93a2d19 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -208,7 +208,7 @@ read_all_connections_from_fw (GHashTable *connections, const char *sysfs_dir) ibft = nmi_ibft_read (sysfs_dir); g_hash_table_iter_init (&iter, ibft); - while (g_hash_table_iter_next (&iter, (gpointer)&mac, (gpointer)&nic)) { + while (g_hash_table_iter_next (&iter, (gpointer *) &mac, (gpointer *) &nic)) { connection = nm_simple_connection_new (); index = g_hash_table_lookup (nic, "index"); @@ -235,7 +235,6 @@ read_all_connections_from_fw (GHashTable *connections, const char *sysfs_dir) } } - static void parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument) { diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index dbfe5cc9b7..8951e4913e 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -815,6 +815,7 @@ static void test_ibft_ip (void) { const char *const*ARGV = NM_MAKE_STRV ("ip=ibft"); + _test_ibft_ip (ARGV); } @@ -822,6 +823,7 @@ static void test_ibft_rd_iscsi_ibft (void) { const char *const*ARGV = NM_MAKE_STRV ("rd.iscsi.ibft"); + _test_ibft_ip (ARGV); }