Page MenuHomeFreeBSD

pfsync: Make pfsync callbacks per-vnet
ClosedPublic

Authored by kp on Oct 10 2018, 3:16 PM.
Tags
None
Referenced Files
F103557111: D17499.diff
Tue, Nov 26, 12:06 PM
Unknown Object (File)
Sun, Nov 24, 8:55 PM
Unknown Object (File)
Sat, Nov 23, 3:18 AM
Unknown Object (File)
Tue, Nov 19, 8:54 AM
Unknown Object (File)
Tue, Nov 5, 2:09 AM
Unknown Object (File)
Tue, Oct 29, 3:04 PM
Unknown Object (File)
Oct 19 2024, 12:26 AM
Unknown Object (File)
Oct 8 2024, 8:56 AM
Subscribers

Details

Summary

The callbacks are installed and removed depending on the state of the
pfsync device, which is per-vnet. The callbacks must also be per-vnet.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20114
Build 19608: arc lint + arc unit

Event Timeline

bz added a subscriber: bz.

I only scrolled through but it looks good to me.

This revision is now accepted and ready to land.Oct 10 2018, 5:17 PM
bz requested changes to this revision.Oct 10 2018, 5:26 PM

Actually, why is this needed?

This revision now requires changes to proceed.Oct 10 2018, 5:26 PM

Each vnet may choose to set pfsync0 down, which (see pfsyncioctl() / SIOCSIFFLAGS) set or clear these callbacks.
And while it's okay for a single vnet to decide it doesn't want pfsync to be enabled, it's not okay for that decision to affect all vnets.

OK. I was thinking (when reading the UNINIT change) that that only starts to make sense after virtualising these here; and then I wondered why I'd want to virtualise if these called functions would be perfectly fine and happy to determine if pfsync is enabled (RUNNING) in this vnet or not. That way there'd be less virtualisation.

However this solution does save the per-packet call into the callback functions, so is probably preferred?

This revision is now accepted and ready to land.Oct 10 2018, 6:49 PM

Yes. Especially because pfsync currently does a lot of work (including taking a single lock, which really kills throughput) even if it won't actually end up sending the sync packets out.
It should be possible to change pfsync so we don't virtualise the callback pointers and instead immediately check if it's enabled and return if not. We would have to be careful about locking around the up/down (and defer) flags, so it'd be a larger change than this.

This revision was automatically updated to reflect the committed changes.