Table.Close() had a value receiver, so setting t.t = nil only modified
the callee's copy of the handle. The caller's Table was left looking
valid:
t, _ := nftables.NewTable(...)
t.Close()
t.IsValid() // true
Worse, Close() only dropped the nftables handle, and nftApply() opens a
new one whenever it finds none. So Apply() and Reload() on a closed table
silently reopened a handle and carried on updating the ruleset.
At the root of it, a Table looked like a plain value but behaved like a
reference, and nothing stopped it from being copied. So embed table in
Table by value and hand out *Table instead. The Table/table split is
still needed - table's fields have to be exported for text/template -
but reference semantics are now visible at every call site, and they're
enforced: because table contains a sync.Mutex, "go vet" reports both a
copy of a Table and a method or function that takes one by value, so the
shape of this bug is no longer expressible.
Close() therefore can't invalidate the table by clearing a pointer, it
has to record the state. Add a closed flag, and refuse to open a new
nftables handle for a table that's been closed.
Apply() checked neither for a closed table nor for a nil *Table, which
would have panicked. It now reports an error, checking the closed state
with applyLock held so that it can't race with Close(), and before the
in-memory table is touched so that a rejected update isn't recorded as
applied.
The invalid table is now a nil *Table rather than a zero-value Table,
which also removes the need for consumers to return an empty Table
alongside an error. That made it obvious that the nftabler was leaking
the table it had just created when it gave up on setting up IPv6, so
close it.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correct the version referenced in the comment describing the migration
performed by Network.dropLegacyFilterDirectAccess.
The migration was introduced in v28.2.0, not v28.0.2. Also update the
TODO to clarify that the migration can be removed once we no longer
expect upgrades from v28.0.x or v28.1.x directly to a release that
includes the migration.
Refs: a0ff0a361e ("libnetwork: drop
legacy direct access filter rules")
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
An nftables vmap is just a map whose element values are of type
`verdict`. Generalize VMap, VMapElement and Set to support any element
type.
Add a fluent API to build arbitrary tuple and mapping types from the
composition of other types or the 'typeof' an nftables expression.
Block the invalid composition of named types and `typeof` expressions at
compile time.
There are only a handful of contexts where the data type needs to be
specified: set and map definitions. Modelling set and map types as a
singular "nft type" does not align well with the semantics of nftables.
Map types are always composite types with a key and a value part. Set
types do not have a value part; it is an error to create a map with a
set type or vice versa. Encode this distinction into the Go type system
so it is a compile-time error to try to use a set type in a map context
or a map type in a set context.
Drop the 'NftType' prefix from the primitive set-type constants. The
prefix stutters with the package name and, as discussed above, it is not
accurate to call them "nft types." Verdicts cannot be used as set
elements or map keys. Provide dedicated methods to construct verdict-map
types from set types instead of modelling verdicts as types themselves.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The Cleanup function oportunistically tries to cleanup old rule (if any).
In cases where the rules didn't exist, it would log the error returned
by the command, but this would always be `exit status 1`, which doesn't
provide details about the actual failure (i.e., if it's the expected
"does not exist").
We could improve the code by first calling `nft list table` to check if
rules were found, but that would expose a similar problem (error could
be due to the rules not present, or "other cause".
This patch just changes the logs to be more informative, and passes the
context to allow cancelling the command if the context is cancelled.
Before this patch:
INFO[2026-02-21T13:30:10.428457589Z] Deleting nftables IPv4 rules error="exit status 1"
INFO[2026-02-21T13:30:10.453012214Z] Deleting nftables IPv6 rules error="exit status 1"
With this patch:
INFO[2026-02-21T14:10:06.183933878Z] Deleting nftables IPv4 rules error="exit status 1" output="Error: Could not process rule: No such file or directory\ndelete table ip docker-bridges"
INFO[2026-02-21T14:10:06.212198878Z] Deleting nftables IPv6 rules error="exit status 1" output="Error: Could not process rule: No such file or directory\ndelete table ip6 docker-bridges"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Allow tests to run in parallel with separate network namespaces,
without modifying the global-state namespace/netlink handles in
the "ns" package ... only useful for tests that don't depend on
package "ns".
Use the new option in iptabler/nftabler tests.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Add nftables.Modifier, to hold a queue of commands that can be applied
using Modifier.Apply. No updates are made to the underlying Table
until Apply is called, errors in the queue if commands are deferred
until Apply.
This has the advantages that:
- less error handling is needed in code that generates update commands
- it's transactional, without needing explicit transactions
Minor disadvantages are that it's slightly more difficult to debug updates,
as it's no longer possible to step through the call making an update to
the Table manipulation in a debugger - and errors in the command, and
errors like trying to update a nonexistent chain/set/vmap, deleting an
object that doesn't exist or creating a duplicate are not reported
until the updates are applied (but, the file/line where the rule was
added is reported).
Signed-off-by: Rob Murray <rob.murray@docker.com>
When nftablesdoc tests dump the state of nftables, the argument '-y' /
'--numeric-priority' isn't used, so all priorities should be
stringified. However, there's a bug in older versions of nftables that
prevents the stringification of the 'dstnat' priority — it's currently
dumped as '-100'.
New versions fix that, and thus running these tests on Debian 13 fails
because of this discrepancy with golden files.
So, look for 'type nat hook output priority -100' and stringify the
priority to ensure compatibility across versions of nft.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The `TransporPort` type is comparable; it doesn't have fields that
require special handling. It's defined as;
// TransportPort represents a local Layer 4 endpoint
type TransportPort struct {
Proto Protocol
Port uint16
}
where `Protocol` is an int (with a stringer interface);
type Protocol uint8
So we can remove the `Equal` method, and simplify places where it's
compared.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `GetCopy()` function doesn't de-reference anything, as it's
all a straight copy. We can remove it as it's only making things
more complicated than needed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Change `PortBinding.Equal` to use a value receiver and parameter, this
allows us to use it directly with `slices.IndexFunc`, `DeleteFunc`,
without having to add a wrapper func.
The only exception currently is the `UnmapPorts` function (stub), which
takes portmapperapi.PortBinding as argument; the portmapperapi.PortBinding
type embeds `types.PortBinding`, and it's the only field that's compared
as part of `UnmapPorts`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Rename `PortBinding.GetCopy()` to `PortBinding.Copy()`, which is more
idiomatic, and aligns with other similar methods.
- Change it to a non-pointer receiver; `Copy` does not mutate state, and
the type should still be reasonably small.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For nftables only, never enable IP forwarding on the host. Instead,
return an error on network creation if forwarding is not enabled,
required by a bridge network, and --ip-forward=true.
If IPv4 forwarding is not enabled when the daemon is started with
nftables enabled and other config at defaults, the daemon will
exit when it tries to create the default bridge.
Otherwise, network creation will fail with an error if IPv4/IPv6
forwarding is not enabled when a network is created with IPv4/IPv6.
It's the user's responsibility to configure and secure their host
when they run Docker with nftables.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Since 28.0.0, an iptables rule related to SCTP has only been
included if escape hatch variable DOCKER_IPTABLES_SCTP_CHECKSUM=1
Nobody's reported that the escape hatch was needed, and the rule
it guards doesn't make sense. So, remove.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Packets with the given firewall mark are accepted by the bridge
driver's filter-FORWARD rules.
The value can either be an integer mark, or it can include a
mask in the format "<mark>/<mask>".
Signed-off-by: Rob Murray <rob.murray@docker.com>
