Skip to content
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

Preserve link-local IPv6 on container's lo & allow link-local only IPv6 networks #1756

Closed
wants to merge 2 commits into from

Conversation

aboch
Copy link
Contributor

@aboch aboch commented May 14, 2017

Related to moby/moby/issues/33099
Overrides #1751

As explained in moby/moby#33099 (comment), it is better to keep IPv6 enabled on the container's loopback interface, for backward compatibility with applications which expect that. (We could do this only if enabled on the host, but doing it regardless seems more robust and should not cause any ipv6 advertise packet be sent out the container.)

Also, restore support for creating an --ipv6 network with no routable subnet. Libnetwork core will be ok if the IPAM driver cannot auto allocate a subnet for the network.

@@ -413,6 +418,10 @@ func (a *Allocator) getPredefinedPool(as string, ipV6 bool) (*net.IPNet, error)
}
}

if !supported {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm. wont this check cause a legitimate pool exhaustion mis-categorized as ErrNotSupported ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because getPredefined() returns the list of predefined pools, not the list of available predefined pools.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

What's the status on this one?

Left some nits 😄

network.go Outdated
if err == ipamapi.ErrNotSupported && cfg.PreferredPool == "" && ipVer == 6 {
*cfgList = nil
*infoList = nil
logrus.Infof("IPAM driver does not support autoallocation for IPv%d", ipVer)
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified to just;

logrus.Info("IPAM driver does not support auto-allocation for IPv6")

@@ -54,6 +54,7 @@ var (
ErrIPOutOfRange = types.BadRequestErrorf("Requested address is out of range")
ErrPoolOverlap = types.ForbiddenErrorf("Pool overlaps with other one on this address space")
ErrBadPool = types.BadRequestErrorf("Address space does not contain specified address pool")
ErrNotSupported = types.BadRequestErrorf("Do not support requested operation")
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps (to be consisted with other messages);

ErrNotSupported        = types.BadRequestErrorf("Requested operation is not supported")

@justincormack
Copy link
Contributor

ping, er I mean ping6 ::1 - what is that status here?

- For backward compatibility because some applications expect that

Signed-off-by: Alessandro Boch <aboch@docker.com>
@aboch
Copy link
Contributor Author

aboch commented Aug 4, 2017

Thanks @thaJeztah. Addressed your comments.

- In case the IPAM driver does not support auto-allocation
  of IPv6 address pools, libnetwork to be more lenient so
  that containers' interfaces can have link-local only addresses

Signed-off-by: Alessandro Boch <aboch@docker.com>
@thaJeztah
Copy link
Member

ping @fcrisciani @mavenugo @abhinandanpb PTAL

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants