- User Since
- Aug 8 2014, 12:33 PM (485 w, 4 d)
Nov 1 2021
Oct 30 2021
Oct 5 2021
How about some attribution here to where this code is coming from!
Especially since there is a black history behind, I hope to never revive!
May 13 2021
This kind of breaks the assumption of floating states, 'it will apply in any interface any direction'.
These states can be killed through src/dst options, or potentially you can do this in a daemon managing states in userland and killing states by id.
Why can't you move the interface 'all' to be a flag and just always track the interface the packet came in originally?
Apr 16 2021
Apr 14 2021
You can add me to these reviews @kp !
I have implemented originally all of this without thinking of generic way of reusability.
The intention is to have some metadata tagged on states and rules to easy perfom operation and/or reporting.
The model can be improved by having a better tagging model then just stash new fields in the structure, but porting first the patches and later on collapsing them can be a path forward, as long as the ABI is not impacted too much.
Dec 13 2020
Dec 5 2018
Not a blocker but:
- It would also be nice to have measure if the other side can keep up with the blast of state updates now?
- Even better, provide the same bucket mechanism on reception so it can be distributed on the various cores
Can you please measure the latency of syncing states with this change against previous latency?
Nov 26 2018
Can you add some text to the manual pages for documenting the feature? Possibly linking to some example?
Nov 16 2018
What needs to be considered here is the assumption of pfsync is that a state created in pf will be synched at shortest possible cycle to the cluster member.
By defering that assumption is relaxed so figuring out baselines of before and after this change would make this more easy to reason about.
Oct 30 2018
Oct 14 2018
Continue the great work you are doing on providing test cases for pf.
Jan 31 2018
Jan 27 2018
While i have not much time lately to spend on this, i still think this is the wrong way of doing things since it just creates confusion.
pf(4) has already knows about mbuf_tag(9) and uses it. I would strongly suggest using them until a proper _FWD hook comes to life and allows removing all the 'hacks' in pf(4) and possibly elsewhere.
Jan 6 2018
While this is needed i do not agree that the modifications on the stack and packet filters should be so hackish.
Mar 28 2017
Just curious, do you have any comparison/profiling data if this improves anything?
Mar 24 2017
Mar 17 2017
I would rather remove the pf_unload and pf_load to empty stubs and do their operation in pf_vnet_[un]init wiht DEFAULT_VNET wrapping than this.
Mar 16 2017
It feels a lot like a hack.
Shouldn't the proper VNET accessor be called on creation and teardown?
Mar 6 2017
Mar 5 2017
Feb 21 2017
Include documentation (man pages) though someone should give a review to those changes.
Feb 20 2017
Update considering comments.
Feb 15 2017
Feb 14 2017
Completed review tasks.
Update diff to include all remarks
Feb 12 2017
Feb 10 2017
Just some information to pkesley@ comment.
Feb 9 2017
Just looking for an OK here this is not rocket science!
Feb 5 2017
So i go and commit this in one week if no objections are raised.
Feb 1 2017
Anyone has objections so this can go in?
Jan 19 2017
Update to include context.
Busy with other things.
Updating this to include context.
Update patch to include context as well.
Updated as per comments.
Jan 18 2017
Dec 22 2016
Can't this be made part of a single API where the protocol family is passed so it can be reused and extended easily?
Imagine layer2 forwarding can use this as well.....
Oct 23 2016
I understand and i still think this is _completely_ useless from what you have submitted.
This is not the way this should proceed.
Oct 1 2016
You need to do more than this.
This change is not correct per se.
Jan 28 2016
Jan 27 2016
Jan 25 2016
Any update on the following?
Jan 21 2016
Oct 28 2015
I know this has been committed but please check my remarks.
Oct 2 2015
The easiest way to do this is check if you have an inp and check that the proto is TCP from that and skip cksum update.
Otherwise you will end up with complex code as this which is easier to understand and maintain.
It certainly is not documented properly but this makes it even more.
Sep 28 2015
The problem with IPSec is that this might leak traffic that otherwise would be used by ipsec.
Probably even looped traffic might leak with this scenarios from proxies....
Sep 27 2015
This still breaks IPSEC scenarios at least.
Sep 24 2015
See my replies!
Aug 31 2015
Reduce the diff to only rule handling for now.
I would have put this in pf_refragment6 function call but lets put this in and see later on to improve it further.
Yeah i noticed this and am working on fixing it.
Mostly this is from the relaxed requirement to accept rules without existing interface name!
Did this behavior started after implementing the re fragmentation functinality?
Aug 30 2015
While that is not an open available link but clearly the target is different.
That patch is about IPSec not impacting forwarding path when its not in use at all
while this patch is about avoiding a routing lookup when IPSec is in use more in relation with the commit in FreeBSD removing redundant routing table lookups in forwarding path.
Sure gnn@ i can write Obtained from eri at al.
What have you been smoking lately? Can you clarify where this obtained from should contain and you reasoning behind?
Aug 27 2015
Aug 25 2015
It looks good in general, though I do not like automatic conversion in such case.
Just put in the commit as part of release notes and people need to be fully aware of the change.
Whoever used these options i am certain had their reasons so they should be aware of the change.
I do not think this is the root cause for this, it just hides another issue.
I think that the result of RB_INSERT should be checked when a group/iface is created should be tested.
If that returns NULL the result should be null.
Aug 7 2015
One thing not mentioned in this review is that the codel queue type can be used on any other scheduler like PRIQ, CBQ, HFSC, FAIRQ.
It is quite flexible at that.
This can go in.
I am working on restructuring all this to be a bit more modern and not so...hackish.
Aug 4 2015
I do not see why the receiving of the ICMP_UNREACH is impacted by this?
Aug 3 2015
Can you please put my copyright in the codel files since it was forgotten addition when i did the port?