Move management mode to the QSFP ports, not the transceiver (#1108)

- Moves the management mode subcommands to be directly under `swadm
  switch-port`, rather than `swadm switch-port transceiver`.
- Do not alter the management mode in the transceiver monitoring loop.
  This ensures that if an operator changes the mode, the loop doesn't
  change it back when the transceiver is deemed absent.
- Fixes #1106

Author: bnaecker

dpd/src/transceivers/tofino_impl.rs

@@ -1094,13 +1094,10 @@ impl Switch {
 
                 // If the module is now absent or faulted, let's reflect that in
                 // the switch port's transceiver object.
-                //
-                // TODO-correctness: This overrides whether a switch port has
-                // been disabled. Is that the correct thing to do? It might be
-                // better to only do that if the module is now absent.
                 if absent.is_set(index).unwrap() {
-                    port.set_management_mode(ManagementMode::Automatic)
-                        .unwrap();
+                    // NOTE: Do not modify the management mode itself, see
+                    // https://github.com/oxidecomputer/dendrite/issues/1106 for
+                    // context.
                     port.as_qsfp_mut().unwrap().transceiver = None;
                 } else if timeout_fault.is_set(index).unwrap() {
                     port.as_qsfp_mut().unwrap().transceiver =

swadm/src/main.rs

@@ -9,6 +9,7 @@ use std::io;
 use std::str::FromStr;
 
 use anyhow::Context;
+use common::ports::QsfpPort;
 use structopt::*;
 
 use common::ports::PortId;
@@ -128,6 +129,12 @@ impl FromStr for IpFamily {
     }
 }
 
+pub fn parse_qsfp_port_id(value: &str) -> Result<QsfpPort, String> {
+    value.parse().map_err(|_| {
+        format!("'{}' is invalid; QSFP ports are named qsfp<0-31>", value)
+    })
+}
+
 pub fn parse_port_id(value: &str) -> Result<PortId, String> {
     value.parse().map_err(|_| {
         format!("'{}' is invalid; valid port-ids include qsfp<0-31>, rear<0-31>, or int0", value)

swadm/src/switchport.rs

@@ -15,14 +15,14 @@ use colored::*;
 use structopt::*;
 use tabwriter::TabWriter;
 
-use common::ports::PortId;
+use common::ports::{PortId, QsfpPort};
 use dpd_client::types::{
     self, CmisDatapath, CmisLaneStatus, Sff8636Datapath, SffComplianceCode,
 };
 use dpd_client::Client;
 
-use crate::parse_port_id;
 use crate::LinkPath;
+use crate::{parse_port_id, parse_qsfp_port_id};
 
 // Newtype needed to convince `structopt` to parse a list of fields.
 #[derive(Clone, Debug)]
@@ -164,6 +164,37 @@ pub enum SwitchPort {
     Transceiver(Transceiver),
     /// Manage the attention LEDs on the Sidecar QSFP switch ports.
     Led(Led),
+    /// Get the management mode for a switch port's transceiver.
+    ///
+    /// In most situations, QSFP switch ports are managed automatically, meaning
+    /// their power is controlled autonomously. The modules will remain in low
+    /// power until a link is created on them. Modules will also be turned off
+    /// if they cannot be supported.
+    ///
+    /// Modules may be turned to manual management mode, which allows the
+    /// operator to explicitly control their power. The software will not change
+    /// the power of such a module automatically.
+    #[structopt(visible_aliases = &["mgmt", "mode"])]
+    ManagementMode {
+        /// The QSFP port to operate on.
+        #[structopt(parse(try_from_str = parse_qsfp_port_id))]
+        port_id: QsfpPort,
+    },
+    /// Set the management mode for a switch port's transceiver.
+    ///
+    /// See the help for `management-mode` for details.
+    #[structopt(visible_aliases = &["set-mgmt", "set-mode"])]
+    SetManagementMode {
+        /// The QSFP port to operate on.
+        #[structopt(parse(try_from_str = parse_qsfp_port_id))]
+        port_id: QsfpPort,
+        /// The management mode to set the port to.
+        #[structopt(
+            possible_values = &["automatic", "auto", "manual"],
+            parse(try_from_str = parse_management_mode)
+        )]
+        mode: types::ManagementMode,
+    },
     /// Return the backplane map.
     BackplaneMap {
         /// If true, provide parseable ouptut, separated by a `,`.
@@ -222,37 +253,6 @@ pub enum Transceiver {
         #[structopt(parse(try_from_str = parse_port_id))]
         port_id: PortId,
     },
-    /// Get the management mode for the transceiver.
-    ///
-    /// In most situations, QSFP switch ports are managed automatically, meaning
-    /// their power is controlled autonomously. The modules will remain in low
-    /// power until a link is created on them. Modules will also be turned off
-    /// if they cannot be supported.
-    ///
-    /// Modules may be turned to manual management mode, which allows the
-    /// operator to explicitly control their power. The software will not change
-    /// the power of such a module automatically.
-    #[structopt(visible_alias = "mgmt")]
-    ManagementMode {
-        /// The QSFP port to operate on.
-        #[structopt(parse(try_from_str = parse_port_id))]
-        port_id: PortId,
-    },
-    /// Set the management mode for the transceiver.
-    ///
-    /// See the help for `management-mode` for details.
-    #[structopt(visible_alias = "set-mgmt")]
-    SetManagementMode {
-        /// The QSFP port to operate on.
-        #[structopt(parse(try_from_str = parse_port_id))]
-        port_id: PortId,
-        /// The management mode to set the port to.
-        #[structopt(
-            possible_values = &["automatic", "auto", "manual"],
-            parse(try_from_str = parse_management_mode)
-        )]
-        mode: types::ManagementMode,
-    },
 }
 
 fn parse_management_mode(s: &str) -> anyhow::Result<types::ManagementMode> {
@@ -614,13 +614,6 @@ async fn transceivers_cmd(
                 println!("{:>WIDTH$}: {UNSUPPORTED}", "Aux 3");
             }
         }
-        Transceiver::ManagementMode { port_id } => {
-            let mode = client.management_mode_get(&port_id).await?.into_inner();
-            println!("{mode:?}");
-        }
-        Transceiver::SetManagementMode { port_id, mode } => {
-            client.management_mode_set(&port_id, mode).await?;
-        }
         Transceiver::Datapath { port_id } => {
             let datapath = client
                 .transceiver_datapath_get(&port_id)
@@ -932,6 +925,18 @@ pub async fn switch_cmd(
             }
         }
         SwitchPort::Transceiver(xcvr) => transceivers_cmd(client, xcvr).await?,
+        SwitchPort::ManagementMode { port_id } => {
+            let mode = client
+                .management_mode_get(&PortId::Qsfp(port_id))
+                .await?
+                .into_inner();
+            println!("{mode:?}");
+        }
+        SwitchPort::SetManagementMode { port_id, mode } => {
+            client
+                .management_mode_set(&PortId::Qsfp(port_id), mode)
+                .await?;
+        }
         SwitchPort::Led(led) => led_cmd(client, led).await?,
         SwitchPort::BackplaneMap {
             parseable,