Page MenuHomeFreeBSD

pfsync: Make pfsync callbacks per-vnet
ClosedPublic

Authored by kp on Oct 10 2018, 3:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 22 2024, 10:46 PM
Unknown Object (File)
Apr 22 2024, 7:08 PM
Unknown Object (File)
Jan 24 2024, 1:10 AM
Unknown Object (File)
Dec 20 2023, 3:55 AM
Unknown Object (File)
Dec 2 2023, 4:43 PM
Unknown Object (File)
Sep 7 2023, 12:56 PM
Unknown Object (File)
Sep 7 2023, 12:55 PM
Unknown Object (File)
Sep 7 2023, 12:55 PM
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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.