[PATCH proxmox{,-backup} v2 0/3] fix #6121: add 'bond-miimon' option for network interfaces

[PATCH proxmox{,-backup} v2 0/3] fix #6121: add 'bond-miimon' option for network interfaces

[Christoph Heiss]

Fixes #6121 [0], for both PBS and PDM.

Adds a new 'bond-miimon' option to the network interface API for
specifying the link monitoring interval.

This is needed so that (at least) in active-backup mode, the bond
correctly switches over to the backup link if the active one goes down.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6121

History
=======
v1: https://lore.proxmox.com/pbs-devel/20260925123459.424903-1-c.heiss@proxmox.com/

Changes v1 -> v2:
  * rebased on latest masters
  * changed underlying value type from u8 to u32 (see also v1 discussion)

Diffstat
========

proxmox:

Christoph Heiss (2):
  fix #6121: network-api: api: add `bond-miimon` option for interfaces
  fix #6121: network-api: config: add support for `bond-miimon` option

 proxmox-network-api/src/api_impl.rs      |  9 +++
 proxmox-network-api/src/api_types.rs     | 22 ++++++++
 proxmox-network-api/src/config/lexer.rs  |  3 +
 proxmox-network-api/src/config/mod.rs    | 70 ++++++++++++++++++++++++
 proxmox-network-api/src/config/parser.rs | 23 ++++++++
 5 files changed, 127 insertions(+)

proxmox-backup:

Christoph Heiss (1):
  api: network: reuse interface create/update methods from network-api

 src/api2/node/network.rs | 674 ++-------------------------------------
 1 file changed, 18 insertions(+), 656 deletions(-)

[PATCH proxmox v2 1/3] fix #6121: network-api: api: add `bond-miimon` option for interfaces

[Christoph Heiss]

Partially fixes #6121 [0].

Adds a new 'bond-miimon' option to the network interface API for
specifying the link monitoring interval.

Linux restricts the miimon value to 0..INT_MAX [0], which is reflected
in the API.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6121
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/bonding/bond_options.c?h=v7.3#n404

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * changed underlying miimon value type from u8 to u32, restricting it
    to INT_MAX

 proxmox-network-api/src/api_impl.rs  |  9 +++++++++
 proxmox-network-api/src/api_types.rs | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/proxmox-network-api/src/api_impl.rs b/proxmox-network-api/src/api_impl.rs
index 78f499c5..0a603017 100644
--- a/proxmox-network-api/src/api_impl.rs
+++ b/proxmox-network-api/src/api_impl.rs
@@ -162,6 +162,9 @@ pub fn create_interface(iface: String, config: InterfaceUpdater) -> Result<(), E
                     interface.bond_xmit_hash_policy = config.bond_xmit_hash_policy;
                 }
             }
+            if config.bond_miimon.is_some() {
+                interface.bond_miimon = config.bond_miimon;
+            }
             if let Some(slaves) = &config.slaves {
                 interface.set_bond_slave_list(slaves)?;
             }
@@ -295,6 +298,9 @@ pub fn update_interface(
                 DeletableInterfaceProperty::BondXmitHashPolicy => {
                     interface.bond_xmit_hash_policy = None
                 }
+                DeletableInterfaceProperty::BondMiimon => {
+                    interface.bond_miimon = None;
+                }
             }
         }
     }
@@ -335,6 +341,9 @@ pub fn update_interface(
             }
             interface.bond_xmit_hash_policy = update.bond_xmit_hash_policy;
         }
+        if update.bond_miimon.is_some() {
+            interface.bond_miimon = update.bond_miimon;
+        }
     }
 
     if let Some(cidr) = update.cidr {
diff --git a/proxmox-network-api/src/api_types.rs b/proxmox-network-api/src/api_types.rs
index 58dd7c94..9b267458 100644
--- a/proxmox-network-api/src/api_types.rs
+++ b/proxmox-network-api/src/api_types.rs
@@ -166,6 +166,14 @@ pub const NETWORK_INTERFACE_LIST_SCHEMA: Schema =
             type: BondXmitHashPolicy,
             optional: true,
         },
