Page MenuHomeFreeBSD
Differential D50763

pf: Fix error handling when pf_map_addr() fails
ClosedPublic
Actions

Authored by vegeta_tuxpowered.net on Jun 9 2025, 8:44 PM.
Tags
None
Referenced Files
F123308598: D50763.diff
Sun, Jul 13, 11:53 AM
Unknown Object (File)
Sat, Jul 12, 3:19 AM
Unknown Object (File)
Fri, Jul 11, 5:10 PM
Unknown Object (File)
Thu, Jul 10, 4:57 AM
Unknown Object (File)
Wed, Jul 9, 11:51 AM
Unknown Object (File)
Tue, Jul 8, 6:01 PM
Unknown Object (File)
Mon, Jul 7, 3:14 PM
Unknown Object (File)
Sun, Jul 6, 6:27 PM
View All 22 Files
Subscribers
farrokhi
glebius
imp
melifaro

Details

Reviewers
kp
Commits
rGcbd06dd2afd5: pf: Fix error handling when pf_map_addr() fails
Summary

When pf_map_addr() fails, for example for a NAT pool, we expect packet will
not be forwarded. The error returned by pf_map_addr() has been ignored in
pf_map_addr_sn(), though, causing packets being forwarded without NAT
applied. Catch the error, return the error to caller, let the caller handle
error counters for route-to pools just like it does for NAT pools. Add
tests for NAT and route-to rules.

Improve logging by not hardcoding function name and use func
instead.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

vegeta_tuxpowered.net created this revision.Jun 9 2025, 8:44 PM
Herald added subscribers: glebius, melifaro, farrokhi, imp. · View Herald TranscriptJun 9 2025, 8:44 PM
vegeta_tuxpowered.net requested review of this revision.Jun 9 2025, 8:44 PM
kp added inline comments.Jun 10 2025, 5:12 PM
sys/netpfil/pf/pf_lb.c
813

We probably want that error log being PF_DEBUG_NOISY, or at least some increased verbosity log level.

tests/sys/netpfil/pf/nat.sh
803

That might be better done with atf_check -o "match:map-failed\s+1\s+" or something like it.

vegeta_tuxpowered.net updated this revision to Diff 158375.Sat, Jul 12, 3:23 PM
vegeta_tuxpowered.net updated this revision to Diff 158376.
vegeta_tuxpowered.net marked 2 inline comments as done.
vegeta_tuxpowered.net updated this revision to Diff 158377.Sat, Jul 12, 3:28 PM
vegeta_tuxpowered.net marked an inline comment as not done.
vegeta_tuxpowered.net added inline comments.
sys/netpfil/pf/pf_lb.c
813

I've decided to go with PF_DEBUG_MISC, that is the same level as for example pfsync and state tracking use.

kp accepted this revision.Sat, Jul 12, 4:09 PM

Approved.

sys/netpfil/pf/pf_lb.c
814

I'd print the reason here. That makes debugging easier, and we've got it already so it's easy to do.

This revision is now accepted and ready to land.Sat, Jul 12, 4:09 PM
Closed by commit rGcbd06dd2afd5: pf: Fix error handling when pf_map_addr() fails (authored by vegeta_tuxpowered.net). · Explain WhySun, Jul 13, 11:53 AM
This revision was automatically updated to reflect the committed changes.
vegeta_tuxpowered.net marked an inline comment as not done.
vegeta_tuxpowered.net added a commit: rGcbd06dd2afd5: pf: Fix error handling when pf_map_addr() fails.

Revision Contents
Changeset List

PathSize
sys/
netpfil/
pf/
9 lines
17 lines
tests/
sys/
netpfil/
pf/
32 lines
32 lines

Diff 156732

View Options

sys/netpfil/pf/pf.c

Loading...
View Options

sys/netpfil/pf/pf_lb.c

Loading...
View Options

tests/sys/netpfil/pf/nat.sh

Loading...
View Options

tests/sys/netpfil/pf/route_to.sh

Loading...