Firewall rules hitcount for pfSense 2.1.5 and 2.2.4

brandur

I noticed that 2.3 is nearing Beta stage. Has your great improvement been merged with 2.3(and approved)?

marcelloc

I noticed that 2.3 is nearing Beta stage. Has your great improvement been merged with 2.3(and approved)?

unfortunately no. :(

brandur

@brandur:

I noticed that 2.3 is nearing Beta stage. Has your great improvement been merged with 2.3(and approved)?

unfortunately no. :(

That is just very sad news :'(
Was there any particular reason it didn't "make it"?

(Then the question becomes. Are you going to try to get it committed for a higher version?)

bennyc

Hmm, not nice because this was a super no brainer feature that was very very helpful >:(
I see the pull request has a "CLA label". I have no idea for what CLA stands in this case :-[ Could someone shed a light here?

phil.davis

I see the pull request has a "CLA label". I have no idea for what CLA stands in this case :-[ Could someone shed a light here?[/quote]
That means that the contributor has correctly completed the relevant licensing agreement. So that is a good thing.

From the comments on https://github.com/pfsense/pfsense/pull/1901 it seems that there is some thought to add some support in binaries to make it more efficent to do. But for some reason progress in those comments stops in late Sep 2015.

RonpfS

Cool feature ;)

However on 2.2.6 x32, with pfBlockerNG, it does break pftop/Label
Before it was : USERRULES: pfB_PR, after patching it shows :USERRULES: 1770001532

and the Status: System logs: Firewall Rule column
Instead of displaying pfB_PRI3 auto rule (1770001532) it shows 1770001532 (1770001532)

marcelloc

The trackerid is used to count rules match.
If it get merged one day we will need to change the way pfblocker get his rules.

David_W

@bennyc:

Hmm, not nice because this was a super no brainer feature that was very very helpful >:(

I feel for marcelloc and everyone who finds this useful, as I know this is not the first time he's tried to get this merged.

I feel too for the core developers, as they face a difficult balance between trying to cram in extra features and trying to get 2.3 released as soon as possible. There seems now to be a real determination to get 2.3 released so that there is no need to revisit 2.2.x any further.

Based on a discussion I started in the 2.3 forum, I believe any new features or major changes have now missed the cut for 2.3. The RFC 4638 support I contributed was close to missing the cut, and only made it because it was a complete implementation, had no conflicts with the master branch and caused no regressions.

Hopefully, once 2.3 has released, there will be opportunity for this to be revisited by the necessary people.

Meanwhile, if marcelloc fixed the conflict(s) with master, those who find this useful could install this via System Patches (add .diff to the end of the pull request URL in a web browser, then create a patch using the URL that is shown in your web browser with a base directory of / and a path strip count of 2). This won't work until the conflict(s) are fixed.

BBcan177

@marcelloc:

The trackerid is used to count rules match.
If it get merged one day we will need to change the way pfblocker get his rules.

pfBlockerNG is already using tracker IDs…

The issue with the current "Rule Count" code, is that its modifying the Description field in certain coditions which removes the human-readable text.... So I don't think the code in the pfBNG package needs to be changed. Let me know if you see it differently and I will consider making changes to the pfBNG code.

Thanks

marcelloc

Great. But first the code merge :)

RonpfS

@marcelloc:

The trackerid is used to count rules match.
If it get merged one day we will need to change the way pfblocker get his rules.

But the patches changes things in "Status: System logs: Firewall" and "pftop/Label", not the pfBlockerNG tabs.

loos

I'm working to merge this request, but in a way it doesn't break any of existing code.

We'll have this for 2.3.

Thanks for this great contribution.

bennyc

@loos:

I'm working to merge this request, but in a way it doesn't break any of existing code.

We'll have this for 2.3.

Thanks for this great contribution.

Sounds great, thanks ! :) :)

marcelloc

It's close to get merged 8) after loos-br function added to 2.3

https://github.com/pfsense/pfsense/pull/1901

hit_count_23.PNG_thumb

hit_count_23_02.PNG_thumb

brandur

Exciting ;D
(fingers crossed)

JackL

Congrats Marcelloc!

Ou did an excellent work with the hitcount. I look forward to see the feature in pfSense 2.3! ;)

[]`s
Jack

Valex

Great work marcelloc!
Hope this feature will be in 2.3 because it is so useful.
Last idea of Renato seems to be great too because you have dynamic counters without reloading the page.

Edit: Seems like Renato is merging it ;)

jimp

2.3 does now have a hit counter on the rules view, though it was reworked a bit in the process.

JackL

@jimp:

2.3 does now have a hit counter on the rules view, though it was reworked a bit in the process.

Great!

Congrats for all! ;-)

[]`s
Jack