+        "bond-miimon": {
+            description: "Link monitoring interval in milliseconds for bond interfaces.",
+            type: Integer,
+            optional: true,
+            default: 100,
+            minimum: 0,
+            maximum: i32::MAX as i64,
+        },
         altnames: {
             description: "List of altnames for this interface",
             type: Array,
@@ -240,6 +248,8 @@ pub struct Interface {
     pub bond_primary: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub bond_xmit_hash_policy: Option<BondXmitHashPolicy>,
+    #[serde(rename = "bond-miimon", skip_serializing_if = "Option::is_none")]
+    pub bond_miimon: Option<u32>,
 
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub altnames: Vec<String>,
@@ -271,6 +281,7 @@ impl Interface {
             bond_mode: None,
             bond_primary: None,
             bond_xmit_hash_policy: None,
+            bond_miimon: None,
             altnames: Vec::new(),
         }
     }
@@ -363,6 +374,8 @@ pub enum DeletableInterfaceProperty {
     /// Delete bond transmit hash policy
     #[serde(rename = "bond_xmit_hash_policy")]
     BondXmitHashPolicy,
+    /// Delete bond miimon (set to default 100)
+    BondMiimon,
 }
 
 #[api(
@@ -447,6 +460,14 @@ pub enum DeletableInterfaceProperty {
                 type: BondXmitHashPolicy,
                 optional: true,
             },
+            "bond-miimon": {
+                description: "Link monitoring interval in milliseconds for bond interfaces.",
+                type: Integer,
+                optional: true,
+                default: 100,
+                minimum: 0,
+                maximum: i32::MAX as i64,
+            },
             slaves: {
                 schema: NETWORK_INTERFACE_LIST_SCHEMA,
                 optional: true,
@@ -480,5 +501,6 @@ pub struct InterfaceUpdater {
     pub bond_primary: Option<String>,
     #[serde(rename = "bond_xmit_hash_policy")]
     pub bond_xmit_hash_policy: Option<BondXmitHashPolicy>,
+    pub bond_miimon: Option<u32>,
     pub slaves: Option<String>,
 }
-- 
2.56.0

[PATCH proxmox v2 2/3] fix #6121: network-api: config: add support for `bond-miimon` option

[Christoph Heiss]

Fixes #6121 [0].

Adds support for the 'bond-miimon' option to the ifupdown(2) config
writer and parser.

The option is always written out for bonds, with a default value of 100
- much like PVE does it.

This is needed so that (at least) in active-backup mode, the bond
correctly switches over to the backup link if the active one goes down.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6121

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 proxmox-network-api/src/config/lexer.rs  |  3 +
 proxmox-network-api/src/config/mod.rs    | 70 ++++++++++++++++++++++++
 proxmox-network-api/src/config/parser.rs | 23 ++++++++
 3 files changed, 96 insertions(+)

diff --git a/proxmox-network-api/src/config/lexer.rs b/proxmox-network-api/src/config/lexer.rs
index 4729d462..2a646508 100644
--- a/proxmox-network-api/src/config/lexer.rs
+++ b/proxmox-network-api/src/config/lexer.rs
@@ -30,6 +30,7 @@ pub enum Token {
     BondMode,
     BondPrimary,
     BondXmitHashPolicy,
+    BondMiimon,
     EOF,
 }
 
@@ -62,6 +63,8 @@ static KEYWORDS: LazyLock<HashMap<&'static str, Token>> = LazyLock::new(|| {
     map.insert("bond_primary", Token::BondPrimary);
     map.insert("bond_xmit_hash_policy", Token::BondXmitHashPolicy);
     map.insert("bond-xmit-hash-policy", Token::BondXmitHashPolicy);
+    map.insert("bond-miimon", Token::BondMiimon);
+    map.insert("bond_miimon", Token::BondMiimon);
     map
 });
 
diff --git a/proxmox-network-api/src/config/mod.rs b/proxmox-network-api/src/config/mod.rs
index 09165bfb..3eff9284 100644
--- a/proxmox-network-api/src/config/mod.rs
+++ b/proxmox-network-api/src/config/mod.rs
@@ -89,6 +89,9 @@ fn write_iface_attributes(iface: &Interface, w: &mut dyn Write) -> Result<(), Er
                 }
             }
 
+            let miimon = iface.bond_miimon.unwrap_or(100);
+            writeln!(w, "\tbond-miimon {miimon}")?;
+
             let slaves = iface.slaves.as_ref().unwrap_or(&EMPTY_LIST);
             if slaves.is_empty() {
                 writeln!(w, "\tbond-slaves none")?;
@@ -791,4 +794,71 @@ iface individual_name inet manual
         assert_eq!(parse_vlan_raw_device_from_name("vmbr0"), None);
         assert_eq!(parse_vlan_raw_device_from_name("vmbr0.200"), Some("vmbr0"));
     }
+
+    #[test]
+    fn test_write_network_config_bond_default_miimon() {
+        let mut bond = Interface::new("bond0".to_owned());
+        bond.interface_type = NetworkInterfaceType::Bond;
+        bond.method = Some(NetworkConfigMethod::Manual);
+        bond.slaves = Some(vec![String::from("eth0"), String::from("eth1")]);
+        bond.bond_mode = Some(LinuxBondMode::ActiveBackup);
+
+        let mut eth0 = Interface::new("eth0".to_owned());
+        eth0.interface_type = NetworkInterfaceType::Eth;
+
+        let mut eth1 = Interface::new("eth1".to_owned());
+        eth1.interface_type = NetworkInterfaceType::Eth;
+
+        let nw_config = NetworkConfig {
+            interfaces: BTreeMap::from([
+                ("eth0".into(), eth0),
+                ("eth1".into(), eth1),
+                ("bond0".into(), bond),
+            ]),
+            order: vec![NetworkOrderEntry::Iface("bond0".to_owned())],
+        };
+        assert_eq!(
+            String::try_from(nw_config).unwrap().trim(),
+            r#"
+iface bond0 inet manual
+	bond-mode active-backup
+	bond-miimon 100
+	bond-slaves eth0 eth1"#
+                .trim()
+        );
+    }
+
+    #[test]
+    fn test_write_network_config_bond_explicit_miimon() {
+        let mut bond = Interface::new("bond0".to_owned());
+        bond.interface_type = NetworkInterfaceType::Bond;
+        bond.method = Some(NetworkConfigMethod::Manual);
+        bond.slaves = Some(vec![String::from("eth0"), String::from("eth1")]);
+        bond.bond_mode = Some(LinuxBondMode::ActiveBackup);
+        bond.bond_miimon = Some(250);
+
+        let mut eth0 = Interface::new("eth0".to_owned());
+        eth0.interface_type = NetworkInterfaceType::Eth;
+
+        let mut eth1 = Interface::new("eth1".to_owned());
+        eth1.interface_type = NetworkInterfaceType::Eth;
+
+        let nw_config = NetworkConfig {
+            interfaces: BTreeMap::from([
+                ("eth0".into(), eth0),
+                ("eth1".into(), eth1),
+                ("bond0".into(), bond),
+            ]),
+            order: vec![NetworkOrderEntry::Iface("bond0".to_owned())],
+        };
+        assert_eq!(
+            String::try_from(nw_config).unwrap().trim(),
+            r#"
+iface bond0 inet manual
+	bond-mode active-backup
+	bond-miimon 250
+	bond-slaves eth0 eth1"#
+                .trim()
+        );
+    }
 }
