Re: [PATCH] bug#10629: Prevent dynamic and fixed leases overlapping

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 7393 bytes --]

Hi,

Great discussion everyone.

I guess we have two problems on our hand:

1) How do we do this right?

2) How do we make it compatible with the current UI?

Rewriting the whole UI is a big thing and changing anything *will* break people’s networks. That is something we cannot do.

In IPFire 3, I added the option to add multiple pools and in those add multiple ranges of IP addresses that can be assigned. Maybe those who understood how this is supposed to work can review this and tell me if this was a good idea, or if I got it wrong:

  https://git.ipfire.org/?p=network.git;a=blob;f=src/functions/functions.dhcpd;h=3b1214fb937d0efd8ea8c3a4d4113f13bc3b5d52;hb=HEAD

Maybe we can use this as a study for question 1) because we do not need to care about compatibility.

Secondly, we need to look at IPFire 2 and *if* this can be done right without breaking anything.

-Michael

> On 19 Feb 2021, at 11:37, Adolf Belka (ipfire) <adolf.belka(a)ipfire.org> wrote:
> 
> Hi Tom,
> 
> On 18/02/2021 23:40, Tom Rymes wrote:
>>> On Feb 18, 2021, at 12:08 PM, Adolf Belka (ipfire-dev) <adolf.belka(a)ipfire.org> wrote:
>>> 
>>> Hi All,
>>> 
>>>> On 18/02/2021 17:05, Tom Rymes wrote:
>>>> 
>>>>>> On Feb 18, 2021, at 10:29 AM, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
>>>>> 
>>>>>> Gesendet: Donnerstag, 18. Februar 2021 um 16:18 Uhr
>>>>>> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
>>>>>> 
>>>>>>>> On 18 Feb 2021, at 14:01, Adolf Belka (ipfire-dev) <adolf.belka(a)ipfire.org> wrote:
>>>>>>> On 18/02/2021 14:06, Michael Tremer wrote:
>>>>>>> 
>>>> [snip]
>>>> 
>>>>>>> The only thing that would be more difficult is if the user expands an existing dynamic range that already overlaps with two fixed leases so that it now overlaps with four fixed leases. It will be difficult to determine which were the original existing and which are the new overlapped leases without having a parameter stored in a file that counts which leases were overlapping when the Core Update upgrade is carried out. That could be done but I think the simplest will be that any overlap due to a change in the dynamic range should just get a warning and not an error. Does that sound okay.
>>>>>> Oh yeah, difficult question.
>>>>>> 
>>>>>> I did not assume that this was very dynamic before.
>>>>>> 
>>>>>> Potentially it is a good solution to simply split the pool in the backend and not tell the user. So in the configuration instead of writing 192.168.0.100-192.168.0.200, you would add a break for every static lease:
>>>>>> 
>>>>>> * 192.168.0.100 - 192.168.0.105
>>>>>> * 192.168.0.107 - 192.168.0.112
>>>>>> * 192.168.0.114 - 192.168.0.200
>>>>>> 
>>>>>> In this example, 192.168.0.106 and 192.168.0.113 would be static leases.
>>>>>> 
>>>>>> That would make the solution transparent for the user, but a pain for the developer.
>>>>>> 
>>>>> I do not think, that pools are right tool for the problem. Address pools are used with a slight different semantic in dhcpd. This difference may increase in future.
>>>>> Not mentioning the effort to split the set of possible IP addresses and to verify this process.
>>>> [snip]
>>>> 
>>>> This was my original suggestion for how to handle this, and at the time I opened the big, it was my understanding that this was the proper way to handle this. Others have arrived at a different conclusion, and to me it seems like more of a style thing than a functional thing.
>>>> 
>>>> Using multiple range definitions to exclude fixed addresses also has the benefit of allowing a user to keep a host’s address the same, which can be very helpful in instances where a lot of work may be required to change a host’s IP.
>>>> 
>>>> Tom
>>> 
>>> I investigated this when I picked up this bug and I came to the same conclusion as Bernhard.
>>> 
>>> dhcpd has separate address pools so that users can have a range of dynamic IP's that have tailored options for specific clients.
>>> 
>>> You can then have a specific address pool for a group of clients that for instance have a short default lease time and specific ntp and dns server addresses etc. Another address pool can have a longer default lease time and different ntp and dns servers etc
>>> 
>>> pfSense has implemented Additional Pools into its WUI and that could be something that IPFire looks at later. If we have created already split pools with no specified options then we have to decide how to deal with these when we do move to additional pools. We are likely to run into the same problem as we have now of being limited in what we do because the code has already been modified in a different direction that could create problems for those existing users, so we end up deciding that Additional Pools is too difficult to do because it could disrupt existing users. They might not even know they have split pools. Then we would have to figure out how to deal with split pools that don't want to be seen as Additional Pools and with those that do.
>>> 
>>> If we stay as we are with a single pool then moving to Additional Pools at some future time is not obstructed. With the current bug fix we will have warnings for existing users that what they . That pool then covers are doing has problems but it won't block them from using their existing fixed leases that overlap with the dynamic range. If they cannot change they at least know that they are doing something wrong and could have problems. We don't create an obstruction that make future improvements difficult or impossible to implement.
>>> 
>>> 
>>> Regards,
>>> 
>>> Adolf.
>>> 
>> Adolf,
>> Isn’t the only way to make these multiple pools work to manually specify which MAC addresses qualify for each pool? Doesn’t that make them fixed leases?
> 
> For one pool you have allow unknown-clients. That pool then acts like the normal dynamic allocation of IPs.
> The second pool you have deny unknown-clients. This then allows clients with host declarations that don't have fixed IP addresses defined to get dynamic IPs from a different range.
> Then you still have the host declarations where you define the IP address which are then like the traditional IPFire Fixed Leases.
> 
> The second pool does require mac addresses to be defined but not IP addresses. Currently IPFire only has host declarations with both mac address and IP address.
> 
> So you then have unknown clients getting one dynamic IP range, known clients that don't have IPs preset getting another dynamic IP range and known clients with defined IPs getting those IPs. You can then have settings like DNS server, nip server, default lease time etc defined differently at a global level and for each address pool.
> 
> 
> Another way is to create a class based on the option host-name if the client has been set up to send it. You can select all those clients that have part of their host-name defined in a certain way. i.e. you have clients that are named foobar-xyz then you can create a class for all clients that have a hostname starting with foobar. Then you can create a pool that only applies to members of that class.
> 
> 
> None of the above can be done currently with the WUI but would be possible with dhcpd if someone wanted to implement it into the WUI.
> 
> Regards,
> 
> Adolf.
> 
>> I think I’m missing a piece of this puzzle.
>> Tom