Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (M of N)
ClosedPublic
Actions

Authored by kaktus on Feb 11 2020, 9:11 PM.

Details

Reviewers

kib
kp
glebius

Commits

rS358223: Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (6 of many)

Summary

r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are still not MPSAFE (or already are but aren’t properly marked). Use it in preparation for a general review of all nodes.
This is non-functional change that adds annotations to SYSCTL_NODE and SYSCTL_PROC nodes using one of the soon-to-be-required flags.
Mark everything as MPSAFE.

You’re asked for a review based on src/MAINTAINERS entry.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kaktus created this revision.Feb 11 2020, 9:11 PM

Herald added a subscriber: farrokhi. · View Herald TranscriptFeb 11 2020, 9:11 PM

kaktus added inline comments.Feb 16 2020, 11:50 AM

sys/netinet/ip_carp.c
225 ↗	(On Diff #68146)	MPSAFE
229 ↗	(On Diff #68146)	Require some locking? Or is the path too short and we don't care?
238 ↗	(On Diff #68146)	Probably MPSAFE? Use atomic to change the value, but sends the notification via taskqueue(9).

Herald added a subscriber: melifaro. · View Herald TranscriptFeb 16 2020, 11:50 AM

kaktus mentioned this in D23110: Introduce CTLFLAG_NEEDGIANT.Feb 16 2020, 5:18 PM

I'm not sure why we care about CTLFLAG_MPSAFE on nodes with no handler (pf, pfsync), but I have no objections.

sys/netinet/ip_carp.c
229 ↗	(On Diff #68146)	I think it's just safe. All we do is overwrite one value, which ought to be atomic all by itself. Even if it's not, the consequences would be limited to a one-off packet with an odd DSCP value.
238 ↗	(On Diff #68146)	Yeah, that looks safe to me too.