Skip to content

Conversation

robmry
Copy link
Contributor

@robmry robmry commented Aug 7, 2025

Description

  • Some clarifications and a hopefully-gentler intro for the networking overview page
  • Bridge driver - add some more intro words
  • With nftables on the way - refer to "firewall" instead of "iptables" in the top-level description in packet-filtering-firewalls.md, move out the iptables specifics, and port-publishing (which applies to both iptables and nftables).
  • Add firewall-nftables.md describing migration to experimental --firewall-backend=nftables.

Reviews

Copy link

netlify bot commented Aug 7, 2025

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit f312d53
🔍 Latest deploy log https://app.netlify.com/projects/docsdocker/deploys/68a82e3a6e723d0008c7a6d3
😎 Deploy Preview https://deploy-preview-23215--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions bot added area/engine Issue affects Docker engine/daemon area/networking Relates to anything around networking labels Aug 7, 2025
@robmry robmry changed the title Engine networking overview updates Engine networking updates for moby 29.0 Aug 7, 2025
@robmry robmry force-pushed the moby_networking_tweaks branch 3 times, most recently from fbdde7d to 4309ba9 Compare August 8, 2025 11:21
However, this is not recommended for most users as it will likely break
container networking.

### Docker and iptables chains
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth mentioning the raw-PREROUTING chain somewhere (but this may not be the most appropriate section though)?

Comment on lines 142 to 143
In `nat` mode, when a port is published to a specific host address, that
port is only accessible via the host interface with that address. So,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not true. If a Docker host has two NICs with addresses 192.168.100.10/24 and 10.0.0.10/24, IP forwarding is enabled, and a port published on 192.168.100.10, another host in the 10.0.0.0/24 subnet can access that port if its routing table has an entry for routing 192.168.100.10 via 10.0.0.10.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬

Copy link
Contributor Author

@robmry robmry Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬

I can pick this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rearranged the surrounding paragraphs a bit, and added a "Note" block about direct routing to addresses on other host interfaces.

Comment on lines 144 to 145
for example, publishing a port to an address on the loopback interface
means remote hosts cannot access it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this part is true as loopback addresses aren't intended to be routed.

Comment on lines +290 to +306
This changes the default binding address to `127.0.0.1` for published container
ports on the default bridge network.
Restart the daemon for this change to take effect.
Alternatively, you can use the `dockerd --ip` flag when starting the daemon.
Copy link
Member

@akerouanton akerouanton Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rendered as a single line (see here), so no need to put extra new lines in the md file.

Copy link
Contributor Author

@robmry robmry Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rendered as a single line (see here), so no need to put extra new lines in the md file.

But semantic linefeeds are a thing.

@ArthurFlag ArthurFlag changed the base branch from main to moby29 August 22, 2025 08:59
Copy link
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize that one of my earlier suggestion is slightly wrong 🙈 Otherwise, LGTM.

Copy link
Contributor Author

@robmry robmry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArthurFlag - thank you for making the updates, I think they're all related to pre-existing text that moved, so maybe a review on my own PR counts (!) ... they LGTM.

@@ -44,8 +44,7 @@ Here are some examples:
> the outside world as well.
>
> If you include the localhost IP address (`127.0.0.1`, or `::1`) with the
> publish flag, only the Docker host and its containers can access the
> published container port.
> publish flag, only the Docker host.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> publish flag, only the Docker host.
> publish flag, only the Docker host can access the published container port.

Comment on lines +290 to +306
This changes the default binding address to `127.0.0.1` for published container
ports on the default bridge network.
Restart the daemon for this change to take effect.
Alternatively, you can use the `dockerd --ip` flag when starting the daemon.
Copy link
Contributor Author

@robmry robmry Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rendered as a single line (see here), so no need to put extra new lines in the md file.

But semantic linefeeds are a thing.

Comment on lines 142 to 143
In `nat` mode, when a port is published to a specific host address, that
port is only accessible via the host interface with that address. So,
Copy link
Contributor Author

@robmry robmry Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬

I can pick this up.

@robmry
Copy link
Contributor Author

robmry commented Aug 26, 2025

@ArthurFlag - shall I squash the review-comment commits to tidy up the history?

Not sure what happened in the "path-warnings" test, it says ...

#18 [npm 2/2] RUN --mount=source=package.json,target=package.json     --mount=source=package-lock.json,target=package-lock.json     --mount=type=cache,target=/root/.npm     npm ci
#18 1.262 npm error code E403
#18 1.262 npm error 403 403 Forbidden - GET https://registry.npmjs.org/yallist/-/yallist-5.0.0.tgz
#18 1.262 npm error 403 In most cases, you or one of your dependencies are requesting
#18 1.262 npm error 403 a package version that is forbidden by your security policy, or
#18 1.262 npm error 403 on a server you do not have access to.
#18 1.264 npm error A complete log of this run can be found in: /root/.npm/_logs/2025-08-26T13_56_56_669Z-debug-0.log
#18 ERROR: process "/bin/sh -c npm ci" did not complete successfully: exit code: 1

@ArthurFlag
Copy link
Contributor

ArthurFlag commented Aug 26, 2025

No need to squash the history, you can just squash and merge when ready :)

Great work 🙇

Oh and yeah, the npm registry is struggling today. These are non-required tests, no worries.

robmry added 4 commits August 27, 2025 17:57
Signed-off-by: Rob Murray <rob.murray@docker.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
With nftables on the way - refer to "firewall" instead of
"iptables" in the top-level description of packet-filtering-firewalls,
move out the iptables specifics, and port-publishing (which
applies to both iptables and nftables).

Signed-off-by: Rob Murray <rob.murray@docker.com>
Adds engine/network/firewall-nftables.md

Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry force-pushed the moby_networking_tweaks branch from ea7e170 to 2b123b9 Compare August 27, 2025 16:58
@robmry
Copy link
Contributor Author

robmry commented Aug 27, 2025

No need to squash the history, you can just squash and merge when ready :)

Thanks Arthur. It's probably marginally worth keeping the original four commits separate, so I squashed the updates into them.

I don't have merge-powers here, but I see the target's now branch moby29, so I guess it can be merged when @akerouanton is happy with the updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine Issue affects Docker engine/daemon area/networking Relates to anything around networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants