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>
The early return duplicated the length check, making the custom error
path unreachable. As a result, a nil error was wrapped and returned
instead of the intended error message.
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
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>
Now `dockerd-rootless.sh` launches RootlessKit with `--detach-netns`
so as to run the daemon in the host network namespace.
The libnetwork namespaces are allocated inside the "detached" netns
(`$ROOTLESSKIT_STATE_DIR/netns`) that is associated with slirp4netns,
vpnkit, pasta, etc., as the rootless daemon has no `CAP_NET_ADMIN` for
the host network namespace.
This will enable:
- Accelerated (and deflaked) `docker pull`, `docker push`, `docker build`, etc
- Proper support for `docker pull 127.0.0.1:.../...`
- Proper support for `dockern run --net=host`
See also:
- rootless-containers/rootlesskit PR 379
- containerd/nerdctl PR 2723
NOTE: libnetwork contains code generated by Claude Code
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
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>
In rootless mode, the Engine needs to call the rootless port driver to
know which IP address it should bind to inside of its network namespace.
The slirp4netns port drivers doesn't support binding to IPv6 address, so
we need to detect that before listening on the port.
Before commit 201968cc0, this wasn't a problem because the Engine was
binding the port, then calling rootless port driver to learn whether the
proto/IP family was supported, and listen on the port if so.
Starting with that commit, the Engine does bind + listen in one go, and
then calls the port driver — this is too late. Fix the bug by checking
if the port driver supports the PortBindingReq, and only allocate the
port if so.
Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>
When the bridge driver encounters an error during network
creation, delete the bridge device if one has been added.
Signed-off-by: Rob Murray <rob.murray@docker.com>
These utilities are very handy to use in integration tests, too. Move
the package so it can be imported by them.
Signed-off-by: Cory Snider <csnider@mirantis.com>
These utilities are going to be needed elsewhere in the daemon to handle
netip values from API requests.
Signed-off-by: Cory Snider <csnider@mirantis.com>
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>
When running:
docker network create --ipv6 b46
docker run --rm -ti \
--network name=b46,driver-opt=com.docker.network.endpoint.sysctls=net.ipv6.conf.IFNAME.disable_ipv6=1 \
busybox
IPv6 is enabled in the container and the network, so an IPv6 address
will be allocated for the endpoint.
But, when the sysctl is applied, the IPv6 address will be removed
from the interface ... so, no unsolicited neighbour advertisement
should be (or can be) sent and, the endpoint should not be treated
as dual-stack when selecting a gateway endpoint and, if it is
selected as the gateway endpoint, setting up an IPv6 route via the
network will fail.
So, if the IPv6 address disappears after sysctls have been applied,
release the address and remove it from the endpoint's config.
Signed-off-by: Rob Murray <rob.murray@docker.com>
When macvlan is given no '--gateway' option, an address is
allocated from IPAM and a default route is set up via that
gateway. But, the gateway address is not assigned to anything
in the Docker macvlan network - it must be external, and
IPAM shouldn't try to guess it.
When IPv6 auto-configuration is enabled in the network the
macvlan is connected to, the macvlan driver races against it
to set up the gateway. When autoconfig wins, container creation
fails because the default route already exists.
So ...
- disable IPAM gateway address allocation for macvlan
- update the Join code to allow for no configured gateway
- always disable 'docker_gwbridge' connection for macvlan
networks, so it's not hooked up when there is no gateway
address.
Libnet assumes an endpoint with no statically configured default
gateway or route does not provide external connectivity. So, it
disables external DNS access, and will not select the endpoint
as gateway for containers. So, where an IPAM allocated gateway
address would have been assigned before, tell libnet to assume
there will be an auto-configured gateway.
Signed-off-by: Rob Murray <rob.murray@docker.com>
newDriver, which creates a new instance of the bridge driver, is the
only place where the driver config field is set. So there's no need to
gate access to it with a mutex.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Libnetwork passes a map[string]any to the bridge driver's Register
function. This forces the daemon to convert its configuration into a
map, and the driver to convert that map back into a struct.
This is unnecessary complexity, and makes it harder to track down where
and how bridge driver configuration fields are set.
Refactor libnetwork to let the daemon register the bridge.Configuration
directly through a new option `OptionBridgeConfig`.
The bridge driver now takes a `Configuration` param that needs no
special treatment.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This method is only used by the cnmallocator to allocate Swarm-scoped
network resources. Its only concrete implementation is in the ovmanager.
Other network drivers are implementing it too to adhere to the
driverapi.Driver interface, but they all return a 'not implemented'
error.
Extract this method into a separate interface, and add a dedicated
RegisterNetworkAllocator to the driver registry. Update the cnmallocator
to load 'network allocators' instead of 'drivers'.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The cnmallocator package has a map of supported network drivers which
are registered using a pkg-local driver registry. This registry is then
used to load drivers, and if they have a 'local' DataScope, they aren't
used for anything. Drivers with a 'global' DataScope are called to
allocate cluster-wide network resources.
Instantiating builtin network drivers may have unintended side-effects
(e.g. the bridge driver registers a callback that should run when
firewalld is reloaded), so libnetwork has dummy '*manager' drivers that
do nothing but carry the same Capability than the original driver they
masquerade.
Put 'local drivers' (e.g. those with DataScope 'local') into a separate
list that just contains drivers' name, and don't register them into the
cnmallocator's driver registry.
Remove all the dummy '*manager' drivers as they're not needed anymore.
Signed-off-by: Albin Kerouanton <albinker@gmail.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>
Instead of setting up firewall rules directly in the routed port mapper,
we now rely on the bridge driver to handle firewall reconfiguration.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Since commit 4e246efcd, individual portmappers are responsible for
setting up firewall rules and proxying according to their needs.
This change moves the responsibility back to the bridge driver, removing
unnecessary code duplication across portmappers. For now, only the nat
portmapper takes advantage of this.
This partially reverts commit 4e246efcd.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
When a endpoint's net.IPNet is loaded from store and converted
to a netip.Addr, unmap it so that iptables rules don't contain
IPv4-mapped IPv6 addresses.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Previous commit changed the OSAllocator to listen after binding a port,
such that we're 100% sure that the port is free. We can now make the
OSAllocator responsible for retrying port allocations when it tries to
find an ephemeral port, or a free port in a range.
Move the retry logic from the 'nat' portmapper to the OSAllocator.
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>
