Page MenuHomeFreeBSD

vnet: (read) lock the vnet list while iterating it
ClosedPublic

Authored by kp on Dec 6 2023, 2:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 9:55 AM
Unknown Object (File)
Sat, Nov 30, 4:03 PM
Unknown Object (File)
Nov 15 2024, 10:02 AM
Unknown Object (File)
Nov 15 2024, 9:52 AM
Unknown Object (File)
Nov 15 2024, 7:26 AM
Unknown Object (File)
Sep 30 2024, 12:14 AM
Unknown Object (File)
Sep 28 2024, 7:19 AM
Unknown Object (File)
Sep 27 2024, 4:54 PM

Details

Summary

Ensure that the vnet list cannot be modified while we're running through
it.

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 requested review of this revision.Dec 6 2023, 2:48 PM
mjg added inline comments.
sys/net/vnet.c
607

you should unlock in the same order for consistency

the VNET_FOREACH macro should presumably assert on the lock?

sys/net/vnet.c
607

It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand.
So we'd have to MPASS(sx_locked(&vnet_sxlock) || rw_rowned(&vnet_rwlock));. Which is doubly annoying because rw_rowned() doesn't exist.

sys/net/vnet.c
607

that is a gap i'm going to plug

This revision is now accepted and ready to land.Dec 6 2023, 3:04 PM
zlei added a subscriber: zlei.
zlei added inline comments.
sys/net/vnet.c
596

In vnet_register_sysinit() we have lock order

VNET_SYSINIT_WLOCK();
VNET_LIST_RLOCK;

I think it is best to make lock order consistent with that in vnet_register_sysinit(), although dead lock is not possible.

607

you should unlock in the same order for consistency

I'd prefer to unlock in the reverse order. That is more natural for me.
Anyway unlock order does not matter much.

the VNET_FOREACH macro should presumably assert on the lock?

ipreass_drain() in sys/netinet/ip_reass.c also miss read lock.
I think it is good to have read lock assert in macro VNET_FOREACH .