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

[Tom Rymes <tom@rymes.net>]

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



> 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 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?

I think I’m missing a piece of this puzzle.

Tom