diff --git a/proxmox-network-api/src/config/parser.rs b/proxmox-network-api/src/config/parser.rs
index 71c9ec0f..2b789f64 100644
--- a/proxmox-network-api/src/config/parser.rs
+++ b/proxmox-network-api/src/config/parser.rs
@@ -378,6 +378,12 @@ impl<R: BufRead> NetworkParser<R> {
                     interface.bond_xmit_hash_policy = Some(policy);
                     self.eat(Token::Newline)?;
                 }
+                Token::BondMiimon => {
+                    self.eat(Token::BondMiimon)?;
+                    let miimon = self.next_text()?.parse()?;
+                    interface.bond_miimon = Some(miimon);
+                    self.eat(Token::Newline)?;
+                }
                 Token::VlanId => {
                     self.eat(Token::VlanId)?;
                     let vlan_id = self.next_text()?.parse()?;
@@ -918,4 +924,21 @@ iface individual_name inet static
         assert_eq!(iface.method, Some(NetworkConfigMethod::Static));
         assert_eq!(iface.cidr, Some(String::from("10.0.0.100/16")));
     }
+
+    #[test]
+    fn test_network_config_parser_bond_miimon() {
+        let input = r#"
+iface bond0 inet manual
+	bond-slaves eth0 eth1
+	bond-mode active-backup
+	bond-miimon 200
+"#;
+
+        let mut parser = NetworkParser::new(input.as_bytes());
+        let config = parser.parse_interfaces(None).unwrap();
+
+        let iface = config.interfaces.get("bond0").unwrap();
+        assert_eq!(iface.interface_type, NetworkInterfaceType::Bond);
+        assert_eq!(iface.bond_miimon, Some(200));
+    }
 }
-- 
2.56.0

[PATCH proxmox-backup v2 3/3] api: network: reuse interface create/update methods from network-api

[Christoph Heiss]

They are nearly identical in parameters and implementation, only that the
network-api crate uses a struct instead of a long list of single
parameters.

The only real difference are bit more strict checks, namely:
- the 'auto' method is now rejected for IPv4, which is IPv6 only
- enforces that the method is 'static' when explicitly setting CIDR/GW

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
In combination w/ the earlier patches in the series; also fixes #6121
[0].

Changes v1 -> v2:
  * no changes

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6121

 src/api2/node/network.rs | 674 ++-------------------------------------
 1 file changed, 18 insertions(+), 656 deletions(-)

diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs
index 07ef4bb64..591b4b6e8 100644
--- a/src/api2/node/network.rs
+++ b/src/api2/node/network.rs
@@ -1,5 +1,4 @@
-use anyhow::{Error, bail};
-use serde::{Deserialize, Serialize};
+use anyhow::Error;
 use serde_json::{Value, to_value};
 
 use proxmox_router::{ApiMethod, Permission, Router, RpcEnvironment};
@@ -9,86 +8,14 @@ use pbs_api_types::{
     Authid, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA,
 };
 
+use proxmox_config_digest::ConfigDigest;
 use proxmox_network_api::{
-    self as network, BondXmitHashPolicy, CIDR_V4_SCHEMA, CIDR_V6_SCHEMA, IP_V4_SCHEMA,
-    IP_V6_SCHEMA, Interface, LinuxBondMode, NETWORK_INTERFACE_ARRAY_SCHEMA,
-    NETWORK_INTERFACE_LIST_SCHEMA, NETWORK_INTERFACE_NAME_SCHEMA, NetworkConfig,
-    NetworkConfigMethod, NetworkInterfaceType, parse_vlan_id_from_name,
-    parse_vlan_raw_device_from_name,
+    self as network, DeletableInterfaceProperty, Interface, InterfaceUpdater,
+    NETWORK_INTERFACE_NAME_SCHEMA,
 };
 
 use proxmox_rest_server::WorkerTask;
 
