Skip to content

Conversation

@robmry
Copy link
Contributor

@robmry robmry commented Jul 30, 2024

- What I did

  • Added top level network-create option EnableIPv4 (which can later be hooked up to an option --ipv4 in the CLI, equivalent to --ipv6).
  • Added equivalent driver option com.docker.network.enable_ipv4.
    • Like enable_ipv6, it can be set via default-network-opts.
  • Show EnableIPv4 alongside EnableIPv6 in "inspect" output.
  • Predefined Linux none/host and Windows null networks are created with EnableIPv4=false.
  • Other networks, including predefined Linux bridge default to EnableIPv4=true.\
  • Ignore (clear) EnableIPv4 in a network create request if the API version is less than 1.47.
  • Require --experimental to disable IPv4.

Follow-up PRs will make the option do-something.

- How I did it

The first commit here, bumping the API version to 1.47, is likely to disappear - as this will be merged after another PR that does the same thing. But, for now, it means there's somewhere to put an API version-history.md update.

The rest is fairly machanical copying of EnableIPv6 behaviour.

- How to verify it

Can't disable IPv4 without --experimental:

# docker network create -o com.docker.network.enable_ipv4=false n1
Error response from daemon: IPv4 can only be disabled if experimental features are enabled

With --experimental, can disable IPv4:

# docker network create -o com.docker.network.enable_ipv4=false n1
e3fc95637aa2d9902ed188960283e025b95521cd44c95a645e0b48e7548ef2e6
# docker network inspect n1
[
    {
        "Name": "n1",
        ...
        "EnableIPv4": false,
        "EnableIPv6": false,

Default for a new bridge network is true:

# docker network create n2
9d71a90d0353fd352d53495fca214fa1a8ddecaca90c959a4ef9ed2318438d05
# docker network inspect n2
[
    {
        "Name": "n2",
        "Id": "9d71a90d0353fd352d53495fca214fa1a8ddecaca90c959a4ef9ed2318438d05",
        ...
        "EnableIPv4": true,
        "EnableIPv6": false,

Predefined host network shows EnableIPv4:false, like the existing EnableIPv6:false:

# docker network inspect host
[
    {
        "Name": "host",
        ...
        "EnableIPv4": false,
        "EnableIPv6": false,

Predefined bridge network has EnableIPv4:true:

# docker network inspect bridge
[
    {
        "Name": "bridge",
        ...
        "Driver": "bridge",
        "EnableIPv4": true,
        "EnableIPv6": false,
       ...

Marshalling/unmarshalling a libnetwork.Network with EnableIPv4:true is covered in an updated unit test.

- Description for the changelog

- API Changes:
  - `POST /networks/create` now has an `EnableIPv4` field. Setting it to `false`
  disables IPv4 IPAM for the network. It can only be set to `false` if the
  daemon has experimental features enabled.
  - `GET /networks/{id}` now returns an `EnableIPv4` field showing whether the
  network has IPv4 IPAM enabled.

Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry added kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny area/networking Networking impact/api impact/changelog area/networking/ipv6 Networking labels Jul 30, 2024
@robmry robmry added this to the 28.0.0 milestone Jul 30, 2024
@robmry robmry self-assigned this Jul 30, 2024
@robmry robmry requested review from akerouanton and corhere July 30, 2024 19:24
Copy link
Contributor

@corhere corhere left a comment

Choose a reason for hiding this comment

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

LGTM! Small PRs for the win!

api/swagger.yaml Outdated
description: "Optional custom IP scheme for the network."
$ref: "#/definitions/IPAM"
EnableIPv4:
description: "Enable IPv4 on the network."
Copy link
Member

Choose a reason for hiding this comment

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

It'd great to leave a note about the experimentalness of this property, both to warn API users that its implementation / usage / semantics might evolve (although unlikely), and also to warn that it's not available if dockerd's experimental flag isn't set. For instance, we do something similar for CDISpecDirs.

return nil, errdefs.InvalidParameter(fmt.Errorf("driver-opt %q is not a valid bool", netlabel.EnableIPv4))
}
}
if !enableIPv4 && !daemon.config().Experimental && create.ConfigFrom == nil {
Copy link
Member

Choose a reason for hiding this comment

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

We're trying to move away from 'experimental' features as it's coarse-grained, to replace it with fine-grained feature flags. Would it make sense to introduce a new feature flag for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR on its own is basically-useless (!). It'll carry on being basically-useless for a few more PRs yet.

I don't think we've decided to release/announce IPv6-only (EnableIPv4:false) as experimental, or behind a feature flag - but, we could do either if we decide to release with it still incomplete. Depending on what that looks like, we might want to feature-flag it for a specific network driver or something like that. Or, if it looks risky and likely to change, we might make the whole thing experimental / feature-flagged until we get some feedback. But, I don't think we'll know until we're at that point.

So, just locking it behind --experimental for now seems like the best approach?

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, I won't dwell on that.

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.

LGTM

robmry added 3 commits July 31, 2024 18:38
Signed-off-by: Rob Murray <rob.murray@docker.com>
Similar to EnableIPv6:
- Set it if EnableIPv4 is specified in a create request.
- Otherwise, set it if included in `default-network-opts`.
  - Apart from in a config-from network, so that it doesn't look
    like the API request set the field.
- Include the new field in  Network marshalling/unmarshalling test.

Signed-off-by: Rob Murray <rob.murray@docker.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry force-pushed the v6only/add_option_enable_ipv4 branch from 3d1d1eb to 1f542d5 Compare July 31, 2024 17:39
@robmry
Copy link
Contributor Author

robmry commented Jul 31, 2024

I forgot to ignore EnableIPv4 in create requests using older versions of the API ... the latest push fixes that. Also, noted that --experimental is needed for EnableIPv4:false in the API docs.

EnableIPv4 isn't removed from inspect output for older versions of the API - which seems to be the approach we've taken in the past. (It'd be awkward to implement, as the response is a JSON marshalled version of network.Inspect, and omitempty would be wrong for newer versions of the API when EnableIPv4=false.)

@robmry robmry requested review from akerouanton and corhere July 31, 2024 17:43
@robmry robmry deleted the v6only/add_option_enable_ipv4 branch November 15, 2024 18:31
aevesdocker pushed a commit to docker/docs that referenced this pull request Feb 20, 2025
## Description

Updates for moby 28.0 networking.

## Related issues or tickets

Series of commits ...
- Fix description of 'inhibit_ipv4'
- Not changed in moby 28.0, updated to clarify difference from (new)
IPv6-only networks.
- Updates to default bridge address config
  - moby/moby#48319
- Describe IPv6-only network config
  - moby/moby#48271
  - docker/cli#5599
- Update description of gateway modes
  - moby/moby#48594
  - moby/moby#48596
  - moby/moby#48597
- Describe gateway selection in the networking overview.
  - docker/cli#5664
- Describe gateway mode `isolated`
  - moby/moby#49262

## Reviews

<!-- Notes for reviewers here -->
<!-- List applicable reviews (optionally @tag reviewers) -->

- [ ] Technical review
- [ ] Editorial review
- [ ] Product review

---------

Signed-off-by: Rob Murray <rob.murray@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking/ipv6 Networking area/networking Networking impact/api impact/changelog kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants