Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add bootstrap APIs for VSOCK sockets #2479

Merged
merged 32 commits into from
Aug 3, 2023

Conversation

simonjbeaumont
Copy link
Contributor

@simonjbeaumont simonjbeaumont commented Jul 21, 2023

Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While NIO has support for building channels using sockets that were created out of band (withConnectedSocket(_:) and withBoundSocket(_:), there are no APIs that facilitate the creation of VSOCK-based channels.

Modifications:

Given AF_VSOCK is just another address family, extending enum SocketAddress with a new case would be the most natural approach. However this is a breaking API change, so this pull request introduces a new type, VsockAddress, and some bootstrap APIs.

Note that this depends on #2477 and #2476, which make NIO more tolerant of channels backed by sockets other than those represented by SocketAddress.

Linux and Darwin man pages differ in both API and behaviour:

  • Support for SOCK_DGRAM: only supported on Linux.
  • Getting the local CID via ioctl(2): Called with /dev/vsock on Linux and with the socket on Darwin.
  • Use of VMADDR_CID_LOCAL: only present on Linux.
  • Use of VMADDR_CID_ANY with connect(2): documented only on Darwin

To accommodate these, this change only covers SOCK_STREAM and provides a consistent API for getting the local context ID for a socket (but discourage its use on Linux in the documentation).

This change also includes some tweaks to NIOEchoClient and NIOEchoServer—when run with two integer parameters, these are considered to be the VSOCK context ID and port number.

Result:

It's now possible to bootstrap clients and servers using VsockAddress. For example, you can do the following to run NIOEchoServer and NIOEchoClient in the same Linux VM over loopback (note VMADDR_CID_LOCAL = 1 on Linux in the following example):

[root@3255e403fa47 code]# $(swift build --show-bin-path)/NIOEchoServer -1 12345
Server started and listening on [VSOCK]-1:12345
[root@3255e403fa47 code]# $(swift build --show-bin-path)/NIOEchoClient 1 12345
Please enter line to send to the server
Hello!
Client connected to unknown
Received: 'Hello!' back from the server, closing channel.
Client closed

It's important to callout that these binaries used to rely on localAddress and remoteAddress being non-nil. When working with sockets that are not handled by enum SocketAddress these will be nil, so it's worth calling out because it seems common that people make the non-nil assumption.

@simonjbeaumont simonjbeaumont requested a review from Lukasa July 21, 2023 11:41
@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Jul 21, 2023
@simonjbeaumont
Copy link
Contributor Author

Hm, looking at the CI failures. Locally the following works:

% docker-compose -f ./docker/docker-compose.yaml -f ./docker/docker-compose.2204.59.yaml run shell
root@aaf1579beac7:/code# rm -rf .build
root@aaf1579beac7:/code# swift run NIOEchoServer -1 12345
...
Server started and listening on [VSOCK]-1:12345

...so does building the tests in the same container:

root@aaf1579beac7:/code# swift build --build-tests
...
[433/433] Linking swift-nioPackageTests.xctest
Build complete! (47.44s)

...but in CI we get:

12:44:26 /code/Sources/NIOPosix/Bootstrap.swift:336:33: error: cannot convert value of type 'VsockAddress' to expected argument type 'SocketAddress'
12:44:26             try socket.bind(to: vsockAddress)
12:44:26                                 ^
12:44:26 error: fatalError

I'll investigate.

@simonjbeaumont
Copy link
Contributor Author

I'm also seeing this in the CI logs...

12:43:23 + uname -a
12:43:23 Linux 43442937c113 3.10.0-1160.15.2.el7.x86_64 #1 SMP Wed Feb 3 15:06:38 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

...which poses a bit of a problem, because we might need a newer Linux Kernel version:

VERSIONS

       Support for VMware (VMCI) has been available since Linux 3.9.
       KVM (virtio) is supported since Linux 4.8.  Hyper-V is supported
       since Linux 4.14.

       VMADDR_CID_LOCAL is supported since Linux 5.6.  Local
       communication in the guest and on the host is available since
       Linux 5.6.  Previous versions supported only local communication
       within a guest (not on the host), and with only some transports
       (VMCI and virtio).

Copy link
Contributor Author

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

I've addressed most of the comments in the latest round of commits—I think all of them that relate to the API, although there's some open discussion around the local context ID.

What remains is for me to try and use the user event to trigger the bind, rather than doing it in the bootstrap itself.

…sent

Signed-off-by: Si Beaumont <beaumont@apple.com>
Signed-off-by: Si Beaumont <beaumont@apple.com>
Signed-off-by: Si Beaumont <beaumont@apple.com>
Signed-off-by: Si Beaumont <beaumont@apple.com>
@simonjbeaumont simonjbeaumont marked this pull request as ready for review July 25, 2023 11:12
@simonjbeaumont simonjbeaumont requested a review from Lukasa July 25, 2023 11:12
@simonjbeaumont
Copy link
Contributor Author

Note the tests will be skipped in CI because the kernel is too old. Here is an example run with the latest commit in a Linux environment:

% swift test --filter Vsock
Build complete! (27.46s)
Test Suite 'Selected tests' started at 2023-07-25 16:05:03.138
Test Suite 'EchoServerClientTest' started at 2023-07-25 16:05:03.154
Test Case 'EchoServerClientTest.testEchoVsock' started at 2023-07-25 16:05:03.154
Test Case 'EchoServerClientTest.testEchoVsock' passed (0.051 seconds)
Test Suite 'EchoServerClientTest' passed at 2023-07-25 16:05:03.206
         Executed 1 test, with 0 failures (0 unexpected) in 0.051 (0.051) seconds
Test Suite 'VsockAddressTest' started at 2023-07-25 16:05:03.206
Test Case 'VsockAddressTest.testDescriptionWorks' started at 2023-07-25 16:05:03.206
Test Case 'VsockAddressTest.testDescriptionWorks' passed (0.0 seconds)
Test Case 'VsockAddressTest.testGetLocalCID' started at 2023-07-25 16:05:03.206
Test Case 'VsockAddressTest.testGetLocalCID' passed (0.001 seconds)
Test Case 'VsockAddressTest.testSocketAddressEquality' started at 2023-07-25 16:05:03.207
Test Case 'VsockAddressTest.testSocketAddressEquality' passed (0.0 seconds)
Test Case 'VsockAddressTest.testSocketAddressEqualitySpecialValues' started at 2023-07-25 16:05:03.207
Test Case 'VsockAddressTest.testSocketAddressEqualitySpecialValues' passed (0.0 seconds)
Test Suite 'VsockAddressTest' passed at 2023-07-25 16:05:03.207
         Executed 4 tests, with 0 failures (0 unexpected) in 0.001 (0.001) seconds
Test Suite 'Selected tests' passed at 2023-07-25 16:05:03.207
         Executed 5 tests, with 0 failures (0 unexpected) in 0.052 (0.052) seconds

Signed-off-by: Si Beaumont <beaumont@apple.com>
@Lukasa Lukasa merged commit 858cb89 into apple:main Aug 3, 2023
1proprogrammerchant added a commit to 1proprogrammerchant/swift-nio that referenced this pull request Aug 4, 2023
Add bootstrap APIs for VSOCK sockets (apple#2479)
finagolfin added a commit to finagolfin/swift-nio that referenced this pull request Aug 5, 2023
Motivation

NIO wasn't building on Android

Modifications

- Add the right checks for Android

Result

Built and passed all tests on Android AArch64
Lukasa pushed a commit that referenced this pull request Aug 5, 2023
Motivation

NIO wasn't building on Android

Modifications

- Add the right checks for Android

Result

Built and passed all tests on Android AArch64
1proprogrammerchant added a commit to 1proprogrammerchant/swift-nio that referenced this pull request Aug 6, 2023
Get Android building again after apple#2479 (apple#2490)
glbrntt pushed a commit to grpc/grpc-swift that referenced this pull request Aug 8, 2023
Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While both GRPC has support for building channels using sockets that were created out of band (`withConnectedSocket(_:)` and `withBoundSocket(_:)`, there are no APIs that facilitate the creation of VSOCK-based channels.

apple/swift-nio#2479 adds a `VsockAddress` type and associated bootstrap APIs, but these need corresponding APIs here to be used in GRPC clients and servers.

Modifications:

- Add `ConnectionTarget.vsockAddress` and `BindTarget.vsockAddress `.
- Add `ClientBootstrapProtocol connect(to vsockAddress:)` and `ServerBootstrapProtocol.bind(to vsockAddress:)`.
- Add `Server.bind(vsockAddress: VsockAddress)`.

Signed-off-by: Si Beaumont <beaumont@apple.com>
@allevato
Copy link

I stumbled upon this PR because we were importing the latest swift-nio changes into our monorepo, where we're building exclusively with explicit modules and strict layering (-fmodules-strict-decluse). This change causes the following error in that mode:

error: module CNIODarwin does not depend on a module exporting 'sys/vsock.h'
#include <sys/vsock.h>
         ^

Unfortunately, it appears that this is because no module in Xcode's SDK includes sys/vsock.h (it's not in Darwin, nor in any other module defined by the module maps there). In a less strict/implicit module build, it would just get textually included.

We can probably reduce the strictness here on our side for this specific module, but do any of the Apple folks have thoughts here? I don't know what the state of explicit modules is for Swift/SPM builds, or whether they're planning to use -fmodules-strict-decluse as well—if so, this inclusion could cause issues there as well.

@shahzadmajeed
Copy link

I'm seeing same build error on my end

@shahzadmajeed
Copy link

@allevato what strictness level (flag to be specific) fixes this issue for you and how/where do you add that?

@Lukasa
Copy link
Contributor

Lukasa commented Aug 15, 2023

Thanks folks. SwiftNIO has never formally promised to build with non-standard build flags, and this is one of those cases. Can I recommend that you use Feedback Assistant to ask for this header to be modularised? In the meantime I recommend unsetting the build flag.

@shahzadmajeed
Copy link

Hi @Lukasa I just looked at my build command log and couldn't find that build flag. Where do you think this might be coming from?

@Lukasa
Copy link
Contributor

Lukasa commented Aug 16, 2023

It is not likely to be the only flag that triggers this. How are you building?

@shahzadmajeed
Copy link

I tried swift build, xcodebuild archive and direct Xcode run action.. all failed with same error.

Basically, i'm building a dynamic framework that links against static frameworks to create a single XCFramework. It builds fine when I use old versions but fails on latest version.

I will see if I can find other linker flags that are sent to xcodebuild invocation.

@Lukasa
Copy link
Contributor

Lukasa commented Aug 17, 2023

Can you provide your build log?

@shahzadmajeed
Copy link

shahzadmajeed commented Aug 17, 2023

Sure.. here is build log

Build DomainKit_2023-08-17T10-27-28.txt

and here is build settings for my DomainKit project (via. xcodebuild -showBuildSettings)

DomainKitBuildSettings.txt

Build Settings of NIO project

SwiftNioBuildSettings.txt

I can add you to a private github example project if you want to try out yourself.

@Lukasa
Copy link
Contributor

Lukasa commented Aug 18, 2023

That's very strange, I can't find a reason for this compile error in those flags. Can you please set up that github example project?

@allevato
Copy link

In @shahzadmajeed's case, I see that Clang is being invoked with the following flags:

-Wnon-modular-include-in-framework-module -Werror=non-modular-include-in-framework-module

which I believe is the default behavior for an Xcode project since the corresponding Xcode setting is named "Allow Non-modular Includes in Framework Modules" and setting it would disable those flags.

@Lukasa
Copy link
Contributor

Lukasa commented Aug 18, 2023

Good catch: I was looking at the Swift compiler invocation, which is where the error is actually produced, not the C setting.

So again, the issue here is Tuist building this module as a framework.

@Lukasa
Copy link
Contributor

Lukasa commented Aug 18, 2023

Our advice then would be to use the corresponding Xcode setting to allow the non-modular include.

@allevato
Copy link

allevato commented Aug 18, 2023

I've filed FB12999916 to cover the underlying problem in the SDK. (If any Apple folks can make sure the appropriate team sees it, that would be appreciated!)

Good catch: I was looking at the Swift compiler invocation, which is where the error is actually produced, not the C setting.

So again, the issue here is Tuist building this module as a framework.

I looked again and I was still confused here, because one of the errors is coming from the swift-frontend invocation, but none of those flags are present. My only guess is that it's related to the header maps being passed into the Swift search paths?

@shahzadmajeed
Copy link

shahzadmajeed commented Aug 18, 2023

I believe I tried building framework with non-modular headers before posting here and I encountered same error but I can double check that.

Also, Tuist is only used to generate project. I'm using xcodebuild to archive and create xcframework

@shahzadmajeed
Copy link

@allevato when you say Swift search paths do you mean Frameworks Search Paths?

WendellXY pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While both GRPC has support for building channels using sockets that were created out of band (`withConnectedSocket(_:)` and `withBoundSocket(_:)`, there are no APIs that facilitate the creation of VSOCK-based channels.

apple/swift-nio#2479 adds a `VsockAddress` type and associated bootstrap APIs, but these need corresponding APIs here to be used in GRPC clients and servers.

Modifications:

- Add `ConnectionTarget.vsockAddress` and `BindTarget.vsockAddress `.
- Add `ClientBootstrapProtocol connect(to vsockAddress:)` and `ServerBootstrapProtocol.bind(to vsockAddress:)`.
- Add `Server.bind(vsockAddress: VsockAddress)`.

Signed-off-by: Si Beaumont <beaumont@apple.com>
pinlin168 pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While both GRPC has support for building channels using sockets that were created out of band (`withConnectedSocket(_:)` and `withBoundSocket(_:)`, there are no APIs that facilitate the creation of VSOCK-based channels.

apple/swift-nio#2479 adds a `VsockAddress` type and associated bootstrap APIs, but these need corresponding APIs here to be used in GRPC clients and servers.

Modifications:

- Add `ConnectionTarget.vsockAddress` and `BindTarget.vsockAddress `.
- Add `ClientBootstrapProtocol connect(to vsockAddress:)` and `ServerBootstrapProtocol.bind(to vsockAddress:)`.
- Add `Server.bind(vsockAddress: VsockAddress)`.

Signed-off-by: Si Beaumont <beaumont@apple.com>
@shahzadmajeed
Copy link

@Lukasa sorry for delay. I added you to my github private repo which is just a sample project to setup static frameworks. You should get an invite. I have added a section at the top on how to reproduce the error

IceRocky pushed a commit to IceRocky/grpc-swift that referenced this pull request May 28, 2024
Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While both GRPC has support for building channels using sockets that were created out of band (`withConnectedSocket(_:)` and `withBoundSocket(_:)`, there are no APIs that facilitate the creation of VSOCK-based channels.

apple/swift-nio#2479 adds a `VsockAddress` type and associated bootstrap APIs, but these need corresponding APIs here to be used in GRPC clients and servers.

Modifications:

- Add `ConnectionTarget.vsockAddress` and `BindTarget.vsockAddress `.
- Add `ClientBootstrapProtocol connect(to vsockAddress:)` and `ServerBootstrapProtocol.bind(to vsockAddress:)`.
- Add `Server.bind(vsockAddress: VsockAddress)`.

Signed-off-by: Si Beaumont <beaumont@apple.com>
teskobif7 added a commit to teskobif7/grpc-swift that referenced this pull request Aug 14, 2024
Motivation:

The VSOCK address family facilitates communication between virtual machines and the host they are running that need a communications channel that is independent of virtual machine network configuration.

While both GRPC has support for building channels using sockets that were created out of band (`withConnectedSocket(_:)` and `withBoundSocket(_:)`, there are no APIs that facilitate the creation of VSOCK-based channels.

apple/swift-nio#2479 adds a `VsockAddress` type and associated bootstrap APIs, but these need corresponding APIs here to be used in GRPC clients and servers.

Modifications:

- Add `ConnectionTarget.vsockAddress` and `BindTarget.vsockAddress `.
- Add `ClientBootstrapProtocol connect(to vsockAddress:)` and `ServerBootstrapProtocol.bind(to vsockAddress:)`.
- Add `Server.bind(vsockAddress: VsockAddress)`.

Signed-off-by: Si Beaumont <beaumont@apple.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants