Repository navigation
libnetwork: don't depend on nft when linked against libnftables - #52820
Merged
Merged
Conversation
corhere
added
kind/enhancement
There was a problem hiding this comment.
Pull request overview
This PR removes the hard dependency on the nft userspace binary when the daemon is built/linked with libnftables, so the nftables firewall backend can be enabled and stale tables can be cleaned up via libnftables APIs instead of shelling out to nft.
Changes:
- Replace the old “
nftPathfound in PATH” enablement gate with a build-dependent preflight (nftrequired only for exec-based builds; no-op for libnftables builds). - Introduce
RunCmd()to execute raw nftables command buffers via the selected backend (exec vs libnftables). - Update the bridge driver nftables cleaner to delete stale tables via
nftables.RunCmd()instead ofexec.Command("nft", ...).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| daemon/libnetwork/internal/nftables/nftables_linux.go | Switch enablement state to a boolean, add RunCmd(), and refactor table handle management. |
| daemon/libnetwork/internal/nftables/nft_exec_linux.go | Add exec-backend preflight for nft and refactor to a context object type. |
| daemon/libnetwork/internal/nftables/nft_cgo_linux.go | Add libnftables-backend preflight (no nft binary dependency) and refactor to a context object type. |
| daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go | Use nftables.RunCmd() for cleanup instead of invoking nft directly. |
Comment on lines
234
to
238
| func Disable() { | ||
| nftPath = "" | ||
| enabled = false | ||
| incrementalUpdateTempl = nil | ||
| reloadTempl = nil | ||
| enableOnce = sync.Once{} |
Comment on lines
+23
to
+25
| func preflight() error { | ||
| return nil | ||
| } |
corhere
force-pushed
the
libnftables-without-nft
branch
from
June 10, 2026 18:43
04c695a to
b547078
Compare
Comment on lines
233
to
239
| // Disable undoes Enable. Intended for unit testing. | ||
| func Disable() { | ||
| nftPath = "" | ||
| enabled = false | ||
| incrementalUpdateTempl = nil | ||
| reloadTempl = nil | ||
| enableOnce = sync.Once{} | ||
| } |
Comment on lines
+23
to
+30
| var lookPathNft = sync.OnceValues(func() (string, error) { | ||
| p, err := exec.LookPath("nft") | ||
| if err != nil { | ||
| log.G(context.Background()).WithError(err).Warnf("Failed to find nft tool") | ||
| return "", fmt.Errorf("failed to find nft tool: %w", err) | ||
| } | ||
| return p, nil | ||
| }) |
Comment on lines
26
to
31
| func tryCleanup(ctx context.Context, family nftables.Family, label string) { | ||
| cmd := exec.CommandContext(ctx, "nft", "delete", "table", string(family), dockerTable) | ||
| out, err := cmd.CombinedOutput() | ||
| err := nftables.RunCmd(ctx, fmt.Appendf(nil, "delete table %s %s", family, dockerTable)) | ||
| if err != nil { | ||
| // May not exist ("Error: Could not process rule: No such file or directory") | ||
| log.G(ctx).WithFields(log.Fields{ | ||
| "error": err, | ||
| "output": string(bytes.TrimRight(out, "\n ^")), // remove "^^^^^" added in nft's error message. | ||
| }).Info("Deleting nftables " + label + " rules") | ||
| log.G(ctx).WithError(err).Info("Deleting nftables " + label + " rules") | ||
| return |
Afford applying nft commands via libnftables without needing to go through our table abstraction. Make the table abstraction responsible for lazily allocating an nft context. Signed-off-by: Cory Snider <csnider@mirantis.com>
When the daemon is linked against libnftables it programs the kernel without invoking the `nft` command. Allow the nftables firewall backend to be enabled when libnftables is used, irrespective of whether `nft` is installed on the host. Update the bridge network driver to clean up stale nftables tables in iptables mode without depending on the `nft` command. Signed-off-by: Cory Snider <csnider@mirantis.com>
thaJeztah
force-pushed
the
libnftables-without-nft
branch
from
June 11, 2026 08:55
b547078 to
8e3e9f4
Compare
Member
|
Did a quick rebase after #52822 was merged (which fixed the CI failures) |
thaJeztah
reviewed
Jun 11, 2026
| func tryCleanup(ctx context.Context, family nftables.Family, label string) { | ||
| cmd := exec.CommandContext(ctx, "nft", "delete", "table", string(family), dockerTable) | ||
| out, err := cmd.CombinedOutput() | ||
| err := nftables.RunCmd(ctx, fmt.Appendf(nil, "delete table %s %s", family, dockerTable)) |
Member
There was a problem hiding this comment.
Starting to wonder if we should change nftables.RunCmd to accept a string instead of a byte-slice
Comment on lines
34
to
35
| cCmd := C.CString(string(nftCmd)) | ||
| defer C.free(unsafe.Pointer(cCmd)) |
Member
There was a problem hiding this comment.
Because it would still be converted to a string here, so we'd first allocate a byte-slice, then allocate a string here?
Or was there some path where it would still benefit from passing a byte-slice around?
Contributor
There was a problem hiding this comment.
I think it's a []byte because that's what the original exec version of nftApply needed (now in nft_exec_linux.go) ... so it could be changed to a string.
Member
There was a problem hiding this comment.
Ah, thanks for the context; it looks unrelated to this PR, so not a blocker for this one!
Member
|
let me bring this one in so that you can rebase #52833 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the daemon is linked against libnftables it programs the kernel without invoking the
nftcommand. Allow the nftables firewall backend to be enabled when libnftables is used, irrespective of whethernftis installed on the host.Update the bridge network driver to clean up stale nftables tables in iptables mode without depending on the
nftcommand.Release notes (optional)
A picture of a cute animal (not mandatory but encouraged)