Please align the constants in the header file

ae's remarks

Update the maxentries sysctl once the hashtable has been init in order to reflect the actual size used.

In D15050#317087, @ae wrote:

Can you provide some numbers what benefits you have with this patch?

Can you provide some numbers what benefits you have with this patch?

In D14536#316763, @darkfiberiru_gmail.com wrote:

Sorry I thought I had already replied to this. I have issues with keeping that as the default value as it has been deprecated since 1998. As such it's not really a sane default and not compatible with much modern routing gear. The setting I choose comes directly our of the RFC for this type of traffic.

Well, as I said, I have no views on what the value should be. I defaulted to keeping the old value, but if you've got a good reason to change it that's fine by me.

In D14536#305909, @kristof wrote:

I have no specific views on what the value should be. The remark mostly comes from the desire to avoid having the kernel enforce policy. It's only a small extra step here for more flexibility.
I think I'd make net.inet.carp.dscp be an integer, with a default value of (the value of) IPTOS_LOWDELAY.

AFAIU, the patch has been neither finished nor submitted to master.
The last activity was more than a year ago, so should it be considered as abandoned?
Are there any plans to add the feature into the kernel?

Split in to many separate reviews.

In D14966#314881, @bcr wrote:

Bump the .Dd when you do the actual commit (once network has approved, too).

OK from manpages. Bump the .Dd when you do the actual commit (once network has approved, too). Thanks for working on this!

Looks good to me.

I've made it work with additional changes, but don't have a workload + hardware combo that benefits from the reduction in cache misses.

Keep the old hooks so other pfil users don't need to change. Allow pf to use the new style of hook, with the flags argument.

I have no specific views on what the value should be. The remark mostly comes from the desire to avoid having the kernel enforce policy. It's only a small extra step here for more flexibility.
I think I'd make net.inet.carp.dscp be an integer, with a default value of (the value of) IPTOS_LOWDELAY.

In D14536#305526, @kristof wrote:

Would it make sense to set the DSCP value to the value configured in 'net.inet.carp.dscp' rather than a hardcoded value?

Would it make sense to set the DSCP value to the value configured in 'net.inet.carp.dscp' rather than a hardcoded value?

Fixed default setting of new sysctl in man page.

In D13715#296796, @ae wrote:

I think someone from pfSense or openSense already reworked PF to work "inplace" like ipfw does, i.e. it returns mbuf back to the function from where pfil was invoked.

In D13715#296744, @kristof wrote:

I'd like to fix the issue where pf can't reliably figure out if it should call ip6_forward() or ip6_output(). I'd like to do so without negatively affecting the general forwarding performance (with or without pf).

In D13715#296702, @eri wrote:

In D13715#295452, @kristof wrote:

In D13715#295449, @eri wrote:

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.

I'm not sure I see how this would create confusion. This merely presents more information about the packet, and where the netpfil hook being called from.

pf(4) already has a loop detection mechanism that i created to support divert(9) and dummunet(9), not sure if the later ever made it into FreeBSD.

Now I'm confused. This isn't about loop detection. This is about detecting if a PFIL_OUT packet is being forwarded or output.

In D13715#295452, @kristof wrote:

In D13715#295449, @eri wrote:

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.

I'm not sure I see how this would create confusion. This merely presents more information about the packet, and where the netpfil hook being called from.

I'm against the last proposal. It is not costless to tag each forwarded packet and then remove the tag. This will seriously hit the performance.

(Apologies; last comment on this matter)

I guess the "cost should be relatively low" comment is kind of wrong. OUT hooks that care about whether it's forwarded or not will take a hit if it's not forwarded and the tag is not present, but I have no idea how heavy this would be- I have no notion of how heavily used mbuf tags are. =)

In D13715#295452, @kristof wrote:

In D13715#295449, @eri wrote:

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.

My gut reaction (i.e. without benchmarks) is that there'd be a performance penalty for tagging all forwarded packets.

In D13715#295449, @eri wrote:

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.

I'm not sure I see how this would create confusion. This merely presents more information about the packet, and where the netpfil hook being called from.

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.

Having had time to review it again, I think it looks good. This iteration exposes it as a flag to describes the path the packet's taken, rather than exposing it as the direction the packet is going in and having places where it was necessary to then mask that fact by flipping dir back to OUT where paths didn't yet know about FWD

Sorry, removal of manpages was unintentional- caches, grrrr.

Based around a suggestion from Kyle Evans (who also did all of the work), introduce a flags variable to the pfil callbacks. Keep using PFIL_OUT for forwarded packets, but set the PFIL_FWD flag for them. This allows pf to work out if a packet is being forwarded or not, with essentially no changes to other netpfil consumers.

In D13715#288702, @eri wrote:

While this is needed i do not agree that the modifications on the stack and packet filters should be so hackish.

More context. No changes to the diff.

While this is needed i do not agree that the modifications on the stack and packet filters should be so hackish.

Can you please update the patch with additional context according to https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface