netother.cgi: show content of routing table 220

Message ID 45750d21-4ee6-4269-c2b5-56ef4e75c259@ipfire.org
State Superseded
Headers
Series netother.cgi: show content of routing table 220 |

Commit Message

Peter Müller March 7, 2020, 6:46 p.m. UTC
  Since IPsec routing information do not show up in the normal routing
table, also displaying the contents of table 220 on netother.cgi might
be useful for debugging purposes.

Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
---
 html/cgi-bin/netother.cgi | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Michael Tremer March 8, 2020, 11:42 a.m. UTC | #1
Hi,

I appreciate the thought, but I think the implementation might be very confusing.

I think the patch could be improved by:

* Removing the 220 number and simply call it “IPsec VPN Routing Table”

* Not show the box when the table is empty which it will be for all users that are not using IPsec

And since it is basically a static table, I do not see what there is to gain for the user from this. How can this help with debugging?

-Michael

> On 7 Mar 2020, at 18:46, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Since IPsec routing information do not show up in the normal routing
> table, also displaying the contents of table 220 on netother.cgi might
> be useful for debugging purposes.
> 
> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
> ---
> html/cgi-bin/netother.cgi | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/html/cgi-bin/netother.cgi b/html/cgi-bin/netother.cgi
> index dde1b603a..ac02b8148 100644
> --- a/html/cgi-bin/netother.cgi
> +++ b/html/cgi-bin/netother.cgi
> @@ -79,6 +79,12 @@ if ( $querry[0] =~ "fwhits"){
> 	print "<pre>$output</pre>\n";
> 	&Header::closebox();
> 
> +	&Header::openbox('100%', 'left', "$Lang::tr{'routing table entries'} 220");
> +	$output = `/sbin/ip route list table 220`;
> +	$output = &Header::cleanhtml($output,"y");
> +	print "<pre>$output</pre>\n";
> +	&Header::closebox()
> +
> 	&Header::openbox('100%', 'left', $Lang::tr{'arp table entries'});
> 	$output = `/sbin/ip neigh show`;
> 	$output = &Header::cleanhtml($output,"y");
> -- 
> 2.16.4
  
Peter Müller March 8, 2020, 1:15 p.m. UTC | #2
Hello Michael,

> Hi,
> 
> I appreciate the thought, but I think the implementation might be very confusing.
> 
> I think the patch could be improved by:
> 
> * Removing the 220 number and simply call it “IPsec VPN Routing Table”

Okay, good point.

> 
> * Not show the box when the table is empty which it will be for all users that are not using IPsec

ACK.

> 
> And since it is basically a static table, I do not see what there is to gain for the user from this. How can this help with debugging?

For the same reasons we display contents of the routing table, I guess. The user is
able to do quick plausibility checks over it, without digging/using the search
engine of his/hers least distrust for the command that shows him the IPsec routing information.

Thanks, and best regards,
Peter Müller

> 
> -Michael
> 
>> On 7 Mar 2020, at 18:46, Peter Müller <peter.mueller@ipfire.org> wrote:
>>
>> Since IPsec routing information do not show up in the normal routing
>> table, also displaying the contents of table 220 on netother.cgi might
>> be useful for debugging purposes.
>>
>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>> ---
>> html/cgi-bin/netother.cgi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/html/cgi-bin/netother.cgi b/html/cgi-bin/netother.cgi
>> index dde1b603a..ac02b8148 100644
>> --- a/html/cgi-bin/netother.cgi
>> +++ b/html/cgi-bin/netother.cgi
>> @@ -79,6 +79,12 @@ if ( $querry[0] =~ "fwhits"){
>> 	print "<pre>$output</pre>\n";
>> 	&Header::closebox();
>>
>> +	&Header::openbox('100%', 'left', "$Lang::tr{'routing table entries'} 220");
>> +	$output = `/sbin/ip route list table 220`;
>> +	$output = &Header::cleanhtml($output,"y");
>> +	print "<pre>$output</pre>\n";
>> +	&Header::closebox()
>> +
>> 	&Header::openbox('100%', 'left', $Lang::tr{'arp table entries'});
>> 	$output = `/sbin/ip neigh show`;
>> 	$output = &Header::cleanhtml($output,"y");
>> -- 
>> 2.16.4
>
  
Michael Tremer March 8, 2020, 1:35 p.m. UTC | #3
Okay. Will you update the patch accordingly, please?

-Michael

> On 8 Mar 2020, at 13:15, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Hello Michael,
> 
>> Hi,
>> 
>> I appreciate the thought, but I think the implementation might be very confusing.
>> 
>> I think the patch could be improved by:
>> 
>> * Removing the 220 number and simply call it “IPsec VPN Routing Table”
> 
> Okay, good point.
> 
>> 
>> * Not show the box when the table is empty which it will be for all users that are not using IPsec
> 
> ACK.
> 
>> 
>> And since it is basically a static table, I do not see what there is to gain for the user from this. How can this help with debugging?
> 
> For the same reasons we display contents of the routing table, I guess. The user is
> able to do quick plausibility checks over it, without digging/using the search
> engine of his/hers least distrust for the command that shows him the IPsec routing information.
> 
> Thanks, and best regards,
> Peter Müller
> 
>> 
>> -Michael
>> 
>>> On 7 Mar 2020, at 18:46, Peter Müller <peter.mueller@ipfire.org> wrote:
>>> 
>>> Since IPsec routing information do not show up in the normal routing
>>> table, also displaying the contents of table 220 on netother.cgi might
>>> be useful for debugging purposes.
>>> 
>>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>>> ---
>>> html/cgi-bin/netother.cgi | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/html/cgi-bin/netother.cgi b/html/cgi-bin/netother.cgi
>>> index dde1b603a..ac02b8148 100644
>>> --- a/html/cgi-bin/netother.cgi
>>> +++ b/html/cgi-bin/netother.cgi
>>> @@ -79,6 +79,12 @@ if ( $querry[0] =~ "fwhits"){
>>> 	print "<pre>$output</pre>\n";
>>> 	&Header::closebox();
>>> 
>>> +	&Header::openbox('100%', 'left', "$Lang::tr{'routing table entries'} 220");
>>> +	$output = `/sbin/ip route list table 220`;
>>> +	$output = &Header::cleanhtml($output,"y");
>>> +	print "<pre>$output</pre>\n";
>>> +	&Header::closebox()
>>> +
>>> 	&Header::openbox('100%', 'left', $Lang::tr{'arp table entries'});
>>> 	$output = `/sbin/ip neigh show`;
>>> 	$output = &Header::cleanhtml($output,"y");
>>> -- 
>>> 2.16.4
>>
  

Patch

diff --git a/html/cgi-bin/netother.cgi b/html/cgi-bin/netother.cgi
index dde1b603a..ac02b8148 100644
--- a/html/cgi-bin/netother.cgi
+++ b/html/cgi-bin/netother.cgi
@@ -79,6 +79,12 @@  if ( $querry[0] =~ "fwhits"){
 	print "<pre>$output</pre>\n";
 	&Header::closebox();
 
+	&Header::openbox('100%', 'left', "$Lang::tr{'routing table entries'} 220");
+	$output = `/sbin/ip route list table 220`;
+	$output = &Header::cleanhtml($output,"y");
+	print "<pre>$output</pre>\n";
+	&Header::closebox()
+
 	&Header::openbox('100%', 'left', $Lang::tr{'arp table entries'});
 	$output = `/sbin/ip neigh show`;
 	$output = &Header::cleanhtml($output,"y");