Hi,

First of all, thank you for this quick response.

I had previously written to Michael TREMER about the problem I was having pushing patches for IPFire with Git. He helped me find the solution. I know, having written to him yesterday, that he is not currently available.

I also spoke with Stéphane PAUTREL (who supports French translation updates), about the developments that I wanted to share with the IPFire user community.

Concerning this patch (dhcpi.cgi), in fact now that you have pointed it out to me, I see the errors I made. I hope that I have corrected them...

I will have other developments but I want to finish the "process" of the first patch, to then send the following ones "one by one".

I have written a detailed documentation of this process, for the moment in French, which I will later translate into English, to propose it for publication on the IPFire wiki...

I also think that Stéphane PAUTREL will also have patches to offer.

Have a good day, Sébastien

Le 10/11/2023 à 19:26, Adolf Belka a écrit :

Hi Sebastien,

Thanks for the patch. I have tested this out and found a few quirks with it.

The table for that section is set at 100%. The original columns then added up to 100%.

Your patch added in an additional column at 20% making the total width now 120% but the overall table is still set at 100%. I think you need to adjust the width of each of the columns including the new column to come to a total of 100%.

In your line for the heading you have a closing <a/> tag for a hyperlink but there is no equivalent opening <a> tag in the line and no hyperlink defined. I am not sure if this is giving any problems but the closing <a/> tag is not needed there as there is no hyperlink.

When evaluating your patch I found that your Hostname column expands to keep the hostname on one line. This forces the other columns (next-server, filename and root path) to be squeezed up and made into multiple line entries. I think the hostname column should also be allowed to be a multi line entry to balance the columns up.

Eventually the other columns reach a min width and then the table expands outside of the overall frame of the IPFire page. This may be due to the columns totalling more than 100% but I am not familiar enough with html coding to be certain of that.

The line

if (!$rname || $rname eq "") { $rname = $Lang::tr{'lookup failed'}; }

has a Language lookup but the phrase 'lookup failed' has not been added into the English, German or French Language files.

Regards,

Adolf.

On 09/11/2023 22:36, Sebastien GISLAIN wrote:

...

---

html/cgi-bin/dhcp.cgi | 7 +++++++   1 file changed, 7 insertions(+)

diff --git a/html/cgi-bin/dhcp.cgi b/html/cgi-bin/dhcp.cgi index f45703f0b..aabf565d7 100755 --- a/html/cgi-bin/dhcp.cgi +++ b/html/cgi-bin/dhcp.cgi @@ -21,6 +21,7 @@     use strict;   use experimental 'smartmatch'; +use IO::Socket;     # enable only the following on debugging purpose   #use warnings; @@ -1009,6 +1010,7 @@ print <<END   <tr>       <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FETHER'><b>$Lang::tr{'mac address'}</b></a></th>       <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FIPADDR'><b>$Lang::tr{'ip address'}</b></a></th> +    <th width='20%' align='center'><b>$Lang::tr{'hostname'}</b></a></th>       <th width='15%' align='center'><b>$Lang::tr{'remark'}</b></th>       <th width='15%' class='boldbase' align='center'><b>next-server</b></th>       <th width='15%' class='boldbase' align='center'><b>filename</b></th> @@ -1106,9 +1108,14 @@ foreach my $line (@current2) {       $TAG4 = "class='red'" if ($dhcpsettings{'KEY2'} ne $key);       }   +    # resolved name (if exists) +    my $iaddr = inet_aton($temp[1]); +    my $rname = gethostbyaddr($iaddr, AF_INET); +    if (!$rname || $rname eq "") { $rname = $Lang::tr{'lookup failed'}; }       print <<END   <td align='center' $col>$TAG2$temp[0]$TAG3</td>   <td align='center' $TAG4 $col>$TAG0$temp[1]$TAG1</td> +<td align='center' $col>$rname&nbsp;</td>   <td align='center' $col>$temp[6]&nbsp;</td>   <td align='center' $col>$temp[3]&nbsp;</td>   <td align='center' $col>$temp[4]&nbsp;</td>

