Page MenuHomeFreeBSD
Differential D31739

pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules
ClosedPublic
Actions

Authored by kp on Aug 31 2021, 8:24 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Aug 10, 9:08 PM
Unknown Object (File)
Sat, Aug 8, 9:38 PM
Unknown Object (File)
Sat, Aug 8, 7:11 PM
Unknown Object (File)
Sat, Aug 8, 6:07 PM
Unknown Object (File)
Sat, Aug 8, 6:05 PM
Unknown Object (File)
Sat, Aug 8, 4:57 PM
Unknown Object (File)
Sat, Aug 8, 11:39 AM
Unknown Object (File)
Fri, Aug 7, 9:45 PM
View All Files
Subscribers
ae
farrokhi
glebius
imp
melifaro
mjg

Details

Reviewers
None
Group Reviewers
network
pfsense
Commits
rG20c4899a8eea: pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules
Summary

Avoid the overhead of acquiring a (read) RULES lock when processing the
Ethernet rules.
We can get away with that because when rules are modified they're staged
in V_pf_keth_inactive. We take care to ensure the swap to V_pf_keth is
atomic, so that pf_test_eth_rule() always sees either the old rules, or
the new ruleset.

We need to take care not to delete the old ruleset until we're sure no
pf_test_eth_rule() is still running with those. We accomplish that by
using NET_EPOCH_CALL() to actually free the old rules.

Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 41833
Build 38721: arc lint + arc unit

Event Timeline

kp created this revision.Aug 31 2021, 8:24 AM
Herald added subscribers: melifaro, farrokhi, ae, imp. · View Herald TranscriptAug 31 2021, 8:24 AM
kp requested review of this revision.Aug 31 2021, 8:24 AM
kp added a parent revision: D31738: pfctl: Print Ethernet rules.
kp added a child revision: D31740: pfctl: Move pf_eth_rule definition into pfctl.
Harbormaster completed remote builds in B41278: Diff 94415.Aug 31 2021, 8:25 AM
kp updated this revision to Diff 95940.Sep 29 2021, 3:00 PM

Rebase

Harbormaster completed remote builds in B41833: Diff 95940.Sep 29 2021, 3:00 PM
kp updated this revision to Diff 96814.Oct 13 2021, 3:31 PM

Rebase & cleanup

Harbormaster completed remote builds in B42107: Diff 96814.Oct 13 2021, 3:31 PM
mjg added a subscriber: mjg.Feb 16 2022, 7:53 AM

Why the use of ck_* instead of atomic_*?

Herald added a subscriber: glebius. · View Herald TranscriptFeb 16 2022, 7:53 AM
kp added a comment.Feb 16 2022, 2:45 PM
In D31739#776252, @mjg wrote:

Why the use of ck_* instead of atomic_*?

That'd probably work too, but we typically use the ck_* functions in combination with epoch. For example, in ifnet_byindex() or prison_ip_check().

This revision was not accepted when it landed; it landed in state Needs Review.Mar 2 2022, 4:01 PM
Closed by commit rG20c4899a8eea: pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules (authored by kp). · Explain Why
This revision was automatically updated to reflect the committed changes.
kp added a commit: rG20c4899a8eea: pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules.

Revision Contents
Changeset List

PathSize
sys/
net/
3 lines
netpfil/
pf/
20 lines
36 lines
1 line

Diff 95940

View Options

sys/net/pfvar.h

Loading...
View Options

sys/netpfil/pf/pf.c

Loading...
View Options

sys/netpfil/pf/pf_ioctl.c

Loading...
View Options

sys/netpfil/pf/pf_ruleset.c

Loading...