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

socket: add support for AF_VSOCK #1091

Merged
merged 2 commits into from
Jul 12, 2019
Merged

Conversation

stefano-garzarella
Copy link
Contributor

This patch adds the support of AF_VSOCK in the socket module.
VSOCK is present since Linux 3.9.

@stefano-garzarella stefano-garzarella force-pushed the vsock-master branch 2 times, most recently from 3a23e41 to ed495c0 Compare June 26, 2019 09:22
@stefano-garzarella
Copy link
Contributor Author

I added an entry in the CHANGELOG, more description in the commit message and forced push.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

The patch looks good so far. But could you please add some tests for the functionality?

@stefano-garzarella
Copy link
Contributor Author

The patch looks good so far. But could you please add some tests for the functionality?

Sure, I'll add some tests for VSOCK.

@stefano-garzarella
Copy link
Contributor Author

@asomers I add a test. Unfortunately, the current VSOCK implementation in Linux doesn't support loopback devices, so, for now, I add a simple test that tries to bind forbidden address and tries to connect, receiving an error.

For the 'AddressFamily' I think it is enough, what do you think?

@asomers
Copy link
Member

asomers commented Jul 1, 2019

All the test needs to do is ensure that Nix is binding correctly; we don't need a full regression test for vsock. So if bind returns a distinctive error code then that's enough.

@stefano-garzarella
Copy link
Contributor Author

All the test needs to do is ensure that Nix is binding correctly; we don't need a full regression test for vsock. So if bind returns a distinctive error code then that's enough.

@asomers Okay, so what I tried in the second patch that I pushed on this PR is exactly what you said, because a bind on reserved address should return EADDRNOTAVAIL.

#[derive(Copy, Clone)]
pub struct VsockAddr(pub sockaddr_vm);

// , PartialEq, Eq, Debug, Hash
Copy link
Member

Choose a reason for hiding this comment

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

What exactly does this comment mean, and why the leading comma?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry about that! I think was a line copied by struct AlgAddr that I used as a template for VsockAddr. I'll remove this line.

@stefano-garzarella
Copy link
Contributor Author

@asomers I removed the useless comment line that you pointed out and added few comments on VsockAddr to explain cid and port fields.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Ok, I think this PR is finally ready!

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 11, 2019

Merge conflict

@asomers asomers added this to the 0.15.0 milestone Jul 11, 2019
@asomers
Copy link
Member

asomers commented Jul 11, 2019

Please rebase to fix the merge conflict.

This patch adds AF_VSOCK support to AddressFamily in order to use
VSOCK socket.

Signed-off-by: Stefano Garzarella <[email protected]>
The current VSOCK implementation does not support loopback devices,
so, for now, we expect a failure in the spawned thread when it
tries to connect.

Signed-off-by: Stefano Garzarella <[email protected]>
@stefano-garzarella
Copy link
Contributor Author

Please rebase to fix the merge conflict.

@asomers done!

Thanks,
Stefano

@asomers
Copy link
Member

asomers commented Jul 12, 2019

bors r+

bors bot added a commit that referenced this pull request Jul 12, 2019
1091: socket: add support for AF_VSOCK r=asomers a=stefano-garzarella

This patch adds the support of AF_VSOCK in the socket module.
VSOCK is present since Linux 3.9.



Co-authored-by: Stefano Garzarella <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 12, 2019

Build succeeded

@bors bors bot merged commit db72b7e into nix-rust:master Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants