-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[Bugfix] use AF_INET6 instead of AF_INET for OpenAI Compatible Server #9583
Conversation
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can do one of these:
🚀 |
Signed-off-by: xiaozijin <[email protected]> use AF_INET6 instead of AF_INET fallback to AF_INET if IPv6 is not available refactory create socket refactory create socket style and logging fix style and logging fix style fix style fix
there's a simple implemention addr = ("", args.port)
if socket.has_dualstack_ipv6():
sock = socket.create_server(addr, family=socket.AF_INET6, dualstack_ipv6=True)
else:
sock = socket.create_server(addr) |
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
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.
This looks great now. Thanks for the IPv6 fix!
AF_INET6 does not support both ipv4 and ipv6 by default. It can be ipv6 only, so we should not use it here by default. |
hi @njhill please review this pr at your convenience and share more suggesstions :) |
It looks like the suggested change from previous review still needs to be completed. |
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
Signed-off-by: xiaozijin <[email protected]>
should all fixed and ci passed, please check again :) |
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.
lgtm!
It's ready for another look from you, @youkaichao
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.
thanks for the fix!
Currently the OpenAI Compatible Server creates a socket outside of
vllm.entrypoints.launcher.serve_http
, and this socket uses thesocket.AF_INET
address family. On machines with only IPv6 addresses, this limitation prevents the socket from being accessed externally.I made a small modification, changing it to
socket.AF_INET6
, so that it supports both IPv4 and IPv6 addresses.