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

[Christoph Heiss <c.heiss@proxmox.com>]

Thanks for the review!

On 11/9/26 11:15, Christian Ebner wrote:
> one question inline.
> 
> On 9/25/26 2:35 PM, Christoph Heiss wrote:
>> [..]
>> --- a/proxmox-network-api/src/api_types.rs
>> +++ b/proxmox-network-api/src/api_types.rs
>> @@ -228,6 +228,12 @@ pub const NETWORK_INTERFACE_LIST_SCHEMA: Schema =
>>               type: BondXmitHashPolicy,
>>               optional: true,
>>           },
>> +        "bond-miimon": {
>> +            description: "Link monitoring interval in milliseconds 
>> for bond interfaces.",
>> +            type: u8,
> 
> isn't this to restrictive?
> 
> At least the Kernel seem to accept larger values AFAICT:
> - https://github.com/torvalds/linux/blob/master/drivers/net/bonding/ 
> bond_options.c#L149
> - https://github.com/torvalds/linux/blob/master/drivers/net/bonding/ 
> bond_main.c#L6174

Good catch.

At least ifupdown2 says that the value range should be to 0-255 [0] - 
which is what I did go off - but actually never enforces that. I.e. the 
value is never validated against `validrange`, it's just a documentation 
thing at the end.

The netlink interface also indeed accepts a u32 [1] FWIW, so that's the 
right thing to do.

E.g. setting up `/etc/network/interfaces` with

   auto bond0
   iface bond0 inet manual
     bond-slaves nic1 foo
     bond-miimon 5000
     bond-mode 802.3ad

works just fine, `cat /sys/class/net/bond0/bonding/miimon` also shows 
the correct value.

I'll quickly send a v2 with that changed.

[0] 
https://github.com/CumulusNetworks/ifupdown2/blob/master/ifupdown2/addons/bond.py#L80-L85 

[1] 
https://github.com/torvalds/linux/blob/7475ea8b58616b33c04afb976cf0fbaad9dd63fb/drivers/net/bonding/bond_netlink.c#L255

> 
>> +            optional: true,
>> +            default: 100,
>> +        },