Re: git: c3fbd9c6212c - main - Revert "stand: Remove double words in source code comments"
On 2023-04-18 at 02:09 EDT, Gordon Bergling wrote: The branch main has been updated by gbe: URL: https://cgit.FreeBSD.org/src/commit/?id=c3fbd9c6212cad2634f5958ffec0370166ec7204 commit c3fbd9c6212cad2634f5958ffec0370166ec7204 Author: Gordon Bergling AuthorDate: 2023-04-18 06:08:35 + Commit: Gordon Bergling CommitDate: 2023-04-18 06:08:35 + Revert "stand: Remove double words in source code comments" The sentence, "The base address that we the boot0 code to to run it." is correct. Reported by:jrtc27 This reverts commit b12ccd0bb1b37f32e972bb3e945e4025fe409e2f. --- stand/i386/boot0/Makefile | 2 +- stand/libsa/zfs/zfsimpl.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stand/i386/boot0/Makefile b/stand/i386/boot0/Makefile index dad078fd6f71..1453f17751b9 100644 --- a/stand/i386/boot0/Makefile +++ b/stand/i386/boot0/Makefile @@ -36,7 +36,7 @@ BOOT_BOOT0_FLAGS?=0x8f # 0xb6 (182d) corresponds to 10 seconds. BOOT_BOOT0_TICKS?= 0xb6 -# The base address that we the boot0 code to run it. Don't change this +# The base address that we the boot0 code to to run it. Don't change this I really think this comment needs a verb (load), as observed by jrtc27@. Even better (?), as danfe@ suggested, it should possibly be rephrased as: "The base address where we load the boot0 code to run it. Don't change this." or something along those lines. Thanks, Matteo signature.asc Description: PGP signature
Re: git: 6a70a0c8bfa6 - main - Document implicit dependencies of the mlx5(4) & friends.
On 2022-08-11 at 19:33 EDT, Maxim Sobolev wrote: The branch main has been updated by sobomax: URL: https://cgit.FreeBSD.org/src/commit/?id=6a70a0c8bfa6fe8a2739527e5c822aec985e83e9 commit 6a70a0c8bfa6fe8a2739527e5c822aec985e83e9 Author: Maxim Sobolev AuthorDate: 2022-08-11 23:28:27 + Commit: Maxim Sobolev CommitDate: 2022-08-11 23:33:09 + Document implicit dependencies of the mlx5(4) & friends. MFC after: 2 weeks --- sys/amd64/conf/GENERIC | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sys/amd64/conf/GENERIC b/sys/amd64/conf/GENERIC index 53c6043a0146..9178abba36cc 100644 --- a/sys/amd64/conf/GENERIC +++ b/sys/amd64/conf/GENERIC @@ -260,7 +260,8 @@ device le # AMD Am7900 LANCE and Am79C9xx PCnet device ti # Alteon Networks Tigon I/II gigabit Ethernet # Nvidia/Mellanox Connect-X 4 and later, Ethernet only -# mlx5ib requires ibcore infra and is not included by default +# o requires COMPAT_LINUXKPI and xz(4) +# o mlx5ib requires ibcore infra and is not included by default Hi Maxim, May I suggest editing the above line to "mlx5ib requires ibcore infra, thus it is not included here" (replacing "and" with "thus" being the important part). The mlx5en(4) page, in SYNOPSIS, lists also option RATELIMIT, option KERN_TLS, and device firmware. Are these needed? I imagine that "device firmware" certainly is (thus should perhaps be listed in the comment above), but what about the options? device mlx5# Base driver device mlxfw # Firmware update device mlx5en # Ethernet driver Thanks, Matteo signature.asc Description: PGP signature
Re: git: cfa1a1308709 - main - pfctl: fix recrusive printing of ethernet anchors
On 2022-10-07 at 06:13 EDT, Kristof Provost wrote: On 3 Oct 2022, at 18:13, Bryan Drewery wrote: I think there's still a problem here. pfctl -a '*' -sr works pfctl -a 'name/*' -sr does not. So I’ve looked at this a bit more, and I am now going to back away from the whole anchor thing, and try to pretend I didn’t see any of the tentacled horrors that lurk within. To give you an idea of the issues, loading the following ruleset: anchor "foo" { anchor "bar" { pass in } } does exactly what you’d expect: # pfctl -sr -a "*" anchor "foo" all { anchor "bar" all { pass in all flags S/SA keep state } } # pfctl -sr -a "foo/*" anchor "bar" all { pass in all flags S/SA keep state } However, if we `pfctl -Fr` to flush all rules: # pfctl -Fr rules cleared # pfctl -sr -a "*" # pfctl -sr -a "foo/*" anchor "bar" all { pass in all flags S/SA keep state } How is one supposed to know which rules are really loaded in this case? Printing of rules with anchors being broken (I even get a segmentation fault with 'pfctl -a "*" -sr -vv') makes debugging rulesets very hard. Partially, the question I also have is: is printing of rules broken, or is flushing of rules broken, or a third thing? =) Unloading pf to actually delete the bar anchor, and then we set: anchor “foo” And then # echo "pass" | pfctl -g -f - -a "foo/bar" # pfctl -sr -a "*" anchor "foo" all { } # pfctl -sr -a "foo/*" # pfctl -sr -a "foo/bar" pass all flags S/SA keep state There are a lot of issues there, and it’ll take a lot of time and effort to root them out. My plan is to drink heavily and attempt to forget. Kristof Thanks, Matteo signature.asc Description: PGP signature
Re: git: cfa1a1308709 - main - pfctl: fix recrusive printing of ethernet anchors
On 2022-10-18 at 04:44 EDT, Kristof Provost wrote: On 17 Oct 2022, at 19:37, Matteo Riondato wrote: On 2022-10-07 at 06:13 EDT, Kristof Provost wrote: On 3 Oct 2022, at 18:13, Bryan Drewery wrote: I think there's still a problem here. pfctl -a '*' -sr works pfctl -a 'name/*' -sr does not. So I’ve looked at this a bit more, and I am now going to back away from the whole anchor thing, and try to pretend I didn’t see any of the tentacled horrors that lurk within. To give you an idea of the issues, loading the following ruleset: anchor "foo" { anchor "bar" { pass in } } does exactly what you’d expect: # pfctl -sr -a "*" anchor "foo" all { anchor "bar" all { pass in all flags S/SA keep state } } # pfctl -sr -a "foo/*" anchor "bar" all { pass in all flags S/SA keep state } However, if we `pfctl -Fr` to flush all rules: # pfctl -Fr rules cleared # pfctl -sr -a "*" # pfctl -sr -a "foo/*" anchor "bar" all { pass in all flags S/SA keep state } How is one supposed to know which rules are really loaded in this case? Printing of rules with anchors being broken (I even get a segmentation fault with 'pfctl -a "*" -sr -vv') makes debugging rulesets very hard. `pfctl -a "*" -sr` should always produce the expected results, at least as far as I know. The last example above clearly shows that `pfctl -a "*"` does *not* always produce the expected results: after flushing the rules in the root anchor with `pfctl -Fr`, `pfctl -sr -a '*'` does *not* produce the expected results, which would have been anchor "foo" all { anchor "bar" all { pass in all flags S/SA keep state } } I'm wondering what gets printed if you load a ruleset such as: block all anchor "foo" all { pass in all proto udp anchor "bar" all { pass in all proto tcp flags S/SA keep state } } Then print it with `pfctl -sr -a '*'`, then do a `pfctl -Fr`, and then do another `pfctl -sr -a '*'`, followed by a `pfctl -sr -a "foo/*"`. I expect to see, again nothing for the `pfctl -sr -a '*'` after the flush, and something like the following for the `pfctl -sr -a "foo/*"`: anchor "foo" all { pass in all proto udp anchor "bar" all { pass in all proto tcp flags S/SA keep state } } which at least would (partially?) confirm that `pfctl -Fr` only flushes rules in the root anchor. I’d be very interested in seeing a test case where that core dumps, because that is indeed very annoying, and might be something I can fix. I'll try to come up with a minimally reproducible case (it is totally reproducible with my settings, but I'd like to give you something smaller). Partially, the question I also have is: is printing of rules broken, or is flushing of rules broken, or a third thing? =) To the extent that I currently understand this problem I believe the issue is that we’re not always stepping into child anchors to print them. I believe they do get evaluated when the rules are processed. So it’s the printing that’s broken. The flushing is .. not broken, but may not do what you’d expect. When we flush we only flush the root anchor, and other anchors can remain. I think that’s the main source of the strange behaviour I’ve described. Assuming that indeed `pfctl -Fr` really only acts on the root anchor, what makes me worried is whether rules inside child anchors are really evaluated *after* a flush of those in the root anchor with `pfctl -Fr`, but checking if they really are evaluated should be an easy test. Thanks, Matteo signature.asc Description: PGP signature
Re: git: a67b925ff3e5 - main - mail: make The Dragonfly Mail Agent (dma) the default mta.
On 2022-11-07 at 06:50 EST, Baptiste Daroussin wrote: diff --git a/libexec/rc/rc.conf b/libexec/rc/rc.conf index a71a3fa4063d..91b99780eae6 100644 --- a/libexec/rc/rc.conf +++ b/libexec/rc/rc.conf @@ -596,7 +596,7 @@ allscreens_kbdflags=""# Set this kbdcontrol mode for all virtual screens mta_start_script="/etc/rc.sendmail" # Script to start your chosen MTA, called by /etc/rc. # Settings for /etc/rc.sendmail and /etc/rc.d/sendmail: -sendmail_enable="NO" # Run the sendmail inbound daemon (YES/NO). +sendmail_enable="NONE" # Run the sendmail inbound daemon (YES/NO). The comment on the above line should probably be updated: inexperienced users may not know that there is a difference between "NO" and "NONE". sendmail_pidfile="/var/run/sendmail.pid" # sendmail pid file sendmail_procname="/usr/sbin/sendmail"# sendmail process name sendmail_flags="-L sm-mta -bd -q30m" # Flags to sendmail (as a server) Thanks, Matteo signature.asc Description: PGP signature
Re: git: b7104f19147f - main - sendmail: document that sendmail_enable can be set to NONE
On 2022-11-08 at 12:11 EST, Mark Millard wrote: Baptiste Daroussin wrote on Date: Tue, 08 Nov 2022 13:41:29 UTC : On Tue, Nov 08, 2022 at 08:07:46AM -0500, Matteo Riondato wrote: > On 2022-11-07 at 06:50 EST, Baptiste Daroussin > wrote: > > diff --git a/libexec/rc/rc.conf b/libexec/rc/rc.conf > > index a71a3fa4063d..91b99780eae6 100644 > > --- a/libexec/rc/rc.conf > > +++ b/libexec/rc/rc.conf > > @@ -596,7 +596,7 @@ allscreens_kbdflags="" # Set this kbdcontrol > > mode for all virtual screens > > mta_start_script="/etc/rc.sendmail" > > # Script to start your chosen MTA, called by /etc/rc. > > # Settings for /etc/rc.sendmail and /etc/rc.d/sendmail: > > -sendmail_enable="NO" # Run the sendmail inbound daemon (YES/NO). > > +sendmail_enable="NONE" # Run the sendmail inbound daemon > > (YES/NO). > > The comment on the above line should probably be updated: > inexperienced > users may not know that there is a difference between "NO" and > "NONE". > > > sendmail_pidfile="/var/run/sendmail.pid" # sendmail pid file > > sendmail_procname="/usr/sbin/sendmail" # sendmail process name > > sendmail_flags="-L sm-mta -bd -q30m" # Flags to sendmail (as a > > server) You are not the first to report that, I have updated rc.conf a to add "the NONE" value. Also, from the commit: QUOTE -sendmail_enable="NONE" # Run the sendmail inbound daemon (YES/NO). +sendmail_enable="NONE" # Run the sendmail inbound daemon (YES/NO/NONE). END QUOTE My guess is Matteo was after something more like the following text from your commit notes: QUOTE setting sendmail_enable to NONE (which is now the default) turns all the sendmail_*_enable variables to NO END QUOTE In other words, documenting what is different about NONE vs. NO beyond the spelling distinction. Indeed the issue that I intended to raise was that, AFAIK, there is no documentation, for NONE, except if one reads and understands /etc/rc.d/sendmail. It is something that is known by people "in the know", and probably mentioned by a some online tutorials, but I don't think there is anything "official". Even the handbook, in the section about changing MTA (https://docs.freebsd.org/en/books/handbook/mail/#mail-changingmta) does not mention NONE, but explicitly sets the four variables to NO. (Side note: it seems that when this change will be integrated in some -STABLE branch, or when 14 becomes -STABLE, a large update to the relevant sections of the Handbook will be needed) Thanks, Matteo signature.asc Description: PGP signature
Re: git: b7104f19147f - main - sendmail: document that sendmail_enable can be set to NONE
On 2022-11-08 at 14:17 EST, Baptiste Daroussin wrote: Indeed the issue that I intended to raise was that, AFAIK, there is no documentation, for NONE, except if one reads and understands /etc/rc.d/sendmail. It is something that is known by people "in the know", and probably mentioned by a some online tutorials, but I don't think there is anything "official". Even the handbook, in the section about changing MTA (https://docs.freebsd.org/en/books/handbook/mail/#mail-changingmta) does not mention NONE, but explicitly sets the four variables to NO. It is documented as deprecated in rc.sendmail(8) Sorry for missing this: this man page is not linked from sendmail(8), nor from rc(8), nor mentioned in the script that it describes, although it is linked from rc.conf(5) (and from no other manpage, as far as I can tell). If NONE is deprecated, then perhaps it would have been better to set the four variables to NO in the modifcation. Also, I wonder whether the value of "mta_start_script" in rc.conf should be changed to reflect that the default MTA is no longer sendmail, and also what the role of /etc/rc.d/othermta should be (and whether it should be updated as well). Thanks, Matteo signature.asc Description: PGP signature
Re: git: 0b1adc42a15c - main - rc.sendmail: remove unused script
On 2022-11-09 at 10:57 EST, Baptiste Daroussin wrote: The branch main has been updated by bapt: URL: https://cgit.FreeBSD.org/src/commit/?id=0b1adc42a15caea0cffbc962ca6f9e3e7b576834 commit 0b1adc42a15caea0cffbc962ca6f9e3e7b576834 Author: Baptiste Daroussin AuthorDate: 2022-11-09 15:33:03 + Commit: Baptiste Daroussin CommitDate: 2022-11-09 15:55:18 + rc.sendmail: remove unused script 20 years ago the use of rc.sendmail script was dropped in favor of /etc/rc.d/sendmail, it is time to retire the script entirely now. Thank you for doing this. The one lingering issue I see is that there is now no documentation for the rc.conf sendmail_* variables: they were documented in rc.sendmail.8 and rc.conf.5 referred to it, but your previous commit 6a2d6a569b1350f46998a2446fc89059b160fd13 removed that reference from rc.conf.8, and this commit removes rc.sendmail.8. (Sorry for the noise if you're already working on it). Thanks, Matteo signature.asc Description: PGP signature
Re: git: 0b1adc42a15c - main - rc.sendmail: remove unused script
On 2022-11-09 at 14:29 EST, Baptiste Daroussin wrote: Le 9 novembre 2022 18:49:47 GMT+01:00, Matteo Riondato a écrit : The one lingering issue I see is that there is now no documentation for the rc.conf sendmail_* variables: they were documented in rc.sendmail.8 and rc.conf.5 referred to it, but your previous commit 6a2d6a569b1350f46998a2446fc89059b160fd13 removed that reference from rc.conf.8, and this commit removes rc.sendmail.8. (Sorry for the noise if you're already working on it). I prefer too much noise than something forgotten in silence and actually this is no noise Such documentation is supposed to be in rc.conf(5). It would probably be a good idea to push the documentation there. If someone is interested be my guest, otherwise I will look into it in the next couple of weeks. Thank you for taking care of this one so quickly. A few rough corners I still see: 1) sendmail_cert_create should perhaps be set to NO in rc.conf, or the call to sendmail_cert_create() in rc.d/sendmail should also depend on some form of sendmail being enabled. The former action seems easier. 2) the role of rc.d/othermta is quite bizarre, especially now that it checks for a non-existing script (/etc/rc.sendmail). I wonder whether it should be retired, together with the rc.conf variable mta_start_script (which now is set to a non-existing script). It feels like a relic from a long gone past. 3) the comments on some of the sendmail_* variables in rc.conf mention a generic "MTA", but perhaps they should say "sendmail", as that's not the one-and-only MTA. Thanks, Matteo signature.asc Description: PGP signature
Re: git: 0b1adc42a15c - main - rc.sendmail: remove unused script
On 2022-11-10 at 09:18 EST, Baptiste Daroussin wrote: On Thu, Nov 10, 2022 at 09:01:06AM -0500, Matteo Riondato wrote: A few rough corners I still see: 1) sendmail_cert_create should perhaps be set to NO in rc.conf, or the call to sendmail_cert_create() in rc.d/sendmail should also depend on some form of sendmail being enabled. The former action seems easier. Don't know, this has always been like this for a while. Yeah, I see pros and cons of changing it, so not touching it may be wise =) 2) the role of rc.d/othermta is quite bizarre, especially now that it checks for a non-existing script (/etc/rc.sendmail). I wonder whether it should be retired, together with the rc.conf variable mta_start_script (which now is set to a non-existing script). It feels like a relic from a long gone past. https://lists.freebsd.org/archives/freebsd-arch/2022-November/000264.html It is a relic from the past (unused for 20 years) Glad we agree. I should probably start following arch@ again. =) 3) the comments on some of the sendmail_* variables in rc.conf mention a generic "MTA", but perhaps they should say "sendmail", as that's not the one-and-only MTA. I am open to proposals/patches (Can't promise cut and paste will work correctly) diff --git a/libexec/rc/rc.conf b/libexec/rc/rc.conf index 50fed07df1..b5d908e38b 100644 --- a/libexec/rc/rc.conf +++ b/libexec/rc/rc.conf @@ -596,21 +596,21 @@ allscreens_kbdflags=""# Set this kbdcontrol mode for all virtual screens mta_start_script="/etc/rc.sendmail" # Script to start your chosen MTA, called by /etc/rc. # Settings for /etc/rc.d/sendmail: -sendmail_enable="NO" # Run the sendmail inbound daemon (YES/NO/NONE). +sendmail_enable="NO" # Start a inbound/outbound sendmail daemon (YES/NO/NONE). # If NONE, don't start any sendmail processes. sendmail_pidfile="/var/run/sendmail.pid" # sendmail pid file sendmail_procname="/usr/sbin/sendmail" # sendmail process name -sendmail_flags="-L sm-mta -bd -q30m" # Flags to sendmail (as a server) +sendmail_flags="-L sm-mta -bd -q30m" # Flags to sendmail (as inbound/outbound server) sendmail_cert_create="YES" # Create a server certificate if none (YES/NO) -#sendmail_cert_cn="CN" # CN of the generate certificate -sendmail_submit_enable="NO"# Start a localhost-only MTA for mail submission +#sendmail_cert_cn="CN" # CN of the generated certificate +sendmail_submit_enable="NO"# Start a localhost-only sendmail for mail submission sendmail_submit_flags="-L sm-mta -bd -q30m -ODaemonPortOptions=Addr=localhost" - # Flags for localhost-only MTA -sendmail_outbound_enable="NO" # Dequeue stuck mail (YES/NO). -sendmail_outbound_flags="-L sm-queue -q30m" # Flags to sendmail (outbound only) -sendmail_msp_queue_enable="NO" # Dequeue stuck clientmqueue mail (YES/NO). + # Flags for localhost-only sendmail +sendmail_outbound_enable="NO" # Start an outbound-only sendmail (YES/NO). +sendmail_outbound_flags="-L sm-queue -q30m" # Flags to outbund-only sendmail +sendmail_msp_queue_enable="NO" # Start a clientmqueue runner sendmail (YES/NO). sendmail_msp_queue_flags="-L sm-msp-queue -Ac -q30m" - # Flags for sendmail_msp_queue daemon. + # Flags for clientmqueue runner sendmail sendmail_rebuild_aliases="NO" # Run newaliases if necessary (YES/NO). Thanks, Matteo signature.asc Description: PGP signature
Re: git: f53dc31bb3ef - main - src.opts.mk: Disable all of LLVM if C++ support is disabled.
On 2022-11-15 at 22:23 EST, John Baldwin wrote: The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=f53dc31bb3ef387338a7678581e8c7d587da8d2c commit f53dc31bb3ef387338a7678581e8c7d587da8d2c Author: John Baldwin AuthorDate: 2022-11-16 03:21:20 + Commit: John Baldwin CommitDate: 2022-11-16 03:21:20 + src.opts.mk: Disable all of LLVM if C++ support is disabled. This should probably be documented in src.conf(5). Reviewed by:imp, emaste Differential Revision: https://reviews.freebsd.org/D36891 --- share/mk/src.opts.mk | 3 +++ 1 file changed, 3 insertions(+) diff --git a/share/mk/src.opts.mk b/share/mk/src.opts.mk index 4c0913474ef7..ff8c359acc42 100644 --- a/share/mk/src.opts.mk +++ b/share/mk/src.opts.mk @@ -380,6 +380,9 @@ MK_KERBEROS_SUPPORT:= no .if ${MK_CXX} == "no" MK_CLANG:= no +MK_LLD:= no +MK_LLDB:= no +MK_LLVM_BINUTILS:= no MK_GOOGLETEST:= no MK_OFED:= no MK_OPENMP:= no Thanks, Matteo signature.asc Description: PGP signature
Re: git: 62a149bf6219 - main - Add new rc: machine_id to generate /etc/machine-id
On 2022-12-24 at 03:39 EST, Tobias C. Berner wrote: Moin moin I think it tries to solve the same problem of giving the machine a unique id. From the linux man page [1] The machine ID does not change based on local or network configuration or when hardware is replaced. Due to this and its greater length, it is a more useful replacement for the gethostid(3) call that POSIX specifies. Does this make or should it make hostid obsolete? If yes, then changes are needed to deorbit hostid. If not, why not? Thanks, Matteo signature.asc Description: PGP signature
Re: git: 3a1f834b5228 - main - pf: Add code to enable filtering for locally delivered packets
On 2023-06-20 at 10:35 EDT, Doug Rabson wrote: The branch main has been updated by dfr: URL: https://cgit.FreeBSD.org/src/commit/?id=3a1f834b5228986a7c14fd60da13cf2700e80996 commit 3a1f834b5228986a7c14fd60da13cf2700e80996 Author: Doug Rabson AuthorDate: 2023-06-20 13:01:58 + Commit: Doug Rabson CommitDate: 2023-06-20 14:34:01 + pf: Add code to enable filtering for locally delivered packets This is disabled by default since it potentially changes the behavior of existing filter rule sets. To enable this extra filter for packets being delivered locally, use: sysctl net.pf.filter_local=1 service pf restart PR: 268717 Reviewed-by:kp MFC-after: 2 weeks Differential Revision: https://reviews.freebsd.org/D40373 --- UPDATING | 12 sys/netpfil/pf/pf_ioctl.c| 20 tests/sys/netpfil/common/utils.subr | 3 +-- tests/sys/netpfil/pf/fragmentation_compat.sh | 3 ++- tests/sys/netpfil/pf/fragmentation_pass.sh | 3 ++- tests/sys/netpfil/pf/killstate.sh| 24 tests/sys/netpfil/pf/map_e.sh| 3 ++- tests/sys/netpfil/pf/pass_block.sh | 3 ++- tests/sys/netpfil/pf/pfsync.sh | 1 + tests/sys/netpfil/pf/route_to.sh | 3 ++- tests/sys/netpfil/pf/set_skip.sh | 2 +- tests/sys/netpfil/pf/table.sh| 6 -- 12 files changed, 65 insertions(+), 18 deletions(-) diff --git a/UPDATING b/UPDATING index 1980411c1853..f4e13d97006d 100644 --- a/UPDATING +++ b/UPDATING @@ -27,6 +27,18 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 14.x IS SLOW: world, or to merely disable the most expensive debugging functionality at runtime, run "ln -s 'abort:false,junk:false' /etc/malloc.conf".) +20230619: + To enable pf rdr rules for connections initiated from the host, pf + filter rules can be optionally enabled for packets delivered + locally. This can change the behavior of rules which match packets + delivered to lo0. To enable this feature: + + sysctl net.pf.filter_local=1 + service pf restart It seems a bit weird to suggest an action that is not permanent (does not survive reboot). See proposed rewording below. + + When enabled, its best to ensure that packets delivered locally are not s/its/it is/ + filtered, e.g. by adding a 'skip on lo' rule. TBH, I find the phrasing a bit confusing: "to enable pf rdr rules for connections …, pf filter rules can *optionally* be enabled for packets delivered locally". That "optionally" makes it sound as if it is not *required* to enable pf filter rules for packets delivered locally in order to enable pf rdr rules for connections etc etc., but, given this change, I assume it is. Perhaps a better phrasing (assuming I understand the feature) would be: "The new sysctl net.pf.filter_local controls whether PF filter rules are enabled for packets originating from localhost and delivered locally. This feature can be useful for, e.g., enabling rdr rules for connections initiated from localhost and redirected to a different port on localhost. Setting the sysctl to 1 may change the behavior of rules which match packets delivered to lo0, so it may be necessary to add enable the "skip on lo" option." Note that "skip on" is not a rule, even if it is translated to a pair of rules: it's part of the options, and requires "set" before it, per pf.conf(5). Also, I'm assuming (and mention in the rewording) we are talking about rdr rules for port remapping, not rdr rules that redirect to other destinations, but please confirm or adjust. More generally, this new feature should likely also be documented somewhere else (pf(4) ? pfctl(8)? pf.conf(5)?). But apart from the above, I'm a little puzzled: does it mean that until now (and continuing to do so, unless one sets the sysctl to 1), packets originating locally and destined locally were not filtered by pf? I.e., that filtering rules on lo0 had no effect on incoming traffic from localhost? Thanks, Matteo signature.asc Description: PGP signature
Re: git: 3a1f834b5228 - main - pf: Add code to enable filtering for locally delivered packets
On 2023-06-20 at 11:57 EDT, Matteo Riondato wrote: On 2023-06-20 at 10:35 EDT, Doug Rabson wrote: The branch main has been updated by dfr: URL: https://cgit.FreeBSD.org/src/commit/?id=3a1f834b5228986a7c14fd60da13cf2700e80996 commit 3a1f834b5228986a7c14fd60da13cf2700e80996 Author: Doug Rabson AuthorDate: 2023-06-20 13:01:58 + Commit: Doug Rabson CommitDate: 2023-06-20 14:34:01 + pf: Add code to enable filtering for locally delivered packets This is disabled by default since it potentially changes the behavior of existing filter rule sets. To enable this extra filter for packets being delivered locally, use: sysctl net.pf.filter_local=1 service pf restart PR: 268717 Reviewed-by:kp MFC-after: 2 weeks Differential Revision: https://reviews.freebsd.org/D40373 --- UPDATING | 12 sys/netpfil/pf/pf_ioctl.c| 20 tests/sys/netpfil/common/utils.subr | 3 +-- tests/sys/netpfil/pf/fragmentation_compat.sh | 3 ++- tests/sys/netpfil/pf/fragmentation_pass.sh | 3 ++- tests/sys/netpfil/pf/killstate.sh| 24 tests/sys/netpfil/pf/map_e.sh| 3 ++- tests/sys/netpfil/pf/pass_block.sh | 3 ++- tests/sys/netpfil/pf/pfsync.sh | 1 + tests/sys/netpfil/pf/route_to.sh | 3 ++- tests/sys/netpfil/pf/set_skip.sh | 2 +- tests/sys/netpfil/pf/table.sh| 6 -- 12 files changed, 65 insertions(+), 18 deletions(-) diff --git a/UPDATING b/UPDATING index 1980411c1853..f4e13d97006d 100644 --- a/UPDATING +++ b/UPDATING @@ -27,6 +27,18 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 14.x IS SLOW: world, or to merely disable the most expensive debugging functionality at runtime, run "ln -s 'abort:false,junk:false' /etc/malloc.conf".) +20230619: + To enable pf rdr rules for connections initiated from the host, pf + filter rules can be optionally enabled for packets delivered + locally. This can change the behavior of rules which match packets + delivered to lo0. To enable this feature: + + sysctl net.pf.filter_local=1 + service pf restart It seems a bit weird to suggest an action that is not permanent (does not survive reboot). See proposed rewording below. + + When enabled, its best to ensure that packets delivered locally are not s/its/it is/ + filtered, e.g. by adding a 'skip on lo' rule. TBH, I find the phrasing a bit confusing: "to enable pf rdr rules for connections …, pf filter rules can *optionally* be enabled for packets delivered locally". That "optionally" makes it sound as if it is not *required* to enable pf filter rules for packets delivered locally in order to enable pf rdr rules for connections etc etc., but, given this change, I assume it is. Perhaps a better phrasing (assuming I understand the feature) would be: "The new sysctl net.pf.filter_local controls whether PF filter rules are enabled for packets originating from localhost and delivered locally. This feature can be useful for, e.g., enabling rdr rules for connections initiated from localhost and redirected to a different port on localhost. Setting the sysctl to 1 may change the behavior of rules which match packets delivered to lo0, so it may be necessary to add enable the "skip on lo" option." Note that "skip on" is not a rule, even if it is translated to a pair of rules: it's part of the options, and requires "set" before it, per pf.conf(5). Also, I'm assuming (and mention in the rewording) we are talking about rdr rules for port remapping, not rdr rules that redirect to other destinations, but please confirm or adjust. More generally, this new feature should likely also be documented somewhere else (pf(4) ? pfctl(8)? pf.conf(5)?). But apart from the above, I'm a little puzzled: does it mean that until now (and continuing to do so, unless one sets the sysctl to 1), packets originating locally and destined locally were not filtered by pf? I.e., that filtering rules on lo0 had no effect on incoming traffic from localhost? Hi Doug and Kristof, A ping about what I said below, as I think there is a need to better document this sysctl. Additionally, I'm also a little worried about the name of the sysctl, which seems extremely generic, while its use may be much more specific. If the name could be made more specific, that should probably be done before 14 is branched. Thanks, Matteo signature.asc Description: PGP signature
Re: git: f8a8bb7bade9 - main - pfctl: Improve duplicate table name warning
> On Jul 7, 2025, at 11:07 AM, Kristof Provost wrote: > > The branch main has been updated by kp: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=f8a8bb7bade9575c549cbc94500ba706b712c650 > > commit f8a8bb7bade9575c549cbc94500ba706b712c650 > Author: Kristof Provost > AuthorDate: 2025-07-01 10:02:03 + > Commit: Kristof Provost > CommitDate: 2025-07-07 15:06:48 + > >pfctl: Improve duplicate table name warning > >When creating tables inside anchors, pfctl warned about namespace >collisions with global tables, but only in certain cases and with >limited information sometimes leaving users clueless. > >Deferring the check to process_tabledefs() where tables are eventually >created, both anchor and table name are known which allows for checking >all existing anchors. > >With this, warn on all duplicates even in dry-runs (`-n') and print >quoted names so they can be copied to fix configurations right away. > >No functional change in parsing or ruleset production. Based only on the commit message, this may fix PR #262295. Thanks, Matteo