Merge pull request #2622 from zmrow/MAC-address-identify

Support configuration using MAC instead of device name

PROVISIONING-METAL.md

@@ -68,7 +68,6 @@ Full configuration details are covered in the [admin container documentation](ht
 ### Network interface configuration
 
 Bottlerocket for bare metal provides the means to configure the physical network interfaces in the system via TOML-formatted file `net.toml`.
-For now, simple DHCP4 and DHCP6 configuration is supported with plans to support additional configuration in the future.
 
 `net.toml` is read at boot time and generates the proper configuration files in the correct format for each interface described; no default configuration is provided.
 If no network configuration is provided, boot-time services like host containers, `containerd`, and `kubelet` will fail to start.
@@ -78,7 +77,7 @@ When these services fail, your machine will not connect to any cluster and will
 
 The configuration file must be valid TOML and have the filename `net.toml`.
 The first and required top level key in the file is `version`; the latest is version `3`.
-The rest of the file is a map of interface name to supported settings.
+The rest of the file is a map of interface name or MAC address to supported settings.
 Interface names are expected to be correct as per `udevd` naming, no interface naming or matching is supported.
 (See the note below regarding `udevd` interface naming.)
 
@@ -107,8 +106,9 @@ Please keep in mind that when using static addresses, DNS information must be su
   * `via` (IP address): Gateway IP address.  If no gateway is provided, a scope of `link` is assumed.
   * `route-metric` (integer): Relative route priority.
 
-Version `3` adds in support for bonding and vlan tagging.
-The support is limited to mode `1` (`active-backup`) for [bonding](https://www.kernel.org/doc/Documentation/networking/bonding.txt).
+Version `3` adds support for bonding, vlan tagging, and the ability to use a MAC address (colon or dash separated) as the identifier for an interface.
+MAC address identification is limited to interface configuration *only* and may not be used in conjunction with bonds or vlans.
+[Bonding](https://www.kernel.org/doc/Documentation/networking/bonding.txt) support is limited to mode `1` (`active-backup`).
 Future support may include other bonding options - pull requests are welcome!
 Version `3` adds the concept of virtual network devices in addition to interfaces.
 The default type of device is an interface and the syntax is the same as previous versions.
@@ -127,7 +127,7 @@ Bonding configuration creates a virtual network device across several other devi
 
 * Bonding configuration (map):
   * `kind = "bond"`: This setting is required to specify a bond device. Required.
-  * `interfaces` (list of quoted strings of interfaces): Which interfaces should be added to the bond (i.e. `["eno1"]`). The first in the list is considered the default `primary`. These interfaces are "consumed" so no other configuration can refer to them. Required.
+  * `interfaces` (list of quoted strings of interface names, not MAC addresses): Which interfaces should be added to the bond (i.e. `["eno1"]`). The first in the list is considered the default `primary`. These interfaces are "consumed" so no other configuration can refer to them. Required.
   * `mode` (string): Currently `active-backup` is the only supported option. Required.
   * `min-links` (integer): Number of links required to bring up the device
   * `monitoring` (map): Values m ust all be of `miimon` or `arpmon` type.
@@ -144,7 +144,7 @@ Vlan tagging is configured as a new virtual network device stacked on another de
 
 * Vlan configuration (map):
   * `kind = "vlan"`: This setting is required to specify a vlan device.
-  * `device` (string for device): Defines the device the vlan should be configured on. If VLAN tagging is required, this device should recieve all IP address configuration instead of the underlying device.
+  * `device` (string for device name, not MAC address): Defines the device the vlan should be configured on. If VLAN tagging is required, this device should recieve all IP address configuration instead of the underlying device.
   * `id` (integer): Number between 0 and 4096 specifying the vlan tag on the device
 
 Example `net.toml` version `3` with comments:
@@ -194,6 +194,18 @@ to = "10.10.10.0/24"
 from = "192.168.14.5"
 via = "192.168.14.25"
 
+# Interfaces may be configured using their MAC address rather than the interface name.
+# The MAC address must be quoted and colon or dash separated
+["0e:b3:69:44:b6:33"]
+dhcp4 = true
+
+["3e:03:69:49:e6:31".static4]
+addresses = ["10.0.0.15/24"]
+
+[["3e:03:69:49:e6:31".route]]
+to = "default"
+via = "10.0.0.1"
+
 # A bond is a network device that is of `kind` `bond`
 [bond0]
 kind = "bond"
@@ -202,7 +214,7 @@ mode = "active-backup"
 # In this case, the vlan will have addressing, the bond is simply there for use in the vlan
 dhcp4 = false
 dhcp6 = false
-# The first interface in the array is considered `primary` by default
+# The first interface in the array is considered `primary` by default, this list may not contain MAC addresses.
 interfaces = ["eno11", "eno12"]
 
 [bond0.monitoring]
@@ -226,6 +238,7 @@ arpmon-targets = ["192.168.1.1", "10.0.0.2"]
 # VLAN42 is the name of the device, can be anything that is a valid network interface name
 [VLAN42]
 kind = "vlan"
+# `device` may not contain a MAC address.
 device = "bond0"
 id = 42
 dhcp4 = true

packages/os/netdog-tmpfiles.conf

@@ -1,2 +1,3 @@
 d /var/lib/netdog 0700 root root -
+d /run/netdog 0700 root root -
 Z /var/lib/netdog 0700 root root -

packages/os/os.spec

@@ -43,8 +43,7 @@ Source115: link-kernel-modules.service.in
 Source116: load-kernel-modules.service.in
 Source117: cfsignal.service
 Source118: generate-network-config.service
-Source119: prepare-primary-interface.service
-Source120: reboot-if-required.service
+Source119: reboot-if-required.service
 
 # 2xx sources: tmpfilesd configs
 Source200: migration-tmpfiles.conf
@@ -403,7 +402,7 @@ install -d %{buildroot}%{_cross_unitdir}
 install -p -m 0644 \
   %{S:100} %{S:101} %{S:102} %{S:103} %{S:105} \
   %{S:106} %{S:107} %{S:110} %{S:111} %{S:112} \
-  %{S:113} %{S:114} %{S:118} %{S:119} %{S:120} \
+  %{S:113} %{S:114} %{S:118} %{S:119} \
   %{buildroot}%{_cross_unitdir}
 
 %if %{with nvidia_flavor}
@@ -453,7 +452,6 @@ install -p -m 0644 %{S:300} %{buildroot}%{_cross_udevrulesdir}/80-ephemeral-stor
 %{_cross_bindir}/netdog
 %{_cross_tmpfilesdir}/netdog.conf
 %{_cross_unitdir}/generate-network-config.service
-%{_cross_unitdir}/prepare-primary-interface.service
 
 %files -n %{_cross_os}corndog
 %{_cross_bindir}/corndog

sources/api/netdog/README.md

@@ -24,9 +24,6 @@ name of the interface, and valid options are "dhcp4" and "dhcp6".  A "?" may be
 to signify that the lease for the protocol is optional and the system shouldn't wait for it.  A
 valid example: `netdog.default-interface=eno1:dhcp4,dhcp6?`.
 
-The subcommand `prepare-primary-interface` writes the default sysctls for the primary interface to
-file in `/etc/sysctl.d`, and then executes `systemd-sysctl` to apply them.
-
 The subcommand `write-resolv-conf` writes the resolv.conf, favoring DNS API settings and
 supplementing any missing settings with DNS settings from the primary interface's DHCP lease.  It
 is meant to be used as a restart command for DNS API settings.

sources/api/netdog/src/cli/generate_net_config.rs

@@ -1,11 +1,14 @@
 use super::{error, Result};
+use crate::interface_id::InterfaceId;
+use crate::net_config;
 use crate::{
-    net_config, DEFAULT_NET_CONFIG_FILE, KERNEL_CMDLINE, OVERRIDE_NET_CONFIG_FILE,
-    PRIMARY_INTERFACE,
+    DEFAULT_NET_CONFIG_FILE, KERNEL_CMDLINE, OVERRIDE_NET_CONFIG_FILE, PRIMARY_INTERFACE,
+    PRIMARY_MAC_ADDRESS,
 };
 use argh::FromArgs;
 use snafu::{OptionExt, ResultExt};
-use std::{fs, path::Path};
+use std::fs;
+use std::path::Path;
 
 #[derive(FromArgs, PartialEq, Debug)]
 #[argh(subcommand, name = "generate-net-config")]
@@ -40,7 +43,10 @@ pub(crate) fn run() -> Result<()> {
     let primary_interface = net_config
         .primary_interface()
         .context(error::GetPrimaryInterfaceSnafu)?;
-    write_primary_interface(primary_interface)?;
+    // Remove existing primary interface files since the current primary may have changed or may
+    // now be using a MAC address (via an override net.toml/reboot)
+    remove_old_primary_interface()?;
+    write_primary_interface(&primary_interface)?;
 
     let wicked_interfaces = net_config.as_wicked_interfaces();
     for interface in wicked_interfaces {
@@ -51,13 +57,23 @@ pub(crate) fn run() -> Result<()> {
     Ok(())
 }
 
-/// Persist the primary interface name to file
-fn write_primary_interface<S>(interface: S) -> Result<()>
-where
-    S: AsRef<str>,
-{
-    let interface = interface.as_ref();
-    fs::write(PRIMARY_INTERFACE, interface).context(error::PrimaryInterfaceWriteSnafu {
+/// Remove primary interface and mac address files
+fn remove_old_primary_interface() -> Result<()> {
+    for file in &[PRIMARY_INTERFACE, PRIMARY_MAC_ADDRESS] {
+        if Path::exists(Path::new(file)) {
+            fs::remove_file(file).context(error::FileRemoveSnafu { path: file })?;
+        };
+    }
+    Ok(())
+}
+
+/// Persist the primary interface name or MAC to file
+fn write_primary_interface(interface_id: &InterfaceId) -> Result<()> {
+    match interface_id {
+        InterfaceId::Name(name) => fs::write(PRIMARY_INTERFACE, name.to_string()),
+        InterfaceId::MacAddress(mac) => fs::write(PRIMARY_MAC_ADDRESS, mac.to_string()),
+    }
+    .context(error::PrimaryInterfaceWriteSnafu {
         path: PRIMARY_INTERFACE,
     })
 }

sources/api/netdog/src/cli/install.rs

@@ -1,12 +1,14 @@
-use super::{error, InterfaceFamily, InterfaceType, Result};
+use super::{error, primary_interface_name, InterfaceFamily, InterfaceType, Result};
 use crate::dns::DnsSettings;
 use crate::lease::{dhcp_lease_path, static_lease_path, LeaseInfo};
-use crate::{CURRENT_IP, PRIMARY_INTERFACE};
+use crate::{CURRENT_IP, PRIMARY_SYSCTL_CONF, SYSCTL_MARKER_FILE, SYSTEMD_SYSCTL};
 use argh::FromArgs;
 use snafu::{ensure, OptionExt, ResultExt};
+use std::fmt::Write;
 use std::fs;
 use std::net::IpAddr;
 use std::path::{Path, PathBuf};
+use std::process::Command;
 
 #[derive(FromArgs, PartialEq, Debug)]
 #[argh(subcommand, name = "install")]
@@ -38,12 +40,7 @@ pub(crate) struct InstallArgs {
 pub(crate) fn run(args: InstallArgs) -> Result<()> {
     // Wicked doesn't mangle interface names, but let's be defensive.
     let install_interface = args.interface_name.trim().to_lowercase();
-    let primary_interface = fs::read_to_string(PRIMARY_INTERFACE)
-        .context(error::PrimaryInterfaceReadSnafu {
-            path: PRIMARY_INTERFACE,
-        })?
-        .trim()
-        .to_lowercase();
+    let primary_interface = primary_interface_name()?;
 
     if install_interface != primary_interface {
         return Ok(());
@@ -54,14 +51,66 @@ pub(crate) fn run(args: InstallArgs) -> Result<()> {
             interface_type @ (InterfaceType::Dhcp | InterfaceType::Static),
             InterfaceFamily::Ipv4 | InterfaceFamily::Ipv6,
         ) => {
-            let lease = fetch_lease(primary_interface, interface_type, args.data_file)?;
+            let lease = fetch_lease(&primary_interface, interface_type, args.data_file)?;
             write_resolv_conf(&lease)?;
             write_current_ip(&lease.ip_address.addr())?;
+
+            // If we haven't already, set and apply default sysctls for the primary network
+            // interface
+            if !Path::exists(Path::new(PRIMARY_SYSCTL_CONF)) {
+                write_interface_sysctl(primary_interface, PRIMARY_SYSCTL_CONF)?;
+            };
+
+            // Execute `systemd-sysctl` with our configuration file to set the sysctls
+            if !Path::exists(Path::new(SYSCTL_MARKER_FILE)) {
+                let systemd_sysctl_result = Command::new(SYSTEMD_SYSCTL)
+                    .arg(PRIMARY_SYSCTL_CONF)
+                    .output()
+                    .context(error::SystemdSysctlExecutionSnafu)?;
+                ensure!(
+                    systemd_sysctl_result.status.success(),
+                    error::FailedSystemdSysctlSnafu {
+                        stderr: String::from_utf8_lossy(&systemd_sysctl_result.stderr)
+                    }
+                );
+
+                fs::write(SYSCTL_MARKER_FILE, "").unwrap_or_else(|e| {
+                    eprintln!(
+                        "Failed to create marker file {}, netdog may attempt to set sysctls again: {}",
+                        SYSCTL_MARKER_FILE, e
+                    )
+                });
+            }
         }
     }
     Ok(())
 }
 
+/// Write the default sysctls for a given interface to a given path
+fn write_interface_sysctl<S, P>(interface: S, path: P) -> Result<()>
+where
+    S: AsRef<str>,
+    P: AsRef<Path>,
+{
+    let interface = interface.as_ref();
+    let path = path.as_ref();
+    // TODO if we accumulate more of these we should have a better way to create than format!()
+    // Note: The dash (-) preceding the "net..." variable assignment below is important; it
+    // ensures failure to set the variable for any reason will be logged, but not cause the sysctl
+    // service to fail
+    // Accept router advertisement (RA) packets even if IPv6 forwarding is enabled on interface
+    let ipv6_accept_ra = format!("-net.ipv6.conf.{}.accept_ra = 2", interface);
+    // Enable loose mode for reverse path filter
+    let ipv4_rp_filter = format!("-net.ipv4.conf.{}.rp_filter = 2", interface);
+
+    let mut output = String::new();
+    writeln!(output, "{}", ipv6_accept_ra).context(error::SysctlConfBuildSnafu)?;
+    writeln!(output, "{}", ipv4_rp_filter).context(error::SysctlConfBuildSnafu)?;
+
+    fs::write(path, output).context(error::SysctlConfWriteSnafu { path })?;
+    Ok(())
+}
+
 /// Given an interface, its type, and wicked's known location of the lease, compare our known lease
 /// location, parse and return a LeaseInfo.
 fn fetch_lease<S, P>(
@@ -106,3 +155,17 @@ fn write_current_ip(ip: &IpAddr) -> Result<()> {
     fs::write(CURRENT_IP, ip.to_string())
         .context(error::CurrentIpWriteFailedSnafu { path: CURRENT_IP })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn default_sysctls() {
+        let interface = "eno1";
+        let fake_file = tempfile::NamedTempFile::new().unwrap();
+        let expected = "-net.ipv6.conf.eno1.accept_ra = 2\n-net.ipv4.conf.eno1.rp_filter = 2\n";
+        write_interface_sysctl(interface, &fake_file).unwrap();
+        assert_eq!(std::fs::read_to_string(&fake_file).unwrap(), expected);
+    }
+}