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

Lint

Lint Skipped

Unit

Tests Skipped

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	MPSAFE
229	Require some locking? Or is the path too short and we don't care?
238	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	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	Yeah, that looks safe to me too.