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

slirp: fix setup on ipv6 disabled systems #13485

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Mar 11, 2022

When enable_ipv6=true is set for slirp4netns (default since podman v4),
we will try to set the accept sysctl. This sysctl will not exist on
systems that have ipv6 disabled. In this case we should not error and
just ignore the extra ipv6 setup.

Also the current logic to wait for the slirp4 setup was kinda broken, it
did not actually wait until the sysctl was set before starting slirp.
This should now be fixed by using two sync.WaitGroups.

Fixes #13388

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 11, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2022
@baude
Copy link
Member

baude commented Mar 11, 2022

code LGTM

@rhatdan
Copy link
Member

rhatdan commented Mar 13, 2022

LGTM
Needs [NO NEW TESTS NEEDED] Flag.

Should we start erroring out if the user specifies --ipv6 options.

When enable_ipv6=true is set for slirp4netns (default since podman v4),
we will try to set the accept sysctl. This sysctl will not exist on
systems that have ipv6 disabled. In this case we should not error and
just ignore the extra ipv6 setup.

Also the current logic to wait for the slirp4 setup was kinda broken, it
did not actually wait until the sysctl was set before starting slirp.
This should now be fixed by using two `sync.WaitGroup`s.

[NO NEW TESTS NEEDED]

Fixes containers#13388

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99
Copy link
Member Author

Luap99 commented Mar 14, 2022

I confirmed that this patch works on a system with ipv6.disable=1 as kernel parameter.

@Luap99
Copy link
Member Author

Luap99 commented Mar 14, 2022

Should we start erroring out if the user specifies --ipv6 options.

This is not about --ipv6, see commit message, --network slirp4netns:enable_ipv6=true is the default for podman 4.0 so we cannot make this error.

@baude
Copy link
Member

baude commented Mar 14, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2022
@openshift-merge-robot openshift-merge-robot merged commit ae7997a into containers:main Mar 14, 2022
@Luap99 Luap99 deleted the ipv6-slirp branch March 14, 2022 20:06
@baude
Copy link
Member

baude commented Mar 16, 2022

/cherrypick v4.0

@openshift-cherrypick-robot
Copy link
Collaborator

@baude: only containers org members may request cherry picks. You can still do the cherry-pick manually.

In response to this:

/cherrypick v4.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@baude
Copy link
Member

baude commented Mar 17, 2022

/cherrypick v4.0

@openshift-cherrypick-robot
Copy link
Collaborator

@baude: only containers org members may request cherry picks. You can still do the cherry-pick manually.

In response to this:

/cherrypick v4.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mheon
Copy link
Member

mheon commented Mar 17, 2022

/cherrypick v4.0

@openshift-cherrypick-robot
Copy link
Collaborator

@mheon: only containers org members may request cherry picks. You can still do the cherry-pick manually.

In response to this:

/cherrypick v4.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@baude
Copy link
Member

baude commented Mar 17, 2022

/cherrypick v4.0

@openshift-cherrypick-robot
Copy link
Collaborator

@baude: new pull request created: #13546

In response to this:

/cherrypick v4.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@baude
Copy link
Member

baude commented Mar 18, 2022

/cherrypick v4.0-rhel

@openshift-cherrypick-robot
Copy link
Collaborator

@baude: new pull request created: #13555

In response to this:

/cherrypick v4.0-rhel

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman 4.0 hangs indefinitely if ipv6 is disabled on system
6 participants