-fn split_interface_list(list: &str) -> Result<Vec<String>, Error> {
-    let value = NETWORK_INTERFACE_ARRAY_SCHEMA.parse_property_string(list)?;
-    Ok(value
-        .as_array()
-        .unwrap()
-        .iter()
-        .map(|v| v.as_str().unwrap().to_string())
-        .collect())
-}
-
-fn check_duplicate_gateway_v4(config: &NetworkConfig, iface: &str) -> Result<(), Error> {
-    let current_gateway_v4 = config
-        .interfaces
-        .iter()
-        .find(|(_, interface)| interface.gateway.is_some())
-        .map(|(name, _)| name.to_string());
-
-    if let Some(current_gateway_v4) = current_gateway_v4 {
-        if current_gateway_v4 != iface {
-            bail!(
-                "Default IPv4 gateway already exists on interface '{}'",
-                current_gateway_v4
-            );
-        }
-    }
-    Ok(())
-}
-
-fn check_duplicate_gateway_v6(config: &NetworkConfig, iface: &str) -> Result<(), Error> {
-    let current_gateway_v6 = config
-        .interfaces
-        .iter()
-        .find(|(_, interface)| interface.gateway6.is_some())
-        .map(|(name, _)| name.to_string());
-
-    if let Some(current_gateway_v6) = current_gateway_v6 {
-        if current_gateway_v6 != iface {
-            bail!(
-                "Default IPv6 gateway already exists on interface '{}'",
-                current_gateway_v6
-            );
-        }
-    }
-    Ok(())
-}
-
-fn set_bridge_ports(iface: &mut Interface, ports: Vec<String>) -> Result<(), Error> {
-    if iface.interface_type != NetworkInterfaceType::Bridge {
-        bail!(
-            "interface '{}' is no bridge (type is {:?})",
-            iface.name,
-            iface.interface_type
-        );
-    }
-    iface.bridge_ports = Some(ports);
-    Ok(())
-}
-
-fn set_bond_slaves(iface: &mut Interface, slaves: Vec<String>) -> Result<(), Error> {
-    if iface.interface_type != NetworkInterfaceType::Bond {
-        bail!(
-            "interface '{}' is no bond (type is {:?})",
-            iface.name,
-            iface.interface_type
-        );
-    }
-    iface.slaves = Some(slaves);
-    Ok(())
-}
-
 #[api(
     input: {
         properties: {
@@ -175,89 +102,9 @@ pub fn read_interface(iface: String) -> Result<Value, Error> {
             iface: {
                 schema: NETWORK_INTERFACE_NAME_SCHEMA,
             },
-            "type": {
-                type: NetworkInterfaceType,
-                optional: true,
-            },
-            autostart: {
-                description: "Autostart interface.",
-                type: bool,
-                optional: true,
-            },
-            method: {
-                type: NetworkConfigMethod,
-                optional: true,
-            },
-            method6: {
-                type: NetworkConfigMethod,
-                optional: true,
-            },
-            comments: {
-                description: "Comments (inet, may span multiple lines)",
-                type: String,
-                optional: true,
-            },
-            comments6: {
-                description: "Comments (inet5, may span multiple lines)",
-                type: String,
-                optional: true,
-            },
-            cidr: {
-                schema: CIDR_V4_SCHEMA,
-                optional: true,
-            },
-            cidr6: {
-                schema: CIDR_V6_SCHEMA,
-                optional: true,
-            },
-            gateway: {
-                schema: IP_V4_SCHEMA,
-                optional: true,
-            },
-            gateway6: {
-                schema: IP_V6_SCHEMA,
-                optional: true,
-            },
-            mtu: {
-                description: "Maximum Transmission Unit.",
-                optional: true,
-                minimum: 46,
-                maximum: 65535,
-                default: 1500,
-            },
-            bridge_ports: {
-                schema: NETWORK_INTERFACE_LIST_SCHEMA,
-                optional: true,
-            },
-            bridge_vlan_aware: {
-                description: "Enable bridge vlan support.",
-                type: bool,
-                optional: true,
-            },
-            "vlan-id": {
-                description: "VLAN ID.",
-                type: u16,
-                optional: true,
-            },
-            "vlan-raw-device": {
-                schema: NETWORK_INTERFACE_NAME_SCHEMA,
-                optional: true,
-            },
-            bond_mode: {
-                type: LinuxBondMode,
-                optional: true,
-            },
-            "bond-primary": {
-                schema: NETWORK_INTERFACE_NAME_SCHEMA,
-                optional: true,
-            },
-            bond_xmit_hash_policy: {
-                type: BondXmitHashPolicy,
-                optional: true,
-            },
-            slaves: {
-                schema: NETWORK_INTERFACE_LIST_SCHEMA,
-                optional: true,
+            config: {
+                type: InterfaceUpdater,
+                flatten: true,
             },
         },
     },
@@ -266,210 +113,8 @@ pub fn read_interface(iface: String) -> Result<Value, Error> {
     },
 )]
 /// Create network interface configuration.
