svn commit: r345760 - in head: contrib/pf sys/netpfil/pf sbin/pfctl

[Ermal Luçi]

On Mon, Apr 1, 2019 at 2:06 PM Rodney W. Grimes <
freebsd-rwg at gndrsh.dnsmgr.net> wrote:

> > On 1 Apr 2019, at 18:47, Rodney W. Grimes wrote:
> > > I know for a fact that there is desire, with financials avaliable,
> > > to get our code updated.  I do not think there is any specific
> > > criteria desired, other than moved closer to the OpenBSD version.
> > >
> > It?s a good goal, but there are three major issues along the way to
> > importing the latest OpenBSD version. (And I?m sure a whole bunch of
> > smaller ones.)
>
> This is a grand start, thank you for providing this input to the
> process.
>

Certainly helps the process to move forward.

>
> > Those are:
> >
> >   - scalability
> >   - syntax
>

I think syntax is just minor, depending on the effort to be spent. in time,
it is not hard adapting existing syntax on newer pf (it is still a subset
of the OpenBSD implemented one).


> >   - vimage
> >
> > The scalability issue is the obvious difference: OpenBSD?s pf is still
> > very much single-core oriented, whereas the FreeBSD pf version can cope
> > with multiple cores (somewhat) and is significantly faster on multicore
> > hardware. Our version is by no means perfect, but it?s much faster
> > than OpenBSD?s version. Much of the imperfections we have now is there
> > because pf was designed in a giant locked kernel in the first place.
> > Multi-core scalability was not part of its original design.
> >
>
That is correct, there is still locking work possible to improve
performance or integrity of it.


> > Adopting OpenBSDs pf would mean redoing all of the locking work Gleb did
> > years ago. Given the differences in OpenBSD?s pf (e.g. they keep
> > states in a tree, not a hash table) it?s not a matter of replaying the
> > previous work on a new pf version. This is a from the ground up
> > introduction of fine grained locking in a code base that assumes a
> > single giant lock. As I understand it the OpenBSD people are working on
> > the problem as well, but I?ve not seen any diffs yet. If they?ve
> > made significant progress we may be able to base our work on theirs.
> > I don?t think it?d be acceptable to not have this, as it?d mean a
> > very large performance regression.
> >
> > For reference, before I did the pfsync work we essentially had a
> > single-threaded pf when pfsync was enabled. On my test hardware this
> > meant a throughput of ~1.1Mpps, rather than the ~3.9Mpps without pfsync.
> > I?d expect OpenBSDs pf to perform at around that ~1.1Mpps number
> > without locking work.
>
> There is some work even in the CARP story here.


> The project funding source is OS agnostic, would it help if the
> OpenBSD pf implementation was redone in a way that it had fine
> grained locking.  Would it be possible to apply Gleb's work as
> a upstreaming operation to get this work in both implementations?
>
>
This would be quite a good approach to allow future proof.
Since it did not happen up to now, would put a 'hard' classification to the
task forward.


> > The second issue is one of syntax, and that?s what I assume is the
> > main reason people want OpenBSDs pf. We?re still on an older iteration
> > of the pf syntax, but changing that would inevitably mean breaking old
> > configurations. That might be acceptable on a major version update (i.e.
> > going into 13), but would mean the new work could never be backported.
> > That?s probably the only way forward though. I?m playing with
> > importing the ?match? keyword and not breaking the ?scrub?
>
When involved in pfSense i have implemented match action on the FreeBSD pf
version.
That has been done for like 4-6 years ago, so just look at their repo for
its implementation.

> syntax at the same time, but it?s a non-trivial problem, and that?s
> > only one of the steps along the way.
>
> Isnt there more than just syntax?  I thought OpenBSD pf has features
> that are not present in FreeBSD's pf implementation.
>

The architecture of pf is not different so mostly its syntax sugar rather
than else.
As said, i do not see the syntax compatibility difficult to achieve, though
i am against it since it will be a drag to deprecate that.



>
> > Finally there?s vimage. That?s a FreeBSD-only feature, so naturally
> > OpenBSDs pf is not aware of this. I expect that to be relatively easy to
> > add back in, but it?s another obstacle. As vimage is what lets us have
> > the pf tests we?ve got now I?d be very reluctant to let it go.
> > It?s a supported feature in 12.0, so users will start to rely on it as
> > well.
> >
> > TL;DR: It?s possible, but *hard*. Expect is to be many person-months
> > of effort, and there?s no way it?s going to be a smooth ride.
>
> So your quantifying this at man months and not man years,
> that helps a bunch, that is clearly within the scope of
> the funding.
>
> That seems reasonably quantified, from a person who has some experience at
this before.


> >
> > One thing I?ve thought of trying, and that might be an interesting
> > stepping stone, is to create a port (/usr/ports/net/opf or whatever) of
> > OpenBSD?s pf. In that version it?d be acceptable to not fix any of
> > the above issues. It?d still give users to option of getting the new
> > syntax. I?d expect this to be a relatively straightforward exercise.
> > In principle there?s nothing to stop us from doing that same work in
> > base, but we?re **NOT** going to import a fourth firewall. We?re
> > just not.
>
> But 4, its nice, its a power of 2, infact it is the second power of
> 2, such nice round numbers :-)  I do like the idea of a first
> implementation of pf as a port, that being a fast path to update it,
> but would not want that as a long run solution.
>
>
As long as there is a maintainer, it seems a feasible deprecation path.


> > Regards,
> > Kristof
> --
> Rod Grimes
> rgrimes at freebsd.org
> _______________________________________________
> freebsd-pf at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-pf
> To unsubscribe, send any mail to "freebsd-pf-unsubscribe at freebsd.org"
>


-- 
Ermal