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

VSock: Initial datagram implementation for Argo. #1

Merged
merged 3 commits into from Jun 26, 2019

Conversation

eric-ch
Copy link
Collaborator

@eric-ch eric-ch commented Apr 25, 2019

An out-of-tree module for VSock DGRAM over Argo.

Based on headers released with Xen 4.12.
A copy of Xen argo.h header is added to the out-of-tree module to ease the compilation process against existing kernel headers, since it is not shipped as part of Linux headers.
Also re-introduce hypercall5 in argo-compat.h, removed from upstream as it was no longer used, see comments in the header.

Add a test utility for VSock/Argo DGRAM transfers

Simple test tool to send/recv things over VSock through Argo and ease testing. It tries to mimic netcat usage to not look too unfamiliar. socat should be able to achieve the same, with a steeper learning curve. This will be made easier once STREAM communication are implemented I would think.

Provide instructions in a README

Add some instruction on how to test VSock/Argo on existing Xen 4.12.y installations. Try to address caveats that may be encountered and provide some details as to what is this all about.

Copy link
Collaborator

@dozylynx dozylynx left a comment

Choose a reason for hiding this comment

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

I'm very happy to see this initial driver -- thanks for putting it together (with documentation, very nice!). It looks well structured and like a good starting point.

I have some minor feedback points below - nothing heavy. I have not tested this driver yet but it builds fine.

vsock-argo/README.md Outdated Show resolved Hide resolved
vsock-argo/README.md Outdated Show resolved Hide resolved
vsock-argo/README.md Show resolved Hide resolved
vsock-argo/module/include/xen/argo.h Show resolved Hide resolved
vsock-argo/module/argo_ring.c Outdated Show resolved Hide resolved
vsock-argo/module/argo_ring.c Outdated Show resolved Hide resolved
vsock-argo/module/argo_ring.h Outdated Show resolved Hide resolved
@eric-ch
Copy link
Collaborator Author

eric-ch commented May 20, 2019

v2

  • Fixed reported typo
  • Renamed cargo to hatch (after-all the program is only the thing through which the cargo is sent)
  • Refreshed argo.h header to match latest release. Still the two hunks documented above are left to attend out-of-tree module compilation (see VSock: Initial datagram implementation for Argo. #1 (comment))
  • Renamed incorrectly named argo_virq variable to argo_irq and use ARGO_VIRQ in bind_virq_to_irqhandler as it should have been
  • Set default ring_len to 32 pages to match previous implementations default.

@eric-ch
Copy link
Collaborator Author

eric-ch commented Jun 5, 2019

v3

  • Fixed typo in argo_ring_alloc, sanity checks are to be done against argument len, not the global ring_len.

vsock-argo/module/argo_ring.c Show resolved Hide resolved
vsock-argo/README.md Show resolved Hide resolved
vsock-argo/module/argo-compat.h Outdated Show resolved Hide resolved
vsock-argo/module/argo_ring.c Show resolved Hide resolved
vsock-argo/module/argo_ring.c Show resolved Hide resolved
vsock-argo/module/argo_ring.c Outdated Show resolved Hide resolved
vsock-argo/module/argo_ring.h Outdated Show resolved Hide resolved
vsock-argo/module/argo_transport.c Outdated Show resolved Hide resolved
vsock-argo/module/include/xen/argo.h Show resolved Hide resolved
Based on Xen 4.12.0 released headers.
An Argo implementation for VSock for datagram transfers. Support basic
packet communication across domains.

Rings are registered at socket naming (bind(3p)).
There is one ring created per partner in the domain. This includes ANY,
which Xen will allow only in mac-permissive mode for Argo.

Send is synchronous: does an hypercall directly, like V4V used to do.

Recv is handled in a tasklet that will make skbs with data recovered
from the ring.

Ring size is hardcoded to 32 pages currently.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1473
@eric-ch
Copy link
Collaborator Author

eric-ch commented Jun 25, 2019

V4

  • Removed duplicate function argo_ring_has_data_nowrap.
  • Amended README.md intro with specific mention of tested platforms (x86-64).
  • Moved argo-compat.h x86 specific header to asm-x86 sub-directory and amend KBuild to include this architecture specific header only for X86 targets.
  • Initialized argo_irq correctly and check successful VIRQ binding happened before attempting to unbind.
  • Add debug print if the received skb data are not accessible.

Copy link
Collaborator

@dozylynx dozylynx left a comment

Choose a reason for hiding this comment

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

Thanks for the changes for the last round. Moving on to building it now: the build succeeds but it's ignoring the warnings indicated in this review -- mismatching types for the format strings.

Can the build be made to use -Werror ?

vsock-argo/test/hatch/src/hatch.c Outdated Show resolved Hide resolved
vsock-argo/test/hatch/src/hatch.c Outdated Show resolved Hide resolved
Eric Chanudet added 2 commits June 26, 2019 11:42
Very simple tool to send/recv things through VSock.
Basic usage:
    hatch domid port
    hatch -r -p local_port
Options:
    -r, --recv      receive mode, wait for incoming packets.
    -p, --port      set local port number.
    -D              enable debug output on stderr.

Debug is logged on stderr to allow stdout redirection.

If vsock_argo_transport.ko was not loaded, vmw_vsock_vmci_transport.ko
will likely be loaded by default (depends on the installation). You need
to remove vmw_vsock_vmci_transport.ko before you can load
vsock_argo_transport.ko again.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>

OXT-1473
Walk through how to test VSock/Argo.

Signed-off-by: Eric Chanudet <chanudete@ainfosec.com>
@eric-ch
Copy link
Collaborator Author

eric-ch commented Jun 26, 2019

v5

  • Amend embarrassing print format warnings.
  • Use AM_FLAGS to pass -W -Wall -Werror by default to the C compiler.

@dozylynx
Copy link
Collaborator

Builds clean now. Merging.

@dozylynx dozylynx merged commit ada85e3 into OpenXT:master Jun 26, 2019
Tmcg2 pushed a commit to Tmcg2/linux-xen-argo that referenced this pull request Jun 4, 2020
…ux-xen-argo:openxt_master_merge to master

* commit '11866bceac9974f6f688e84b8a6116e8d9c65675':
  argo-module: constify iov pointer
  argo-module: unused variable in kernel <4.19
  argo-module: Linux 5.0 access_ok() stub
  argo-linux: Implicit fall through
  argo-linux: use init_pseudo for vfs
  argo-linux: suppress SMAP warnings
  vsock: suppress SMAP warnings
  gitignore: Remove top-level
  libargo: Amend gitignore
  vsock-argo: Add built-in objects to gitignore
  argo-linux: Add built-in objects to gitignore
  argo-modules: Put some printks behind ARGO_DEBUG
  Try to auto-bind to the partner's domid
  README: Initial documentation.
  vsock/test: Netcat-like utility for Argo DGRAM.
  vsock/module: Initial datagram implementation.
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.

None yet

2 participants