Hi,

I understand your reluctance for this development.

On the one hand, the resolution only concerns local hosts.

On the other hand, in my personal IPFire, with a Class C subnet on green network, I have 156 fixed leases and I don't see any slowdown.

Do you know of any IPFire deployments with class B or A subnets?

Additionally, searching into the local hosts file requires more complex development...

Have a nice day, Sébastien

P.S.    Especially since I had prepared an update on the firewall log page (firewalllog.dat) which was even slower than that of the DHCP server page (dhcp.cgi)! 8'-(

Je comprends ta réticence pour cette évolution. D'une part, la résolution ne porte que sur des hôtes locaux. D'autre part, dans mon IPFire personnel, avec un sous-réseau de classe C, j'ai 156 baux fixes et je ne constate pas de ralentissement. Connais-tu des déploiements d'IPFire avec des sous-réseaux de classe B ou A ?

Le 24/11/2023 à 14:34, Michael Tremer a écrit :

Hello Sebastien,

Thank you very much for submitting this patch. However, I do have some concerns about this being merged.

If you only have a couple of leases, everything is fine, but there are people who have hundreds of DHCP clients on their network, and it has been reported before that this page is loading extremely slow. Adding a DNS request to this will probably make loading the page many times slower than it already is.

And to consider the performance hit, I do not quite know what the benefit is of showing this. Should we not rather parse the DHCP leases file and show the hostname that the client has submitted? That would show the same result, but it would be a lot faster.

-Michael

...

On 9 Nov 2023, at 21:36, Sebastien GISLAINsebastien.gislain@free.fr wrote:

---

html/cgi-bin/dhcp.cgi | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/html/cgi-bin/dhcp.cgi b/html/cgi-bin/dhcp.cgi index f45703f0b..aabf565d7 100755 --- a/html/cgi-bin/dhcp.cgi +++ b/html/cgi-bin/dhcp.cgi @@ -21,6 +21,7 @@

use strict; use experimental 'smartmatch'; +use IO::Socket;

# enable only the following on debugging purpose #use warnings; @@ -1009,6 +1010,7 @@ print <<END

<tr> <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FETHER'><b>$Lang::tr{'mac address'}</b></a></th> <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FIPADDR'><b>$Lang::tr{'ip address'}</b></a></th> + <th width='20%' align='center'><b>$Lang::tr{'hostname'}</b></a></th> <th width='15%' align='center'><b>$Lang::tr{'remark'}</b></th> <th width='15%' class='boldbase' align='center'><b>next-server</b></th> <th width='15%' class='boldbase' align='center'><b>filename</b></th> @@ -1106,9 +1108,14 @@ foreach my $line (@current2) { $TAG4 = "class='red'" if ($dhcpsettings{'KEY2'} ne $key); }

• # resolved name (if exists)
• my $iaddr = inet_aton($temp[1]);
• my $rname = gethostbyaddr($iaddr, AF_INET);
• if (!$rname || $rname eq "") { $rname = $Lang::tr{'lookup failed'}; } print <<END

<td align='center' $col>$TAG2$temp[0]$TAG3</td> <td align='center' $TAG4 $col>$TAG0$temp[1]$TAG1</td> +<td align='center' $col>$rname&nbsp;</td> <td align='center' $col>$temp[6]&nbsp;</td> <td align='center' $col>$temp[3]&nbsp;</td> <td align='center' $col>$temp[4]&nbsp;</td> -- 2.39.2

Hello,

My evolution only concerns the table of fixed leases and not dynamic leases.

I don't understand: the *DHCP server *attributes local IP addresses, which are generally for "private" IP address classes (see attachment).

Why will the DNS resolver look for Internet domain names for these "local" IP addresses?

Regarding firewalllog.dat: it's a reformatting of the table with DNS resolution... of Internet IP addresses! :-O (see attachment)

Regards, Sébastien

Le 28/11/2023 à 11:40, Michael Tremer a écrit :

Hello,

...

