Only enable bridge netfiltering when needed#48492
Conversation
robmry
added
area/networking
robmry
force-pushed
the
48375_bridge_netfiltering
branch
from
09119a0 to
0677efa
Compare
| if out, err := exec.Command("modprobe", "-va", "bridge", "br_netfilter").CombinedOutput(); err != nil { | ||
| log.G(context.TODO()).WithError(err).Warnf("Running modprobe bridge br_netfilter failed with message: %s", out) |
There was a problem hiding this comment.
Wondering if we should start looking at plumbing through context to these networking options (both for OTEL, as well as potentially context-cancellation (e.g. this could use exec.CommandContext).
Definitely not for this PR, but curious if it would make sense to start looking at.
There was a problem hiding this comment.
Yes, it would make sense.
| if _, err := os.Stat("/proc/sys/net/bridge"); err != nil { | ||
| if out, err := exec.Command("modprobe", "-va", "bridge", "br_netfilter").CombinedOutput(); err != nil { | ||
| log.G(context.TODO()).WithError(err).Warnf("Running modprobe bridge br_netfilter failed with message: %s", out) | ||
| return fmt.Errorf("modprobe br_netfilter failed: %w", err) |
There was a problem hiding this comment.
This now both returns and logs the error, whereas previously we would only log it; is there a risk that we're changing behavior?
(Also wondering if the logging should happen on the caller side now that we return these)?
There was a problem hiding this comment.
As it's an error now; perhaps the Warnf should become an Errorf now as well 🤔 (we tend to use "warning" for errors that we can recover from, but as we now no longer recover, it's an Error I guess?
There was a problem hiding this comment.
This now both returns and logs the error, whereas previously we would only log it; is there a risk that we're changing behavior?
If the modprobe failed on startup before, network creation with icc=false would have failed here, with the "cannot restrict inter-container communication" error (just below). I'll add "cannot restrict inter-container communication" to the modprobe error.
So, I don't think there's any change in behaviour, apart from the text in the error message.
(Also wondering if the logging should happen on the caller side now that we return these)?
I think that'd mean duplicating the log lines, would it be an improvement? We could just get rid of the log line, but it might still provide a useful clue for something.
As it's an error now; perhaps the Warnf should become an Errorf now as well 🤔 (we tend to use "warning" for errors that we can recover from, but as we now no longer recover, it's an Error I guess?
I'll change it to an Errorf. (If the modprobe fails, it now means an network create "icc=false" didn't work - before it meant a future network create with "icc=false" would fail if attempted. Recovery is the same as before, just reject the network creation. The rest of the daemon wouldn't be affected in either case.)
Kernel module br_netfilter is loaded when the daemon starts with either iptables or ip6tables enabled. That automatically sets: net.bridge.bridge-nf-call-arptables = 1 net.bridge.bridge-nf-call-ip6tables = 1 net.bridge.bridge-nf-call-iptables = 1 So, when: - docker was running happily with iptables=false, and - no explicit ip6tables=false, and - br_netfilter was not loaded ... the change in moby 27.0 to enable ip6tables by default, resulted in net.bridge.bridge-nf-call-iptables being enabled. If the host also had a firewall with default-drop on its forward chain - that resulted in packets getting dropped between containers on a bridge network. So, only try to load br_netfilter when it's needed - it's only needed to implement "--icc=false", which can only be used when iptables or ip6tables is enabled. Signed-off-by: Rob Murray <[email protected]>
robmry
force-pushed
the
48375_bridge_netfiltering
branch
from
0677efa to
db25b0d
Compare
3 participants
- What I did
- How I did it
Kernel module br_netfilter is loaded when the daemon starts with either iptables or ip6tables enabled. That automatically sets:
So, when:
... the change in moby 27.0 to enable ip6tables by default, resulted in
net.bridge.bridge-nf-call-iptablesbeing enabled, where it wasn't before.If the host also had a firewall with default-drop on its forward chain - that resulted in packets getting dropped between containers on a bridge network.
So, only try to load br_netfilter when it's needed - it's only needed to implement
--icc=false, which only works wheniptables/ip6tablesare enabled.- How to verify it
br_netfilterloaded)."iptables":false, and"ip6tables"left as default.docker network create br4.br4can communicate. For example:docker run --rm -d --name c1 --network br4 nginx; docker run --rm -ti --network br4 alpine wget -O- http://c1br_netfilterhas been loaded,rmmod br_netfilteror reboot to get rid of it.br_netfilterhasn't been loaded.sysctl -a | grep net.bridge.bridge-nf-call.docker network create --ipv6 -o com.docker.network.bridge.enable_icc=false iccf.br_netfilterhas been loaded.- Description for the changelog