Differential D30946

pf: getstates: avoid taking the hashrow lock if the row is empty
ClosedPublic
Actions

Authored by kp on Jun 29 2021, 7:38 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Feb 20 2024, 11:09 PM

	Unknown Object (File)
	Feb 20 2024, 7:30 AM

	Unknown Object (File)
	Jan 29 2024, 4:20 AM

	Unknown Object (File)
	Dec 20 2023, 6:09 AM

	Unknown Object (File)
	Dec 16 2023, 3:26 AM

	Unknown Object (File)
	Dec 14 2023, 10:17 PM

	Unknown Object (File)
	Dec 2 2023, 7:03 PM

	Unknown Object (File)
	Nov 28 2023, 10:32 AM

View All 36 Files

Subscribers

Details

Reviewers

Group Reviewers

network
pfsense

Commits

rGb43db87c97dd: pf: getstates: avoid taking the hashrow lock if the row is empty
rG1882bdd192bc: pf: getstates: avoid taking the hashrow lock if the row is empty
rGa19ff8ce9b58: pf: getstates: avoid taking the hashrow lock if the row is empty

Summary

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kp created this revision.Jun 29 2021, 7:38 PM

Herald added subscribers: melifaro, farrokhi, imp. · View Herald TranscriptJun 29 2021, 7:38 PM

kp requested review of this revision.Jun 29 2021, 7:38 PM

Harbormaster completed remote builds in B40173: Diff 91523.Jun 29 2021, 7:38 PM

kp added a parent revision: D30945: libpfctl: improve DIOCGETSTATESNV size estimation.Jun 29 2021, 7:38 PM

kp added a child revision: D30947: pf: support paging in the DIOCGETSTATESNV call.

mjg accepted this revision.Jun 30 2021, 2:04 PM

This revision is now accepted and ready to land.Jun 30 2021, 2:04 PM

donner added a subscriber: donner.Jun 30 2021, 3:29 PM

donner added inline comments.

sys/netpfil/pf/pf_ioctl.c
5058	Accessing this value without a lock ... may it cause a race condition?

mjg added inline comments.Jun 30 2021, 3:34 PM

sys/netpfil/pf/pf_ioctl.c
5058	It is a data race, but it is tolerable in this case. Absolute worst which can happen is that the row will be skipped when it should not be. But let's say garbage was read. The acquire fence provided by PF_HASHROW_LOCK below makes the previously read possibly bogus state irrelevant as it is forcibly refreshed.

mjg added inline comments.Jun 30 2021, 3:35 PM

sys/netpfil/pf/pf_ioctl.c
5058	That said ideally it would use a data structure which is safe to iterate in presence of modification. I have a rough sketch to implement this, so the state as proposed here should be temporary.

Closed by commit rGa19ff8ce9b58: pf: getstates: avoid taking the hashrow lock if the row is empty (authored by kp). · Explain WhyJul 2 2021, 1:50 PM

This revision was automatically updated to reflect the committed changes.

kp added a commit: rGa19ff8ce9b58: pf: getstates: avoid taking the hashrow lock if the row is empty.

kp removed a child revision: D30947: pf: support paging in the DIOCGETSTATESNV call.Jul 5 2021, 10:10 AM

kp added a commit: rG1882bdd192bc: pf: getstates: avoid taking the hashrow lock if the row is empty.Jul 9 2021, 1:24 PM

kp added a commit: rGb43db87c97dd: pf: getstates: avoid taking the hashrow lock if the row is empty.

Revision Contents
Changeset List

Path

Size

sys/

netpfil/

pf/

4 lines

Diff 91665

sys/netpfil/pf/pf_ioctl.c

Loading...