-
Notifications
You must be signed in to change notification settings - Fork 882
Fix IPv6 Port Forwarding for the Bridge Driver #2604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @bboehmke |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks far better and cleaner as my try.
drivers/bridge/port_mapping.go
Outdated
| pb, err := n.allocatePortsInternal(ep.extConnConfig.PortBindings, ep.addr.IP, defHostIP, ulPxyEnabled) | ||
| if err != nil { | ||
| return nil, err | ||
| var containerIPv6 net.IP = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to set containerIPv6 explicit to nil?
var containerIPv6 net.IP should already be nil by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, thanks
| } | ||
|
|
||
| // IPv6 port binding excluding user land proxy | ||
| if n.driver.config.EnableIP6Tables && ep.addrv6 != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EnableIP6Tables check was removed completely.
Is there anything that prevents a creation of ip6tables rules in AppendForwardingTableEntry of the portmapper if EnableIP6Tables is not set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
libnetwork/drivers/bridge/setup_ip_tables.go
Line 184 in 5c6a95b
| n.portMapperV6.SetIptablesChain(natChain, n.getNetworkBridgeName()) |
is skipped,
pm.chain will be nil and will skip the iptables programminglibnetwork/portmapper/mapper_linux.go
Line 42 in 5c6a95b
| if pm.chain == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I see. That is for sure far cleaner than before.
75f4191 to
f0cba0e
Compare
1. Allocate either a IPv4 and/or IPv6 Port Binding (HostIP, HostPort, ContainerIP, ContainerPort) based on the input and system parameters 2. Update the userland proxy as well as dummy proxy (inside port mapper) to specifically listen on either the IPv4 or IPv6 network Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
f0cba0e to
28576a4
Compare
|
LGTM |
|
thanks @bboehmke can you please formally approve the PR, so I can merge it and vendor it into |
Brings in moby/libnetwork#2604 Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
Brings in moby/libnetwork#2604 Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com> Upstream-commit: 78eafdd947db80832c6d0985c81cb56944a23739 Component: engine
| listener, err := sctp.ListenSCTP("sctp", frontendAddr) | ||
| // detect version of hostIP to bind only to correct version | ||
| ipVersion := ipv4 | ||
| if frontendAddr.IPAddrs[0].IP.To4() == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arkodg curious: would this be an issue? https://twitter.com/bradfitz/status/1341979944151207937?s=20
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we currently don't support IPv4-mapped IPv6 addresses today, we build out a separate ipv4 and ipv6 stack
This looks like a breaking change since until now |
|
According to the Changelog [1] this is the only network related change in 20.10.2. There are no docker-proxy processes started. 20.10.1 starts a process for each port-forwarding. Example: |
I confirm, I have the same problem |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.


ContainerPort) based on the input and system parameters
specifically listen to either the IPv4 or IPv6 network
Signed-off-by: Arko Dasgupta arko.dasgupta@docker.com