-#[allow(clippy::too_many_arguments)]
-pub fn create_interface(
-    iface: String,
-    autostart: Option<bool>,
-    method: Option<NetworkConfigMethod>,
-    method6: Option<NetworkConfigMethod>,
-    comments: Option<String>,
-    comments6: Option<String>,
-    cidr: Option<String>,
-    gateway: Option<String>,
-    cidr6: Option<String>,
-    gateway6: Option<String>,
-    mtu: Option<u64>,
-    bridge_ports: Option<String>,
-    bridge_vlan_aware: Option<bool>,
-    vlan_id: Option<u16>,
-    vlan_raw_device: Option<String>,
-    bond_mode: Option<LinuxBondMode>,
-    bond_primary: Option<String>,
-    bond_xmit_hash_policy: Option<BondXmitHashPolicy>,
-    slaves: Option<String>,
-    param: Value,
-) -> Result<(), Error> {
-    let interface_type = pbs_tools::json::required_string_param(&param, "type")?;
-    let interface_type: NetworkInterfaceType = serde_json::from_value(interface_type.into())?;
-
-    let _lock = network::lock_config()?;
-
-    let (mut config, _digest) = network::config()?;
-
-    if config.interfaces.contains_key(&iface) {
-        bail!("interface '{}' already exists", iface);
-    }
-
-    let mut interface = Interface::new(iface.clone());
-    interface.interface_type = interface_type;
-
-    if let Some(autostart) = autostart {
-        interface.autostart = autostart;
-    }
-    if method.is_some() {
-        interface.method = method;
-    }
-    if method6.is_some() {
-        interface.method6 = method6;
-    }
-    if mtu.is_some() {
-        interface.mtu = mtu;
-    }
-    if comments.is_some() {
-        interface.comments = comments;
-    }
-    if comments6.is_some() {
-        interface.comments6 = comments6;
-    }
-
-    if let Some(cidr) = cidr {
-        let (_, _, is_v6) = network::parse_cidr(&cidr)?;
-        if is_v6 {
-            bail!("invalid address type (expected IPv4, got IPv6)");
-        }
-        interface.cidr = Some(cidr);
-    }
-
-    if let Some(cidr6) = cidr6 {
-        let (_, _, is_v6) = network::parse_cidr(&cidr6)?;
-        if !is_v6 {
-            bail!("invalid address type (expected IPv6, got IPv4)");
-        }
-        interface.cidr6 = Some(cidr6);
-    }
-
-    if let Some(gateway) = gateway {
-        let is_v6 = gateway.contains(':');
-        if is_v6 {
-            bail!("invalid address type (expected IPv4, got IPv6)");
-        }
-        check_duplicate_gateway_v4(&config, &iface)?;
-        interface.gateway = Some(gateway);
-    }
-
-    if let Some(gateway6) = gateway6 {
-        let is_v6 = gateway6.contains(':');
-        if !is_v6 {
-            bail!("invalid address type (expected IPv6, got IPv4)");
-        }
-        check_duplicate_gateway_v6(&config, &iface)?;
-        interface.gateway6 = Some(gateway6);
-    }
-
-    match interface_type {
-        NetworkInterfaceType::Bridge => {
-            if let Some(ports) = bridge_ports {
-                let ports = split_interface_list(&ports)?;
-                set_bridge_ports(&mut interface, ports)?;
-            }
-            if bridge_vlan_aware.is_some() {
-                interface.bridge_vlan_aware = bridge_vlan_aware;
-            }
-        }
-        NetworkInterfaceType::Bond => {
-            if let Some(mode) = bond_mode {
-                interface.bond_mode = bond_mode;
-                if bond_primary.is_some() {
-                    if mode != LinuxBondMode::ActiveBackup {
-                        bail!("bond-primary is only valid with Active/Backup mode");
-                    }
-                    interface.bond_primary = bond_primary;
-                }
-                if bond_xmit_hash_policy.is_some() {
-                    if mode != LinuxBondMode::Ieee802_3ad && mode != LinuxBondMode::BalanceXor {
-                        bail!(
-                            "bond_xmit_hash_policy is only valid with LACP(802.3ad) or balance-xor mode"
-                        );
-                    }
-                    interface.bond_xmit_hash_policy = bond_xmit_hash_policy;
-                }
-            }
-            if let Some(slaves) = slaves {
-                let slaves = split_interface_list(&slaves)?;
-                set_bond_slaves(&mut interface, slaves)?;
-            }
-        }
-        NetworkInterfaceType::Vlan => {
-            if vlan_id.is_none() && parse_vlan_id_from_name(&iface).is_none() {
-                bail!("vlan-id must be set");
-            }
-            interface.vlan_id = vlan_id;
-
-            if let Some(dev) = vlan_raw_device
-                .as_deref()
-                .or_else(|| parse_vlan_raw_device_from_name(&iface))
-            {
-                if !config.interfaces.contains_key(dev) {
-                    bail!("vlan-raw-device {dev} does not exist");
-                }
-            } else {
-                bail!("vlan-raw-device must be set");
-            }
-            interface.vlan_raw_device = vlan_raw_device;
-        }
-        _ => bail!(
-            "creating network interface type '{:?}' is not supported",
-            interface_type
-        ),
-    }
-
-    if interface.cidr.is_some() || interface.gateway.is_some() {
-        interface.method = Some(NetworkConfigMethod::Static);
-    } else if interface.method.is_none() {
-        interface.method = Some(NetworkConfigMethod::Manual);
-    }
-
-    if interface.cidr6.is_some() || interface.gateway6.is_some() {
-        interface.method6 = Some(NetworkConfigMethod::Static);
-    } else if interface.method6.is_none() {
-        interface.method6 = Some(NetworkConfigMethod::Manual);
-    }
-
-    config.interfaces.insert(iface, interface);
-
-    network::save_config(&config)?;
-
-    Ok(())
-}
-
-#[api()]
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-/// Deletable property name
-pub enum DeletableProperty {
-    /// Delete the IPv4 address property.
-    Cidr,
-    /// Delete the IPv6 address property.
-    Cidr6,
-    /// Delete the IPv4 gateway property.
-    Gateway,
-    /// Delete the IPv6 gateway property.
-    Gateway6,
-    /// Delete the whole IPv4 configuration entry.
-    Method,
-    /// Delete the whole IPv6 configuration entry.
-    Method6,
-    /// Delete IPv4 comments
-    Comments,
-    /// Delete IPv6 comments
-    Comments6,
-    /// Delete mtu.
-    Mtu,
-    /// Delete autostart flag
-    Autostart,
-    /// Delete bridge ports (set to 'none')
-    #[serde(rename = "bridge_ports")]
-    BridgePorts,
-    /// Delete bridge-vlan-aware flag
-    #[serde(rename = "bridge_vlan_aware")]
-    BridgeVlanAware,
-    /// Delete bond-slaves (set to 'none')
-    Slaves,
-    /// Delete bond-primary
-    BondPrimary,
-    /// Delete bond transmit hash policy
-    #[serde(rename = "bond_xmit_hash_policy")]
-    BondXmitHashPolicy,
+pub fn create_interface(iface: String, config: InterfaceUpdater) -> Result<(), Error> {
+    proxmox_network_api::create_interface(iface, config)
 }
 
 #[api(
@@ -482,101 +127,21 @@ pub enum DeletableProperty {
             iface: {
                 schema: NETWORK_INTERFACE_NAME_SCHEMA,
             },
-            "type": {
-                type: NetworkInterfaceType,
-                optional: true,
-            },
-            autostart: {
-                description: "Autostart interface.",
-                type: bool,
-                optional: true,
-            },
-            method: {
-                type: NetworkConfigMethod,
-                optional: true,
-            },
-            method6: {
-                type: NetworkConfigMethod,
-                optional: true,
-            },
-            comments: {
-                description: "Comments (inet, may span multiple lines)",
-                type: String,
-                optional: true,
-            },
-            comments6: {
-                description: "Comments (inet5, may span multiple lines)",
-                type: String,
-                optional: true,
-            },
-            cidr: {
-                schema: CIDR_V4_SCHEMA,
-                optional: true,
-            },
-            cidr6: {
-                schema: CIDR_V6_SCHEMA,
-                optional: true,
-            },
-            gateway: {
-                schema: IP_V4_SCHEMA,
-                optional: true,
-            },
-            gateway6: {
-                schema: IP_V6_SCHEMA,
-                optional: true,
-            },
-            mtu: {
-                description: "Maximum Transmission Unit.",
-                optional: true,
-                minimum: 46,
-                maximum: 65535,
-                default: 1500,
-            },
-            bridge_ports: {
-                schema: NETWORK_INTERFACE_LIST_SCHEMA,
-                optional: true,
-            },
-            bridge_vlan_aware: {
-                description: "Enable bridge vlan support.",
-                type: bool,
-                optional: true,
-            },
-            "vlan-id": {
-                description: "VLAN ID.",
-                type: u16,
-                optional: true,
-            },
-            "vlan-raw-device": {
-                schema: NETWORK_INTERFACE_NAME_SCHEMA,
-                optional: true,
-            },
-            bond_mode: {
-                type: LinuxBondMode,
-                optional: true,
-            },
-            "bond-primary": {
-                schema: NETWORK_INTERFACE_NAME_SCHEMA,
-                optional: true,
-            },
-            bond_xmit_hash_policy: {
-                type: BondXmitHashPolicy,
-                optional: true,
-            },
-            slaves: {
-                schema: NETWORK_INTERFACE_LIST_SCHEMA,
-                optional: true,
+            update: {
+                type: InterfaceUpdater,
+                flatten: true,
             },
             delete: {
                 description: "List of properties to delete.",
                 type: Array,
                 optional: true,
                 items: {
-                    type: DeletableProperty,
+                    type: DeletableInterfaceProperty,
                 }
             },
             digest: {
+                type: ConfigDigest,
                 optional: true,
-                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
             },
         },
     },
@@ -585,216 +150,13 @@ pub enum DeletableProperty {
     },
 )]
 /// Update network interface config.
