From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Adolf Belka (ipfire)" To: development@lists.ipfire.org Subject: Re: [PATCH] bug#10629: Prevent dynamic and fixed leases overlapping Date: Sun, 21 Feb 2021 15:02:18 +0100 Message-ID: <21d29657-9206-a6ed-5b39-80ce1a572a9b@ipfire.org> In-Reply-To: <2ADA7D9D-E567-48E5-B77E-6047BA988836@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5240808405453560944==" List-Id: --===============5240808405453560944== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael & Tom On 19/02/2021 19:57, Michael Tremer wrote: > Hi, >=20 > Great discussion everyone. >=20 > I guess we have two problems on our hand: >=20 > 1) How do we do this right? >=20 > 2) How do we make it compatible with the current UI? >=20 > Rewriting the whole UI is a big thing and changing anything *will* break pe= ople=E2=80=99s networks. That is something we cannot do. >=20 > In IPFire 3, I added the option to add multiple pools and in those add mult= iple 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 i= dea, or if I got it wrong: >=20 > https://git.ipfire.org/?p=3Dnetwork.git;a=3Dblob;f=3Dsrc/functions/funct= ions.dhcpd;h=3D3b1214fb937d0efd8ea8c3a4d4113f13bc3b5d52;hb=3DHEAD >=20 > Maybe we can use this as a study for question 1) because we do not need to = care about compatibility. >=20 > Secondly, we need to look at IPFire 2 and *if* this can be done right witho= ut breaking anything. I think the suggestion to only have a warning for existing overlapped=20 leases and an error for creating new ones is a good solution. It allows=20 existing systems with overlapped IP's to continue running but makes the=20 issue visible. The error message to prevent creating new overlapped IP's=20 makes sure that the problem is not extended further. Regards, Adolf. >=20 > -Michael >=20 >> On 19 Feb 2021, at 11:37, Adolf Belka (ipfire) = wrote: >> >> Hi Tom, >> >> On 18/02/2021 23:40, Tom Rymes wrote: >>>> On Feb 18, 2021, at 12:08 PM, Adolf Belka (ipfire-dev) wrote: >>>> >>>> =EF=BB=BFHi All, >>>> >>>>> On 18/02/2021 17:05, Tom Rymes wrote: >>>>> >>>>>>> On Feb 18, 2021, at 10:29 AM, Bernhard Bitsch wrote: >>>>>> >>>>>>> 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 a= n 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 whi= ch were the original existing and which are the new overlapped leases without= having a parameter stored in a file that counts which leases were overlappin= g 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 s= hould 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 bac= kend and not tell the user. So in the configuration instead of writing 192.16= 8.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 leas= es. >>>>>>> >>>>>>> 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 poo= ls are used with a slight different semantic in dhcpd. This difference may in= crease in future. >>>>>> Not mentioning the effort to split the set of possible IP addresses an= d 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 ha= ndle 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 th= e benefit 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 chan= ge a host=E2=80=99s IP. >>>>> >>>>> Tom >>>> >>>> I investigated this when I picked up this bug and I came to the same con= clusion as Bernhard. >>>> >>>> dhcpd has separate address pools so that users can have a range of dynam= ic IP's that have tailored options for specific clients. >>>> >>>> You can then have a specific address pool for a group of clients that fo= r instance have a short default lease time and specific ntp and dns server ad= dresses etc. Another address pool can have a longer default lease time and di= fferent 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 exist= ing 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 th= at 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 t= hat 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 imple= ment. >>>> >>>> >>>> Regards, >>>> >>>> Adolf. >>>> >>> Adolf, >>> Isn=E2=80=99t the only way to make these multiple pools work to manually = specify which MAC addresses qualify for each pool? Doesn=E2=80=99t 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 wi= th host declarations that don't have fixed IP addresses defined to get dynami= c 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 addres= ses. 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 clien= ts that don't have IPs preset getting another dynamic IP range and known clie= nts with defined IPs getting those IPs. You can then have settings like DNS s= erver, nip server, default lease time etc defined differently at a global lev= el and for each address pool. >> >> >> Another way is to create a class based on the option host-name if the clie= nt has been set up to send it. You can select all those clients that have par= t 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 hos= tname starting with foobar. Then you can create a pool that only applies to m= embers 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=E2=80=99m missing a piece of this puzzle. >>> Tom >=20 --=20 Sent from my laptop --===============5240808405453560944==--