[RFC] netifd: add support for dhcp fallback to static ip

[Roman Yeryomin]

On 2024-07-14 14:11, Felix Fietkau wrote:
> On 14.07.24 11:47, Roman Yeryomin wrote:
>> On 2024-07-14 10:42, Felix Fietkau wrote:
>>> On 14.07.24 02:34, Roman Yeryomin wrote:
>>>> It is pretty common use case for a network device to be configured
>>>> as DHCP client but having some fallback static IP address where it
>>>> would be reachable for, e.g., configuration.
>>>> 
>>>> This reacts on udhcpc events and sets/removes ipaddr which is
>>>> configured on an iface from network config.
>>>> This means that for an interface with proto set to dhcp but with
>>>> fallback static IP disabled (ipaddr parameter is not set in config)
>>>> cost of this feature is one uci call.
>>>> If static fallback is enabled (ipaddr is present) then the cost is
>>>> 3 uci calls and one ip call for each deconfig/renew/bound udhcpc
>>>> event.
>>>> It is not nothing but to me it seem like it's a very small overhead
>>>> for such a convenience and I think it should be OpenWrt default at
>>>> least for targets with one ethernet port.
>>>> 
>>>> Signed-off-by: Roman Yeryomin <roman at advem.lv>
>>>> ---
>>>> diff --git 
>>>> a/package/network/config/netifd/files/etc/udhcpc.user.d/fallbackip.sh 
>>>> b/package/network/config/netifd/files/etc/udhcpc.user.d/fallbackip.sh
>>>> new file mode 100644
>>>> index 0000000000..79984e9a2f
>>>> --- /dev/null
>>>> +++ 
>>>> b/package/network/config/netifd/files/etc/udhcpc.user.d/fallbackip.sh
>>>> @@ -0,0 +1,23 @@
>>>> +#!/bin/sh
>>>> +
>>>> +[ -z "$1" ] && echo "Error: should be run by udhcpc" && exit 1
>>>> +
>>>> +ipaddr=$(uci get network.$INTERFACE.ipaddr)
>>>> +[ -z "$ipaddr" ] && exit 0
>>>> +
>>>> +ifname=$(uci get network.$INTERFACE.ifname)
>>>> +[ -z "$ifname" ] && exit 0
>>>> +
>>>> +netmask=$(uci get network.$INTERFACE.netmask)
>>>> +[ -z "$netmask" ] && netmask=24
>>>> +
>>>> +case "$1" in
>>>> +	deconfig)
>>>> +		ip ad add $ipaddr/$netmask dev $ifname
>>>> +	;;
>>>> +	renew|bound)
>>>> +		ip ad del $ipaddr dev $ifname
>>>> +	;;
>>>> +esac
>>>> +
>>>> +exit 0
>>> 
>>> Here's my take on this patch:
>>> 
>>> 1. The feature should be opt-in via an extra config option, since it
>>> changes the behavior in a meaningful way that might be problematic 
>>> for
>>> some users.
>>> 2. IP addresses should be set from within netifd, e.g. via the same
>>> mechanism that the dhcp script uses to bring up the interface.
>>> 3. IP settings should not be read directly from uci, but passed down
>>> from the netifd proto handler, e.g. via env vars.
>>> 
>> 
>> Agree on 2. and 3.
>> Can't say I agree on 1. -- could you please give an example in which
>> situation it could be problematic?
> 
> If you use the ipaddr to request a specific address from the DHCP
> server, but don't want the link to be up when the server is not
> reachable.

Ah, got it, let me send updated version then with 'fallbackip' option.

Regards,
Roman