-#[allow(clippy::too_many_arguments)]
 pub fn update_interface(
     iface: String,
-    autostart: Option<bool>,
-    method: Option<NetworkConfigMethod>,
-    method6: Option<NetworkConfigMethod>,
-    comments: Option<String>,
-    comments6: Option<String>,
-    cidr: Option<String>,
-    gateway: Option<String>,
-    cidr6: Option<String>,
-    gateway6: Option<String>,
-    mtu: Option<u64>,
-    bridge_ports: Option<String>,
-    bridge_vlan_aware: Option<bool>,
-    vlan_id: Option<u16>,
-    vlan_raw_device: Option<String>,
-    bond_mode: Option<LinuxBondMode>,
-    bond_primary: Option<String>,
-    bond_xmit_hash_policy: Option<BondXmitHashPolicy>,
-    slaves: Option<String>,
-    delete: Option<Vec<DeletableProperty>>,
-    digest: Option<String>,
-    param: Value,
+    update: InterfaceUpdater,
+    delete: Option<Vec<DeletableInterfaceProperty>>,
+    digest: Option<ConfigDigest>,
 ) -> Result<(), Error> {
-    let _lock = network::lock_config()?;
-
-    let (mut config, expected_digest) = network::config()?;
-
-    pbs_config::detect_modified_configuration_file(digest, &expected_digest)?;
-
-    if gateway.is_some() {
-        check_duplicate_gateway_v4(&config, &iface)?;
-    }
-    if gateway6.is_some() {
-        check_duplicate_gateway_v6(&config, &iface)?;
-    }
-
-    if let Some(dev) = vlan_raw_device
-        .as_deref()
-        .or_else(|| parse_vlan_raw_device_from_name(&iface))
-    {
-        if !config.interfaces.contains_key(dev) {
-            bail!("vlan-raw-device {dev} does not exist");
-        }
-    }
-
-    let interface = config.lookup_mut(&iface)?;
-
-    if let Some(interface_type) = param.get("type") {
-        let interface_type = NetworkInterfaceType::deserialize(interface_type)?;
-        if interface_type != interface.interface_type {
-            bail!(
-                "got unexpected interface type ({:?} != {:?})",
-                interface_type,
-                interface.interface_type
-            );
-        }
-    }
-
-    if let Some(delete) = delete {
-        for delete_prop in delete {
-            match delete_prop {
-                DeletableProperty::Cidr => {
-                    interface.cidr = None;
-                }
-                DeletableProperty::Cidr6 => {
-                    interface.cidr6 = None;
-                }
-                DeletableProperty::Gateway => {
-                    interface.gateway = None;
-                }
-                DeletableProperty::Gateway6 => {
-                    interface.gateway6 = None;
-                }
-                DeletableProperty::Method => {
-                    interface.method = None;
-                }
-                DeletableProperty::Method6 => {
-                    interface.method6 = None;
-                }
-                DeletableProperty::Comments => {
-                    interface.comments = None;
-                }
-                DeletableProperty::Comments6 => {
-                    interface.comments6 = None;
-                }
-                DeletableProperty::Mtu => {
-                    interface.mtu = None;
-                }
-                DeletableProperty::Autostart => {
-                    interface.autostart = false;
-                }
-                DeletableProperty::BridgePorts => {
-                    set_bridge_ports(interface, Vec::new())?;
-                }
-                DeletableProperty::BridgeVlanAware => {
-                    interface.bridge_vlan_aware = None;
-                }
-                DeletableProperty::Slaves => {
-                    set_bond_slaves(interface, Vec::new())?;
-                }
-                DeletableProperty::BondPrimary => {
-                    interface.bond_primary = None;
-                }
-                DeletableProperty::BondXmitHashPolicy => interface.bond_xmit_hash_policy = None,
-            }
-        }
-    }
-
-    if let Some(autostart) = autostart {
-        interface.autostart = autostart;
-    }
-    if method.is_some() {
-        interface.method = method;
-    }
-    if method6.is_some() {
-        interface.method6 = method6;
-    }
-    if mtu.is_some() {
-        interface.mtu = mtu;
-    }
-    if let Some(ports) = bridge_ports {
-        let ports = split_interface_list(&ports)?;
-        set_bridge_ports(interface, ports)?;
-    }
-    if bridge_vlan_aware.is_some() {
-        interface.bridge_vlan_aware = bridge_vlan_aware;
-    }
-    if let Some(slaves) = slaves {
-        let slaves = split_interface_list(&slaves)?;
-        set_bond_slaves(interface, slaves)?;
-    }
-    if let Some(mode) = bond_mode {
-        interface.bond_mode = bond_mode;
-        if bond_primary.is_some() {
-            if mode != LinuxBondMode::ActiveBackup {
-                bail!("bond-primary is only valid with Active/Backup mode");
-            }
-            interface.bond_primary = bond_primary;
-        }
-        if bond_xmit_hash_policy.is_some() {
-            if mode != LinuxBondMode::Ieee802_3ad && mode != LinuxBondMode::BalanceXor {
-                bail!("bond_xmit_hash_policy is only valid with LACP(802.3ad) or balance-xor mode");
-            }
-            interface.bond_xmit_hash_policy = bond_xmit_hash_policy;
-        }
-    }
-
-    if let Some(cidr) = cidr {
-        let (_, _, is_v6) = network::parse_cidr(&cidr)?;
-        if is_v6 {
-            bail!("invalid address type (expected IPv4, got IPv6)");
-        }
-        interface.cidr = Some(cidr);
-    }
-
-    if let Some(cidr6) = cidr6 {
-        let (_, _, is_v6) = network::parse_cidr(&cidr6)?;
-        if !is_v6 {
-            bail!("invalid address type (expected IPv6, got IPv4)");
-        }
-        interface.cidr6 = Some(cidr6);
-    }
-
-    if let Some(gateway) = gateway {
-        let is_v6 = gateway.contains(':');
-        if is_v6 {
-            bail!("invalid address type (expected IPv4, got IPv6)");
-        }
-        interface.gateway = Some(gateway);
-    }
-
-    if let Some(gateway6) = gateway6 {
-        let is_v6 = gateway6.contains(':');
-        if !is_v6 {
-            bail!("invalid address type (expected IPv6, got IPv4)");
-        }
-        interface.gateway6 = Some(gateway6);
-    }
-
-    if comments.is_some() {
-        interface.comments = comments;
-    }
-    if comments6.is_some() {
-        interface.comments6 = comments6;
-    }
-
-    if interface.cidr.is_some() || interface.gateway.is_some() {
-        interface.method = Some(NetworkConfigMethod::Static);
-    } else {
-        interface.method = Some(NetworkConfigMethod::Manual);
-    }
-
-    if interface.cidr6.is_some() || interface.gateway6.is_some() {
-        interface.method6 = Some(NetworkConfigMethod::Static);
-    } else if interface.method6.is_none() {
-        interface.method6 = Some(NetworkConfigMethod::Manual);
-    }
-
-    if vlan_id.is_some() {
-        interface.vlan_id = vlan_id;
-    }
-    if vlan_raw_device.is_some() {
-        interface.vlan_raw_device = vlan_raw_device;
-    }
-
-    network::save_config(&config)?;
-
-    Ok(())
+    proxmox_network_api::update_interface(iface, update, delete, digest)
 }
 
 #[api(