On 28 Nov 2023, at 06:33, Sébastien GISLAINsebastien.gislain@gmail.com wrote:

Hi,

I understand your reluctance for this development.

On the one hand, the resolution only concerns local hosts.

It is not guaranteed that any DNS lookups will only happen locally, but indeed rather likely.

...

On the other hand, in my personal IPFire, with a Class C subnet on green network, I have 156 fixed leases and I don't see any slowdown.

Do you know of any IPFire deployments with class B or A subnets?

We have lots of users that have a couple of thousand clients behind the firewall, a couple of hundreds isn’t really that rare.

...

Additionally, searching into the local hosts file requires more complex development...

Not the hosts file… The DHCP leases file. That is being parsed further down showing a long table. It contains hostnames that the clients have submitted and you could match them by their MAC/IP address. That way, you only parse one file and you get the hostname that the client thinks it has.

...

Have a nice day, Sébastien

P.S. Especially since I had prepared an update on the firewall log page (firewalllog.dat) which was even slower than that of the DHCP server page (dhcp.cgi)! 8'-(

What would that do?

...

Je comprends ta réticence pour cette évolution. D'une part, la résolution ne porte que sur des hôtes locaux. D'autre part, dans mon IPFire personnel, avec un sous-réseau de classe C, j'ai 156 baux fixes et je ne constate pas de ralentissement. Connais-tu des déploiements d'IPFire avec des sous-réseaux de classe B ou A ?

Le 24/11/2023 à 14:34, Michael Tremer a écrit :

...

Hello Sebastien,

Thank you very much for submitting this patch. However, I do have some concerns about this being merged.

If you only have a couple of leases, everything is fine, but there are people who have hundreds of DHCP clients on their network, and it has been reported before that this page is loading extremely slow. Adding a DNS request to this will probably make loading the page many times slower than it already is.

And to consider the performance hit, I do not quite know what the benefit is of showing this. Should we not rather parse the DHCP leases file and show the hostname that the client has submitted? That would show the same result, but it would be a lot faster.

-Michael

...

On 9 Nov 2023, at 21:36, Sebastien GISLAINsebastien.gislain@free.fr wrote:

---

html/cgi-bin/dhcp.cgi | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/html/cgi-bin/dhcp.cgi b/html/cgi-bin/dhcp.cgi index f45703f0b..aabf565d7 100755 --- a/html/cgi-bin/dhcp.cgi +++ b/html/cgi-bin/dhcp.cgi @@ -21,6 +21,7 @@

use strict; use experimental 'smartmatch'; +use IO::Socket;

# enable only the following on debugging purpose #use warnings; @@ -1009,6 +1010,7 @@ print <<END

<tr> <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FETHER'><b>$Lang::tr{'mac address'}</b></a></th> <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?FIPADDR'><b>$Lang::tr{'ip address'}</b></a></th> + <th width='20%' align='center'><b>$Lang::tr{'hostname'}</b></a></th> <th width='15%' align='center'><b>$Lang::tr{'remark'}</b></th> <th width='15%' class='boldbase' align='center'><b>next-server</b></th> <th width='15%' class='boldbase' align='center'><b>filename</b></th> @@ -1106,9 +1108,14 @@ foreach my $line (@current2) { $TAG4 = "class='red'" if ($dhcpsettings{'KEY2'} ne $key); }

• # resolved name (if exists)
• my $iaddr = inet_aton($temp[1]);
• my $rname = gethostbyaddr($iaddr, AF_INET);
• if (!$rname || $rname eq "") { $rname = $Lang::tr{'lookup failed'}; }

print <<END

<td align='center' $col>$TAG2$temp[0]$TAG3</td> <td align='center' $TAG4 $col>$TAG0$temp[1]$TAG1</td> +<td align='center' $col>$rname&nbsp;</td> <td align='center' $col>$temp[6]&nbsp;</td> <td align='center' $col>$temp[3]&nbsp;</td> <td align='center' $col>$temp[4]&nbsp;</td> -- 2.39.2