nm-initrd-generator: Merge branch 'al/initrdgenerator-ethtool-docs'

- remove duplex option 

- include man entry for rd.ethtool options

https://bugzilla.redhat.com/show_bug.cgi?id=1940934

!960

3 files changed, 92 insertions, 96 deletions

diff --git a/man/nm-initrd-generator.xml b/man/nm-initrd-generator.xml
index 5a758357ed..503700fa80 100644
--- a/man/nm-initrd-generator.xml
+++ b/man/nm-initrd-generator.xml
@@ -147,6 +147,7 @@
             <member><option>nameserver</option></member>
             <member><option>rd.peerdns</option></member>
             <member><option>rd.bootif</option></member>
+            <member><option>rd.ethtool</option></member>
             <member><option>rd.net.timeout.dhcp</option></member>
             <member><option>rd.net.timeout.carrier</option></member>
             <member><option>rd.znet</option></member>
@@ -195,6 +196,21 @@
         example "ib.pkey=ib0.8004".
         </para>
       </listitem>
+
+      <listitem>
+        <para>NetworkManager supports the 
+        <option>rd.ethtool</option>=<replaceable>INTERFACE</replaceable>:<replaceable>AUTONEG</replaceable>:<replaceable>SPEED</replaceable> 
+        kernel command line option to set up ethtool NIC configuration parameters 
+        <replaceable>AUTONEG</replaceable> and <replaceable>SPEED</replaceable>. 
+        The <replaceable>INTERFACE</replaceable> being configured must be 
+        specified, and the other parameters are optional and can be left blank. 
+        When <replaceable>SPEED</replaceable> is set, duplex mode is 
+        automatically set to 'full'. <replaceable>INTERFACE</replaceable> 
+        accepts string values, <replaceable>AUTONEG</replaceable> accepts 
+        boolean values (true and false / on or off / 0 or 1), and 
+        <replaceable>SPEED</replaceable> accepts positive integer values.
+        </para>
+      </listitem>
     </itemizedlist>
   </refsect1>
 
diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c
index 51c12c00ef..aa0d915128 100644
--- a/src/nm-initrd-generator/nmi-cmdline-reader.c
+++ b/src/nm-initrd-generator/nmi-cmdline-reader.c
@@ -1173,92 +1173,60 @@ reader_parse_rd_znet(Reader *reader, char *argument, gboolean net_ifnames)
 static void
 reader_parse_ethtool(Reader *reader, char *argument)
 {
-    NMConnection *  connection = NULL;
-    NMSettingWired *s_wired    = NULL;
-    const char *    read       = NULL;
-    const char *    interface  = NULL;
-    gboolean        autoneg    = FALSE;
-    guint           speed      = 0;
+    const char *    interface   = NULL;
+    NMConnection *  connection  = NULL;
+    NMSettingWired *s_wired     = NULL;
+    const char *    autoneg_str = NULL;
+    gboolean        autoneg     = FALSE;
+    const char *    speed_str   = NULL;
+    guint           speed       = 0;
 
     interface = get_word(&argument, ':');
     if (!interface) {
         _LOGW(LOGD_CORE, "Impossible to set rd.ethtool options: invalid format");
         return;
     }
-    if (!*argument)
+
+    if (!*argument) {
+        _LOGW(LOGD_CORE, "Could not find rd.ethtool options to set");
         return;
+    }
+
+    connection = reader_get_connection(reader, interface, NM_SETTING_WIRED_SETTING_NAME, TRUE);
+    s_wired    = nm_connection_get_setting_wired(connection);
 
-    read = get_word(&argument, ':');
-    if (read) {
-        autoneg = _nm_utils_ascii_str_to_bool(read, -1);
+    autoneg_str = get_word(&argument, ':');
+    if (autoneg_str) {
+        autoneg = _nm_utils_ascii_str_to_bool(autoneg_str, -1);
         if (autoneg == -1)
-            _LOGW(LOGD_CORE, "rd.ethtool autoneg was not set, invalid value");
-        else {
-            connection =
-                reader_get_connection(reader, interface, NM_SETTING_WIRED_SETTING_NAME, TRUE);
-            s_wired = nm_connection_get_setting_wired(connection);
+            _LOGW(LOGD_CORE,
+                  "Invalid value for rd.ethtool.autoneg, rd.ethtool.autoneg was not set");
+        else
             g_object_set(s_wired, NM_SETTING_WIRED_AUTO_NEGOTIATE, autoneg, NULL);
-        }
     }
     if (!*argument)
         return;
 
-    read = get_word(&argument, ':');
-    if (read) {
-        speed = _nm_utils_ascii_str_to_int64(read, 10, 0, G_MAXUINT32, -1);
-        if (speed == -1) {
-            _LOGW(LOGD_CORE,
-                  "rd.ethtool speed was not set, invalid value. Then, duplex was disregarded.");
-            /* Duplex does not need to be evaluated after, because it can't be set without speed value */
-            return;
-        } else {
-            connection =
-                reader_get_connection(reader, interface, NM_SETTING_WIRED_SETTING_NAME, TRUE);
-            s_wired = nm_connection_get_setting_wired(connection);
-            /* duplex option is available for legacy purposes */
-            /* speed must be always set having duplex set, otherwise it will fail in verifications */
-
-            if (*argument) {
-                /* duplex value was informed, and has a valid value */
-                if (NM_IN_STRSET(argument, "half", "full"))
-                    g_object_set(s_wired,
-                                 NM_SETTING_WIRED_SPEED,
-                                 speed,
-                                 NM_SETTING_WIRED_DUPLEX,
-                                 argument,
-                                 NULL);
-
-                /* duplex value was informed, and does not have a valid value */
-                else {
-                    _LOGW(LOGD_CORE,
-                          "rd.ethtool.duplex has a invalid format, duplex was set as default:full");
-                    g_object_set(s_wired,
-                                 NM_SETTING_WIRED_SPEED,
-                                 speed,
-                                 NM_SETTING_WIRED_DUPLEX,
-                                 "full",
-                                 NULL);
-                }
-            } else {
-                /* duplex value was not informed, then it will have default 'full' value */
-                g_object_set(s_wired,
-                             NM_SETTING_WIRED_SPEED,
-                             speed,
-                             NM_SETTING_WIRED_DUPLEX,
-                             "full",
-                             NULL);
-            }
-
-            /* Duplex does not need to be evaluated alone - it can't be set without speed value */
-            return;
-        }
+    speed_str = get_word(&argument, ':');
+    if (speed_str) {
+        speed = _nm_utils_ascii_str_to_int64(speed_str, 10, 0, G_MAXUINT32, -1);
+        if (speed == -1)
+            _LOGW(LOGD_CORE, "Invalid value for rd.ethtool.speed, rd.ethtool.speed was not set");
+        else
+            g_object_set(s_wired,
+                         NM_SETTING_WIRED_SPEED,
+                         speed,
+                         NM_SETTING_WIRED_DUPLEX,
+                         "full",
+                         NULL);
     }
+
     if (!*argument)
         return;
-    else {
-        /* Duplex does not need to be evaluated, because it can't be set without speed value */
-        _LOGW(LOGD_CORE, "rd.ethtool.duplex needs rd.ethtool.speed value to be set");
-    }
+    else
+        _LOGW(LOGD_CORE,
+              "Invalid extra argument '%s' for rd.ethtool, this value was not set",
+              argument);
 }
 
 static void
diff --git a/src/nm-initrd-generator/tests/test-cmdline-reader.c b/src/nm-initrd-generator/tests/test-cmdline-reader.c
index aa12378763..214b771033 100644
--- a/src/nm-initrd-generator/tests/test-cmdline-reader.c
+++ b/src/nm-initrd-generator/tests/test-cmdline-reader.c
@@ -2262,7 +2262,8 @@ test_carrier_timeout(void)
     g_assert_cmpint(carrier_timeout_sec, ==, 20);
 }
 
-/* Obs1.: this function is implemented as macro, and not as a function, to show g_assert() line on debug */
+/* Obs1.: this function is implemented as macro, and not as a function,
+ * to show the correct line in g_assert() debug */
 #define _ethtool_connection_check_and_get(connection)                                \
     ({                                                                               \
         NMSettingWired *_s_wired    = NULL;                                          \
@@ -2273,7 +2274,7 @@ test_carrier_timeout(void)
         g_assert(nm_connection_get_setting_ip4_config(_connection));                 \
         g_assert(nm_connection_get_setting_ip6_config(_connection));                 \
         _s_wired = nm_connection_get_setting_wired(_connection);                     \
-        g_assert(_s_wired);                                                          \
+        g_assert(NM_IS_SETTING_WIRED(_s_wired));                                     \
                                                                                      \
         _s_wired;                                                                    \
     })
@@ -2293,15 +2294,19 @@ test_rd_ethtool(void)
     g_hash_table_unref(connections);
     g_test_assert_expected_messages();
 
-    ARGV        = NM_MAKE_STRV("rd.ethtool=eth0");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0");
+    NMTST_EXPECT_NM_WARN("cmdline-reader: Could not find rd.ethtool options to set");
     connections = _parse_cons(ARGV);
     g_assert_cmpint(g_hash_table_size(connections), ==, 0);
     g_hash_table_unref(connections);
+    g_test_assert_expected_messages();
 
-    ARGV        = NM_MAKE_STRV("rd.ethtool=eth0:");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:");
+    NMTST_EXPECT_NM_WARN("cmdline-reader: Could not find rd.ethtool options to set");
     connections = _parse_cons(ARGV);
     g_assert_cmpint(g_hash_table_size(connections), ==, 0);
     g_hash_table_unref(connections);
+    g_test_assert_expected_messages();
 
     ARGV = NM_MAKE_STRV("rd.ethtool=::");
     NMTST_EXPECT_NM_WARN("cmdline-reader: Impossible to set rd.ethtool options: invalid format");
@@ -2347,46 +2352,47 @@ test_rd_ethtool(void)
     g_object_unref(connection);
 
     ARGV = NM_MAKE_STRV("rd.ethtool=eth0:randomstring");
-    NMTST_EXPECT_NM_WARN("cmdline-reader: rd.ethtool autoneg was not set, invalid value");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid value for rd.ethtool.autoneg, rd.ethtool.autoneg was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
     ARGV        = NM_MAKE_STRV("rd.ethtool=eth0::");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_hash_table_unref(connections);
 
     ARGV = NM_MAKE_STRV("rd.ethtool=eth0::astring");
-    NMTST_EXPECT_NM_WARN("cmdline-reader: rd.ethtool speed was not set, invalid value. Then, "
-                         "duplex was disregarded.");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid value for rd.ethtool.speed, rd.ethtool.speed was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
     ARGV = NM_MAKE_STRV("rd.ethtool=eth0::1000000000000000000000000000000000000");
-    NMTST_EXPECT_NM_WARN("cmdline-reader: rd.ethtool speed was not set, invalid value. Then, "
-                         "duplex was disregarded.");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid value for rd.ethtool.speed, rd.ethtool.speed was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
     ARGV = NM_MAKE_STRV("rd.ethtool=eth0::0.67");
-    NMTST_EXPECT_NM_WARN("cmdline-reader: rd.ethtool speed was not set, invalid value. Then, "
-                         "duplex was disregarded.");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid value for rd.ethtool.speed, rd.ethtool.speed was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
     ARGV = NM_MAKE_STRV("rd.ethtool=eth0::-23");
-    NMTST_EXPECT_NM_WARN("cmdline-reader: rd.ethtool speed was not set, invalid value. Then, "
-                         "duplex was disregarded.");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid value for rd.ethtool.speed, rd.ethtool.speed was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
@@ -2406,33 +2412,39 @@ test_rd_ethtool(void)
     g_assert_cmpstr(nm_setting_wired_get_duplex(s_wired), ==, "full");
     g_object_unref(connection);
 
-    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:::half");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:::bogus");
     NMTST_EXPECT_NM_WARN(
-        "cmdline-reader: rd.ethtool.duplex needs rd.ethtool.speed value to be set");
+        "cmdline-reader: Invalid extra argument 'bogus' for rd.ethtool, this value was not set");
     connections = _parse_cons(ARGV);