-- 
2.56.0

Re: [PATCH proxmox v2 2/3] fix #6121: network-api: config: add support for `bond-miimon` option

[Gabriel Goller]

On 09.11.2026 15:28, Christoph Heiss wrote:
> Fixes #6121 [0].
> 
> Adds support for the 'bond-miimon' option to the ifupdown(2) config
> writer and parser.
> 
> The option is always written out for bonds, with a default value of 100
> - much like PVE does it.

Umm I understand this but what about the bond-arp-interval (arp-mode). These
properties are mutually exclusive...

Maybe we should allow the user to configure arp mode as well?

> This is needed so that (at least) in active-backup mode, the bond
> correctly switches over to the backup link if the active one goes down.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=6121
> 
> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
> Tested-by: Christian Ebner <c.ebner@proxmox.com>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
>   * no changes
> 
>  proxmox-network-api/src/config/lexer.rs  |  3 +
>  proxmox-network-api/src/config/mod.rs    | 70 ++++++++++++++++++++++++
>  proxmox-network-api/src/config/parser.rs | 23 ++++++++
>  3 files changed, 96 insertions(+)
> 
> [snip]

Re: [PATCH proxmox v2 2/3] fix #6121: network-api: config: add support for `bond-miimon` option

[Christoph Heiss]

Thanks for taking a look!

On 11/9/26 15:57, Gabriel Goller wrote:
> On 09.11.2026 15:28, Christoph Heiss wrote:
>> Fixes #6121 [0].
>>
>> Adds support for the 'bond-miimon' option to the ifupdown(2) config
>> writer and parser.
>>
>> The option is always written out for bonds, with a default value of 100
>> - much like PVE does it.
> 
> Umm I understand this but what about the bond-arp-interval (arp-mode). These
> properties are mutually exclusive...

Interesting note. AFAICS we don't allow configuring that property
through our stack anywhere yet, i.e. neither PVE nor PBS/PDM (didn't
look at PMG specifically, but I assume it's the same there).

> 
> Maybe we should allow the user to configure arp mode as well?

We could possibly implement that separately in the future, if arp-mode
is something regularly needed/set by users. But this is rather
tangentially to this series IMO.

Re: [PATCH proxmox v2 2/3] fix #6121: network-api: config: add support for `bond-miimon` option

[Gabriel Goller]

On 10.11.2026 11:13, Christoph Heiss wrote:
> Thanks for taking a look!
> 
> On 11/9/26 15:57, Gabriel Goller wrote:
> > On 09.11.2026 15:28, Christoph Heiss wrote:
> >> Fixes #6121 [0].
> >>
> >> Adds support for the 'bond-miimon' option to the ifupdown(2) config
> >> writer and parser.
> >>
> >> The option is always written out for bonds, with a default value of 100
> >> - much like PVE does it.
> > 
> > Umm I understand this but what about the bond-arp-interval (arp-mode). These
> > properties are mutually exclusive...
> 
> Interesting note. AFAICS we don't allow configuring that property
> through our stack anywhere yet, i.e. neither PVE nor PBS/PDM (didn't
> look at PMG specifically, but I assume it's the same there).

Hmm yeah thats a reason for leaving it out...

> > Maybe we should allow the user to configure arp mode as well?
> 
> We could possibly implement that separately in the future, if arp-mode
> is something regularly needed/set by users. But this is rather
> tangentially to this series IMO.

Haven't looked at the code, if this is something that can be added easily then
it's fine with me.
I think the arp mode is used quite seldomly anyway..

Maybe @Stefan can add something here?