Remove use of nomodeset by default

The troubleshooting kernel command line option nomodeset
unfortunately changes the way framebuffer interactions work
with graphics devices which in some cases can result in kernel
memory to be used for graphics updates. When this happens on
some specific hardware common in rack mount servers with baseboard
management controllers, this can cause the memory bus to become
locked for a brief time while the graphics update is occuring.

This locked memory bus means disk IO can become blocked,
and network cards can overflow their buffers resulting in
packet loss on top of the latency incurred by the graphics
update executing.

As such, we've removed the nomodeset option from default usage and
added a note describing its removal to the documentation along
with a release note.

Change-Id: I9084d88c3ec6f13bd64b8707892758fa87dd7f86

11 files changed, 48 insertions, 18 deletions

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index 17ba547f1..b81b9f38b 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1793,7 +1793,7 @@ function configure_ironic_conductor {
             -subj '/O=OpenStack/OU=DevStack Servers'
     fi
 
-    local pxe_params="nofb nomodeset console=${IRONIC_TTY_DEV}"
+    local pxe_params="nofb console=${IRONIC_TTY_DEV}"
     pxe_params+=" systemd.journald.forward_to_console=yes"
     if is_service_enabled tls-proxy; then
         pxe_params+=" ipa-insecure=1"
@@ -1805,7 +1805,7 @@ function configure_ironic_conductor {
         iniset $IRONIC_CONF_FILE pxe kernel_append_params "$pxe_params"
     fi
 
-    local kernel_append_params="nofb nomodeset console=${IRONIC_TTY_DEV}"
+    local kernel_append_params="nofb console=${IRONIC_TTY_DEV}"
     kernel_append_params+=" systemd.journald.forward_to_console=yes"
 
     # NOTE(dtantsur): avoid setting ipa-insecure for redfish, we have a way to
diff --git a/devstack/tools/ironic/scripts/cirros-partition.sh b/devstack/tools/ironic/scripts/cirros-partition.sh
index facf9b030..38e1764c4 100755
--- a/devstack/tools/ironic/scripts/cirros-partition.sh
+++ b/devstack/tools/ironic/scripts/cirros-partition.sh
@@ -18,7 +18,7 @@ IRONIC_TTY_DEV=${IRONIC_TTY_DEV:-ttyS0,115200}
 # rdroot : boot from the ramdisk present on the root partition instead of
 #          mounting the root partition.
 # dslist : disable Nova metadata support, it takes a long time on boot.
-KARGS=${KARGS:-nofb nomodeset vga=normal console=${IRONIC_TTY_DEV} rdroot dslist=configdrive}
+KARGS=${KARGS:-nofb vga=normal console=${IRONIC_TTY_DEV} rdroot dslist=configdrive}
 
 workdir=$(mktemp -d)
 root_mp=$workdir/root
diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index 063dd1fe5..771908838 100644
--- a/doc/source/admin/drivers/redfish.rst
+++ b/doc/source/admin/drivers/redfish.rst
@@ -214,7 +214,7 @@ user-specified kernel command line parameters.
 .. code-block:: bash
 
   baremetal node set node-0 \
-    --driver-info kernel_append_params="nofb nomodeset vga=normal"
+    --driver-info kernel_append_params="nofb vga=normal"
 
 .. note::
    The ``driver_info`` field is supported starting with the Xena release.
diff --git a/doc/source/admin/interfaces/boot.rst b/doc/source/admin/interfaces/boot.rst
index b9f0bda3e..a4b4cf2a3 100644
--- a/doc/source/admin/interfaces/boot.rst
+++ b/doc/source/admin/interfaces/boot.rst
@@ -49,7 +49,7 @@ configuration option:
 .. code-block:: ini
 
     [pxe]
-    kernel_append_params = nofb nomodeset vga=normal
+    kernel_append_params = nofb vga=normal
 
 .. note::
    The option was called ``pxe_append_params`` before the Xena cycle.
@@ -59,9 +59,9 @@ Per-node and per-instance overrides are also possible, for example:
 .. code-block:: bash
 
   baremetal node set node-0 \
-    --driver-info kernel_append_params="nofb nomodeset vga=normal"
+    --driver-info kernel_append_params="nofb vga=normal"
   baremetal node set node-0 \
-    --instance-info kernel_append_params="nofb nomodeset vga=normal"
+    --instance-info kernel_append_params="nofb vga=normal"
 
 Starting with the Zed cycle, you can combine the parameters from the
 configuration and from the node using the special ``%default%`` syntax:
@@ -74,13 +74,21 @@ configuration and from the node using the special ``%default%`` syntax:
 Together with the configuration above, the following parameters will be
 appended to the kernel command line::
 
-    nofb nomodeset vga=normal console=ttyS0,115200n8
+    nofb vga=normal console=ttyS0,115200n8
 
 .. note::
    Ironic does not do any de-duplication of the resulting kernel parameters.
    Both kernel itself and dracut seem to give priority to the last instance
    of the same parameter.
 
+.. warning::
+   Previously our documentation listed the Linux kernel parameter
+   ``nomodeset`` as an option. This option is intended for troubleshooting,
+   and can greatly degrade performance with Matrox/Aspeed BMC Graphics
+   controllers which is very commonly used on physical servers. The
+   performance degredation can greatly reduce IO capacity upon every
+   console graphics update being written to the screen.
+
 Common options
 --------------
 
diff --git a/doc/source/install/include/kernel-boot-parameters.inc b/doc/source/install/include/kernel-boot-parameters.inc
index f0da59d60..82b4ecc91 100644
--- a/doc/source/install/include/kernel-boot-parameters.inc
+++ b/doc/source/install/include/kernel-boot-parameters.inc
@@ -91,7 +91,7 @@ respectively.
         [pxe]
 
         # Additional append parameters for baremetal PXE boot.
-        kernel_append_params = nofb nomodeset vga=normal console=ttyS0,115200n8
+        kernel_append_params = nofb vga=normal console=ttyS0,115200n8
 
 
 * For node web console configuration is similar with the addition of ``ttyX``
@@ -100,7 +100,7 @@ respectively.
         [pxe]
 
         # Additional append parameters for baremetal PXE boot.
-        kernel_append_params = nofb nomodeset vga=normal console=tty0 console=ttyS0,115200n8
+        kernel_append_params = nofb vga=normal console=tty0 console=ttyS0,115200n8
 
 For detailed information on how to add consoles see the reference documents
 `kernel params`_ and `serial console`_.
diff --git a/ironic/conf/ilo.py b/ironic/conf/ilo.py
index 197378ce7..6d1c52dd2 100644
--- a/ironic/conf/ilo.py
+++ b/ironic/conf/ilo.py
@@ -112,7 +112,7 @@ opts = [
                       'This setting must be set to the octal number '
                       'representation, meaning starting with ``0o``.')),
     cfg.StrOpt('kernel_append_params',
-               default='nofb nomodeset vga=normal',
+               default='nofb vga=normal',
                mutable=True,
                help=_('Additional kernel parameters to pass down to the '
                       'instance kernel. These parameters can be consumed by '
diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py
index d96712824..e4c912192 100644
--- a/ironic/conf/pxe.py
+++ b/ironic/conf/pxe.py
@@ -24,7 +24,7 @@ from ironic.common.i18n import _
 opts = [
     cfg.StrOpt('kernel_append_params',
                deprecated_name='pxe_append_params',
-               default='nofb nomodeset vga=normal',
+               default='nofb vga=normal',
                mutable=True,
                help=_('Additional append parameters for baremetal PXE boot.')),
     cfg.StrOpt('default_ephemeral_format',
diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py
index 3cc9fe015..68aa96141 100644
--- a/ironic/conf/redfish.py
+++ b/ironic/conf/redfish.py
@@ -64,7 +64,7 @@ opts = [
                       'auto-expire. Applies only when `use_swift` is '
                       'enabled.')),
     cfg.StrOpt('kernel_append_params',
-               default='nofb nomodeset vga=normal',
+               default='nofb vga=normal',
                mutable=True,
                help=_('Additional kernel parameters to pass down to the '
                       'instance kernel. These parameters can be consumed by '
diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py
index b6c572125..fbb6318fe 100644
--- a/ironic/tests/unit/drivers/modules/test_image_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_image_utils.py
@@ -583,7 +583,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
             mock_create_boot_iso.assert_called_once_with(
                 mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
                 boot_mode='uefi', esp_image_href='http://bootloader/img',
-                kernel_params='nofb nomodeset vga=normal',
+                kernel_params='nofb vga=normal',
                 root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
                 inject_files=None)
 
@@ -604,7 +604,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
             mock_create_boot_iso.assert_called_once_with(
                 mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
                 boot_mode='uefi', esp_image_href=None,
-                kernel_params='nofb nomodeset vga=normal',
+                kernel_params='nofb vga=normal',
                 root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
                 inject_files=None)
 
@@ -633,7 +633,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
             mock_create_boot_iso.assert_called_once_with(
                 mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
                 boot_mode='bios', esp_image_href=None,
-                kernel_params='nofb nomodeset vga=normal',
+                kernel_params='nofb vga=normal',
                 root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
                 inject_files=None)
 
@@ -702,7 +702,7 @@ class RedfishImageUtilsTestCase(db_base.DbTestCase):
             mock_create_boot_iso.assert_called_once_with(
                 mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',
                 boot_mode='uefi', esp_image_href=None,
-                kernel_params=f'nofb nomodeset vga=normal {kernel_params}',
+                kernel_params=f'nofb vga=normal {kernel_params}',
                 root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
                 inject_files=None)
 
diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py
index ef37e3fc4..02d254b5a 100644
--- a/ironic/tests/unit/drivers/modules/test_ipxe.py
+++ b/ironic/tests/unit/drivers/modules/test_ipxe.py
@@ -813,7 +813,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
                 'aki_path': 'http://192.1.2.3:1234/' + uuid + '/kernel',
                 'ari_path': 'http://192.1.2.3:1234/' + uuid + '/ramdisk',
                 'ramdisk_opts': 'cat meow',
-                'pxe_append_params': 'nofb nomodeset vga=normal ipa-debug=1 '
+                'pxe_append_params': 'nofb vga=normal ipa-debug=1 '
                                      'ipa-global-request-'
                                      'id=' + task.context.request_id,
                 'tftp_server': mock.ANY,
diff --git a/releasenotes/notes/remove-nomodset-7a352a9519c1045b.yaml b/releasenotes/notes/remove-nomodset-7a352a9519c1045b.yaml
new file mode 100644
index 000000000..26d10c262
--- /dev/null
+++ b/releasenotes/notes/remove-nomodset-7a352a9519c1045b.yaml
@@ -0,0 +1,22 @@
+---
+upgrade:
+  - |
+    The defaults for ``kernel_append_params`` have had the Linux kernel
+    command line parameter ``nomodeset`` removed from the defaults for the
+    ``kernel_append_params`` settings. The ``nomodeset`` option is for
+    troubleshooting and changes the behavior of the graphics interface
+    such that memory can be locked upon graphical updates on physical
+    servers with BMC graphical interfaces, which results in spikes in
+    latency and packet loss whenever graphics updates occur. Operators
+    may add the option to their local configuration, but should be aware
+    that large image transfers or other high IO operations can be impacted.
+fixes:
+  - |
+    Agents deploying on physical servers with default kernel arguments were
+    suspetible to packet loss if a Matrox VGA/Aspeed BMC Graphics interface
+    is present on the machine. The defaults have been changed to remove
+    the use of the ``nomodeset`` kernel command line parameter which
+    should only be used for troubleshooting as it has been determined
+    that the memory updates can lock all of the kernel memory upon any
+    console graphics update which can negatively impact IO for Networking
+    or Disk interactions.