Feature #2629
closed
Changed firewall log to show the applied rule description directly on screen, also layout optimization for "Show raw filter logs".
90%
Description
Changed firewall log to show the applied rule description directly on screen, also layout optimization for "Show raw filter logs".
Implement as in attached screenshot, and pullrequest here: [https://github.com/bsdperimeter/pfsense/pull/218].
Files
Updated by Jim Pingle over 12 years ago
I'm not opposed to having that information readily accessible, but that format isn't very easy to read. I don't know that having it in its own column is really necessary, at least by default. That could be a configurable behavior via a checkbox setting on the log settings tab. For someone with a very large ruleset, or a very large number of log lines to show, this could slow things down considerably, so having it be an optional toggle would be great.
Also the log view is already quite wide, especially when viewing entries with IPv6 IPs, this would pretty much guarantee it spills outside of the background of the page.
Consider switching it to:
(a) only show the rule number in its own column
(b) when you click somewhere on the line (or maybe mouseover the rule number or the entire line), it would show the full description in a domTT style window or expand into a new line below the log entry.
Updated by Pi Ba over 12 years ago
- File pfsense.localdomain - Status System logs Firewall - Google Chrome_2012-09-09_21-50-01.png pfsense.localdomain - Status System logs Firewall - Google Chrome_2012-09-09_21-50-01.png added
Would layout attached as new screenshot be ok? !New layout proposal!
As for the performance, there is a 2000 max row limit on the showing of the firewall lines. I currently have 114 firewall rules according to pfTop. And >1000 lines of firewall logging. And cant notice the difference in speed when viewing the page. I have buffered the rules in a php variable, which supposedly is close to O(1) speed, so its not like a call to "pfctl -vvsr | grep..." is needed for every line.. How much rules would be considered a very large set?
Updated by Jim Pingle over 12 years ago
There are people with thousands of rules in the evaluated ruleset, and there are also people on very slow hardware (think alix or similar), so what's fast for you may not be fast for others. The way you did it should be fast enough, but IMHO the behavior should still be optional and off by default. It's nice to have it available, but it may not be something everyone wants/needs to see.
The new format would be OK, just indent the second line so that it starts under the interface column, that way the action and timestamp columns are not visually interrupted by other info. (It makes it much harder to follow quickly)
Updated by Pi Ba over 12 years ago
Thanks for your comments.
Made a few modifications and added a setting to allow for keeping old behavior.
And made a new pull request: [https://github.com/bsdperimeter/pfsense/pull/220]
#-Added a setting for configuring the firewall log to either:
-Not load descriptions
-Show descriptions in a column
-Show descriptions on a second row (after a click on 'show descriptions')
#-'fixed' a few (x)html validation issues..
#-made quick carp firewall logging optional as part of the log default rules setting
Updated by Jim Pingle over 12 years ago
- Status changed from New to Feedback
I approved the request, it looks good now, thanks!
The show/hide toggle is also a good addition, and the three-way option gives a nice configurable choice. I tried it each way and it worked as expected.
Though was it you intent with the show/hide that it starts out hidden? It seems to me that the first time I loaded the page it was expanded, but after clicking the show/hide toggle once it always started with them hidden.
Some other tweaks you might consider for a future refinement:- In the two-line mode, get rid of the cell divider line
- Not exactly related, but it would be easier to see the description/rule relationship if alternating rows had just the slightly different background color, like one white, and the next one an extremely light grey.
Updated by Pi Ba about 12 years ago
Think this issue can be closed now.
Im happy with how it works now. And it has an option for pretty much everyone. -embedded systems without description, -widescreen users with extra column, -other users with extra row. Sorting now also works properly.
Updated by Jim Pingle about 12 years ago
- Status changed from Feedback to Resolved