From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Adolf Belka (ipfire-dev)" To: development@lists.ipfire.org Subject: Re: [PATCH] bug#10629: Prevent dynamic and fixed leases overlapping Date: Thu, 18 Feb 2021 18:08:27 +0100 Message-ID: <52e26906-7210-325f-5819-51777bf9a95e@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6153233332547631289==" List-Id: --===============6153233332547631289== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi All, On 18/02/2021 17:05, Tom Rymes wrote: > >> On Feb 18, 2021, at 10:29 AM, Bernhard Bitsch w= rote: >> >>> Gesendet: Donnerstag, 18. Februar 2021 um 16:18 Uhr >>> Von: "Michael Tremer" >>> >>>>> On 18 Feb 2021, at 14:01, Adolf Belka (ipfire-dev) 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 ex= isting dynamic range that already overlaps with two fixed leases so that it n= ow overlaps with four fixed leases. It will be difficult to determine which w= ere the original existing and which are the new overlapped leases without hav= ing a parameter stored in a file that counts which leases were overlapping wh= en 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 shoul= d 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 a= re used with a slight different semantic in dhcpd. This difference may increa= se 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 o= pened 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 be= nefit of allowing a user to keep a host=E2=80=99s address the same, which can= be very helpful in instances where a lot of work may be required to change a= host=E2=80=99s IP. > > Tom I investigated this when I picked up this bug and I came to the same conclusi= on 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 ins= tance have a short default lease time and specific ntp and dns server address= es etc. Another address pool can have a longer default lease time and differe= nt ntp and dns servers etc pfSense has implemented Additional Pools into its WUI and that could be somet= hing 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 h= ave now of being limited in what we do because the code has already been modi= fied in a different direction that could create problems for those existing u= sers, so we end up deciding that Additional Pools is too difficult to do beca= use 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 do= n'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 so= me future time is not obstructed. With the current bug fix we will have warni= ngs for existing users that what they are doing has problems but it won't blo= ck 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 futu= re improvements difficult or impossible to implement. Regards, Adolf. --===============6153233332547631289==--