Skip to content

Conversation

@poettering
Copy link
Member

This PR is an alternative to @cdown's #17495 which stalled. The basic idea is the same: serialize the attached BPF program fd across reloads/reexecs so that we can close them only after installing the new BPF programs.

Contains a multitude of other BPF clean-ups.

@poettering poettering added the pid1 label Jun 8, 2021
@poettering poettering force-pushed the bpf-firewall-tweaks branch from da61ed5 to f7e8e80 Compare June 8, 2021 17:32
@poettering poettering added the bpf label Jun 8, 2021
@poettering poettering changed the title close bpf firewal reload gap close bpf firewall reload gap Jun 8, 2021
@yuwata yuwata added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jun 8, 2021
These are so many runtime objects, let's add a bpf_firewall_close()
helper that destroys them all, and call that from unit_free(), simply as
an excercise of encapsulating more BPF code in bpf-firewall.c.

This also brings the destruction order and variable declaration order in
struct Unit into the same systematic order.

No change in behaviour just some minor refactoring.
Mostly logging related: let's downgrade logging in dlopen_bpf() for
example, and remove duplicate logging at various places. Add %m to log
messages and so on.
The other BPF infra has a file name prefix of "bpf-" hence do so here
too.
we generally do this for all bpf functions, do so here too.
If we have BPF_F_ALLOW_MULTI support we can install the new program
before we drop the old (because we can install two program at the same
time). Let's do that, and thus fully close the firewall
gap.
@poettering poettering removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jun 8, 2021
@poettering poettering force-pushed the bpf-firewall-tweaks branch from f7e8e80 to dbef3d1 Compare June 8, 2021 20:06
@yuwata
Copy link
Member

yuwata commented Jun 8, 2021

LGTM.

@yuwata yuwata added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Jun 8, 2021
@poettering poettering merged commit bead169 into systemd:main Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bpf good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed pid1

Development

Successfully merging this pull request may close these issues.

2 participants