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

Adding POSTCOPY support #218

Merged
merged 7 commits into from
Jan 24, 2024
Merged

Adding POSTCOPY support #218

merged 7 commits into from
Jan 24, 2024

Conversation

ShadowCurse
Copy link
Contributor

@ShadowCurse ShadowCurse commented Dec 15, 2023

Summary of the PR

Added ability to use VHOST_USER_POSTCOPY_ADVISE, VHOST_USER_POSTCOPY_LISTEN and VHOST_USER_POSTCOPY_END messages. This includes sending them from the frontend and processing them on the backend.

The POSTCOPY messages are only usable when
VHOST_USER_PROTOCOL_F_PAGEFAULT feature is negotiated.

Messages and their descriptions are:

  • VHOST_USER_POSTCOPY_ADVISE:
    When the front-end sends this message to the backend,
    the back-end must open a userfaultfd for later use
    and send it's fd to the front-end.

  • VHOST_USER_POSTCOPY_LISTEN
    When the back-end receives this message it must ensure
    that shared memory is registered with userfaultfd to
    cause faulting of non-present pages.
    This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE.

  • VHOST_USER_POSTCOPY_END
    When the back-end receives this message it must disable the
    userfaultfd. The reply is an acknowledgement only.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR are signed (with git commit -s), and the commit
    message has max 60 characters for the summary and max 75 characters for each
    description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

Copy link
Collaborator

@Ablu Ablu left a comment

Choose a reason for hiding this comment

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

I am not sure if this works with operation modes where regions are mapped on demand (Xen grants, for example). But I will let @vireshk comment on that.

Otherwise, I think commit messages are pretty bare-bone. Not everyone (at least not me :)) is an expert on these migration topics so it might help to explain what is happening in more detail.

@Ablu
Copy link
Collaborator

Ablu commented Dec 15, 2023

@germag: This may be related to your work on migration?

@ShadowCurse ShadowCurse force-pushed the postcopy_vhost branch 2 times, most recently from 7f856e6 to 6703c5d Compare December 15, 2023 15:36
ShadowCurse added a commit to ShadowCurse/rust-vmm-container that referenced this pull request Jan 2, 2024
Added musl-tools package.
It is needed for `userfaultfd` crate to compile for musl target.
The PR that need this: rust-vmm/vhost#218

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
ShadowCurse added a commit to ShadowCurse/rust-vmm-container that referenced this pull request Jan 2, 2024
Added musl-tools package.
This is required for rust-vmm/vhost#218
which adds the userfaultfd crates fails to compile without this.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
ShadowCurse added a commit to ShadowCurse/rust-vmm-container that referenced this pull request Jan 2, 2024
Added musl-tools package.
This is required for rust-vmm/vhost#218
which adds the userfaultfd crate which fails to compile without this.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
JonathanWoollett-Light pushed a commit to rust-vmm/rust-vmm-container that referenced this pull request Jan 2, 2024
Added musl-tools package.
This is required for rust-vmm/vhost#218
which adds the userfaultfd crate which fails to compile without this.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
ShadowCurse added a commit to ShadowCurse/rust-vmm-ci that referenced this pull request Jan 3, 2024
This version adds musl-tools package
which is required for rust-vmm/vhost#218
to compile userfaultfd crate for musl target.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
JonathanWoollett-Light pushed a commit to rust-vmm/rust-vmm-ci that referenced this pull request Jan 3, 2024
This version adds musl-tools package
which is required for rust-vmm/vhost#218
to compile userfaultfd crate for musl target.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@ShadowCurse ShadowCurse force-pushed the postcopy_vhost branch 2 times, most recently from 631868b to ae202c0 Compare January 3, 2024 10:31
@ShadowCurse
Copy link
Contributor Author

Hi @Ablu, I have addressed comments you left before, please take another look. Also @vireshk and @germag, would be nice to hear your thoughts as well.

Regarding ci not passing with musl toolchain, I am working on this.

@stefano-garzarella
Copy link
Member

@ShadowCurse did you test the changes of rust-vmm-container locally?
You can use ./docker.sh build to build a local image and try to build this branch through it.

For example, what I usually do is:

cd rust-vmm-container
./docker.sh build

cd vhost
# I'm using podman, but with docker should be similar
podman run -it --rm --security-opt=seccomp=unconfined --volume $(pwd):/workdir --workdir /workdir localhost/rustvmm/dev:v33_x86_64 
cargo build --target x86_64-unknown-linux-musl

@Ablu
Copy link
Collaborator

Ablu commented Jan 5, 2024

/cc @stsquad

@ShadowCurse ShadowCurse force-pushed the postcopy_vhost branch 4 times, most recently from 91a5e5b to 17f0ac4 Compare January 15, 2024 16:04
@ShadowCurse ShadowCurse force-pushed the postcopy_vhost branch 3 times, most recently from 6329054 to e63a374 Compare January 22, 2024 13:51
Copy link
Collaborator

@Ablu Ablu left a comment

Choose a reason for hiding this comment

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

The typo makes me a bit nervous. It probably results in a no-op for all the builds, or do I miss something here?

Could you elaborate how you tested this? Would it maybe make sense to add automated tests?

@stefano-garzarella
Copy link
Member

stefano-garzarella commented Jan 23, 2024

Now that we have tests for the new code, maybe we can enable postcopy for coverage:

diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json
index d201e56..9923e17 100644
--- a/coverage_config_x86_64.json
+++ b/coverage_config_x86_64.json
@@ -1,5 +1,5 @@
 {
   "coverage_score": 75.0,
   "exclude_path": "vhost/src/vhost_kern/",
-  "crate_features": "vhost/vhost-user-frontend,vhost/vhost-user-backend"
+  "crate_features": "vhost/vhost-user-frontend,vhost/vhost-user-backend,vhost-user-backend/postcopy"
 }

I tried it locally and the coverage test passes successfully.

Copy link
Member

@stefano-garzarella stefano-garzarella left a comment

Choose a reason for hiding this comment

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

LGTM!

Nit: there is a little typo in the last commit title (s/coverade/coverage).

New feature `postcopy` is introduced. It acts as a gate for a
new functionality.
This feature is not compatible with `xen` feature because `xen`
can map memory regions on demand and this does not work with uffd.

Now frondned supports sending POSTCOPY messages to the backend.

The POSTCOPY messages are only usable when
VHOST_USER_PROTOCOL_F_PAGEFAULT feature is negotiated.

The messages and their descriptions are:
- VHOST_USER_POSTCOPY_ADVISE:
    When the front-end sends this message to the backend,
    the back-end must open a userfaultfd for later use
    and send it's fd to the front-end.

- VHOST_USER_POSTCOPY_LISTEN
    When the back-end receives this message it must ensure
    that shared memory is registered with userfaultfd to
    cause faulting of non-present pages.
    This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE.

- VHOST_USER_POSTCOPY_END
    When the back-end receives this message it must disable the
    userfaultfd. The reply is an acknowledgement only.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
This functionality is gated by `postcopy` feature.

Now backend supports processing of POSTCOPY messages.
This is only available when `xen` feature is not in use.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
New feature `postcopy` is introduced. It acts as a gate for a
new functionality.
This feature is not compatible with `xen` feature because `xen`
can map memory regions on demand and this does not work with uffd.

Now backend handler supports processing of POSTCOPY messages.
This is only available when `xen` feature is not in use.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Additional tests that trigger postcopy messages handling
on both frontend and backend side. To run successfully
they need access to /dev/userfaultfd.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
New sections in README files describe what `postcopy`
feature enables and the limitation of using it with
`xen` feature.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
@Ablu Ablu merged commit 3044602 into rust-vmm:main Jan 24, 2024
@Ablu
Copy link
Collaborator

Ablu commented Jan 24, 2024

Thanks!

@ShadowCurse ShadowCurse deleted the postcopy_vhost branch January 24, 2024 15:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants