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.

Sign up for GitHub

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

Jump to bottom

Fix ipv4 ipv6 dualstack #2408

Merged
merged 2 commits into from
Jan 10, 2024
Merged

Fix ipv4 ipv6 dualstack #2408

merged 2 commits into from
Jan 10, 2024

Conversation

yunfeng-scale
Copy link
Contributor

@yunfeng-scale yunfeng-scale commented Jan 10, 2024 •
edited

Fixes #2407
socket.getaddrinfo only returns IPv6 addresses

simon-mo
simon-mo reviewed Jan 10, 2024
View reviewed changes
Copy link
Collaborator

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

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

the fix makes a lot of sense. i do wonder whether this work across public interface, private interface, and k8s based system

vllm/utils.py
@@ -58,7 +58,9 @@ def in_wsl() -> bool:


def get_ip() -> str:
return socket.gethostbyname(socket.gethostname())
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
s.connect(("8.8.8.8", 80)) # Doesn't need to be reachable
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happen if it is not reachable? will it timeout/error?

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 tried some other non-reachable addresses and this still works

@yunfeng-scale
Copy link
Contributor Author

the fix makes a lot of sense. i do wonder whether this work across public interface, private interface, and k8s based system

tested in k8s cluster and it fixes

@simon-mo simon-mo merged commit 4b61c6b into vllm-project:main Jan 10, 2024
2 of 4 checks passed
hongxiayang pushed a commit to hongxiayang/vllm that referenced this pull request Jan 18, 2024
@yunfeng-scale @hongxiayang
get_ip(): Fix ipv4 ipv6 dualstack (vllm-project#2408)
966ce34
hongxiayang pushed a commit to hongxiayang/vllm that referenced this pull request Feb 13, 2024
@yunfeng-scale
get_ip(): Fix ipv4 ipv6 dualstack (vllm-project#2408)
3105b73
@ZiyueHuang
Copy link

@simon-mo @yunfeng-scale I encountered the same issue (OSError: [Errno 101] Network is unreachable) as in #2857, which seems to be introduced by this PR.

I use s.connect(("0.0.0.0", 80)) instead, then it works fine. However, I'm not sure whether this is a proper or universal solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simon-mo simon-mo simon-mo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failed to get local ipv4 IP at start
3 participants
@yunfeng-scale @ZiyueHuang @simon-mo