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

[Christian Ebner <c.ebner@proxmox.com>]

On 11/9/26 2:28 PM, Christoph Heiss wrote:
> 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
The actual limit enforced is i32::MAX afaict, so best to store as u32 
and check for i32::MAX not being exceeded?