-    g_assert_cmpint(g_hash_table_size(connections), ==, 0);
+    g_assert_cmpint(g_hash_table_size(connections), ==, 1);
     g_test_assert_expected_messages();
     g_hash_table_unref(connections);
 
-    ARGV       = NM_MAKE_STRV("rd.ethtool=eth0::10:half");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0::10:bogus");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid extra argument 'bogus' for rd.ethtool, this value was not set");
     connection = _parse_con(ARGV, "eth0");
     s_wired    = _ethtool_connection_check_and_get(connection);
     g_assert(!nm_setting_wired_get_auto_negotiate(s_wired));
     g_assert_cmpint(nm_setting_wired_get_speed(s_wired), ==, 10);
-    g_assert_cmpstr(nm_setting_wired_get_duplex(s_wired), ==, "half");
+    g_assert_cmpstr(nm_setting_wired_get_duplex(s_wired), ==, "full");
+    g_test_assert_expected_messages();
     g_object_unref(connection);
 
-    ARGV       = NM_MAKE_STRV("rd.ethtool=eth0:on:100:full");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:on:100:bogus");
+    NMTST_EXPECT_NM_WARN(
+        "cmdline-reader: Invalid extra argument 'bogus' for rd.ethtool, this value was not set");
     connection = _parse_con(ARGV, "eth0");
     s_wired    = _ethtool_connection_check_and_get(connection);
     g_assert(nm_setting_wired_get_auto_negotiate(s_wired));
     g_assert_cmpint(nm_setting_wired_get_speed(s_wired), ==, 100);
     g_assert_cmpstr(nm_setting_wired_get_duplex(s_wired), ==, "full");
+    g_test_assert_expected_messages();
     g_object_unref(connection);
 
-    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:on:100:anyvalue");
+    ARGV = NM_MAKE_STRV("rd.ethtool=eth0:on:100:bogus");
     NMTST_EXPECT_NM_WARN(
-        "cmdline-reader: rd.ethtool.duplex has a invalid format, duplex was set as default:full");
+        "cmdline-reader: Invalid extra argument 'bogus' for rd.ethtool, this value was not set");
     connection = _parse_con(ARGV, "eth0");
     s_wired    = _ethtool_connection_check_and_get(connection);
     g_assert(nm_setting_wired_get_auto_negotiate(s_wired));