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.

Sign up for GitHub

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

Jump to bottom

[GPU] Enable encryption of cache blob with CacheMode::OPTIMIZE_SIZE #27912

Merged
merged 5 commits into from
Dec 13, 2024
Merged

[GPU] Enable encryption of cache blob with CacheMode::OPTIMIZE_SIZE #27912

merged 5 commits into from
Dec 13, 2024

Conversation

tkrupa-intel
Copy link
Contributor

@tkrupa-intel tkrupa-intel commented Dec 4, 2024 •
edited by ilya-lavrenov
Loading

Details:

  • Enables encryption of cache blob with CacheMode::OPTIMIZE_SIZE in GPU Plugin.
  • Some additional test coverage already present in src/plugins/intel_gpu/tests/functional/shared_tests_instances/behavior/ov_plugin/caching_tests.cpp. Test coverage in this PR is distinct from those tests because it also checks correctness of the results.
  • [GPU] Enable weightless cache with precision conversion #27742 has to be merged first - it guarantees small cache sizes in majority of cases which is important for encryption.

Tickets:

@tkrupa-intel tkrupa-intel requested review from a team as code owners December 4, 2024 14:48
@github-actions github-actions bot added category: inference OpenVINO Runtime library - Inference category: GPU OpenVINO GPU plugin category: CPP API OpenVINO CPP API bindings labels Dec 4, 2024
@e-ddykim
Copy link
Contributor

e-ddykim commented Dec 5, 2024

Instead of encrypting or decrypting the entire blob all at once, can't we do it in multiple batches?

@tkrupa-intel
Copy link
Contributor Author

@e-ddykim Before implementing I talked about this with @ilya-lavrenov and I decided to fulfill an expectation to give encryption algorithm developers a full freedom to analyze/encrypt the whole data. This is only enabled with CacheMode::OPTIMIZE_SIZE so with blobs sized a couple of MB (after the dependency PR is merged).

A second consideration - are we certain that breaking it up into arbitrarily sized blocks wouldn't compromise security of any encryption algorithm a user may provide?

@e-ddykim
Copy link
Contributor

e-ddykim commented Dec 5, 2024 •
edited
Loading

A second consideration - are we certain that breaking it up into arbitrarily sized blocks wouldn't compromise security of any encryption algorithm a user may provide?

I don't know if it affects security.

olpipi
olpipi approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@olpipi olpipi left a comment

Choose a reason for hiding this comment

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

LGTM

just question: Is this feature expected to work on all OSs and platforms?

@tkrupa-intel
Copy link
Contributor Author

@olpipi Yes, that's the intention here.

@tkrupa-intel tkrupa-intel requested a review from a team as a code owner December 9, 2024 14:38
@tkrupa-intel tkrupa-intel requested review from akopytko and removed request for a team December 9, 2024 14:38
@github-actions github-actions bot added the category: docs OpenVINO documentation label Dec 9, 2024
@tkrupa-intel tkrupa-intel force-pushed the private/tkrupa/cache_encryption branch from 6999412 to ea19102 Compare December 10, 2024 08:44
@github-actions github-actions bot removed the category: docs OpenVINO documentation label Dec 10, 2024
@tkrupa-intel tkrupa-intel mentioned this pull request Dec 10, 2024
Closed
@tkrupa-intel
Copy link
Contributor Author

build_jenkins

@ilya-lavrenov ilya-lavrenov added this to the 2025.0 milestone Dec 12, 2024
@p-durandin p-durandin enabled auto-merge December 12, 2024 10:18
@tkrupa-intel tkrupa-intel mentioned this pull request Dec 12, 2024
Merged
@tkrupa-intel
Copy link
Contributor Author

build_jenkins

@p-durandin p-durandin added this pull request to the merge queue Dec 12, 2024
@p-durandin p-durandin removed this pull request from the merge queue due to a manual request Dec 12, 2024
@p-durandin p-durandin added this pull request to the merge queue Dec 13, 2024
@p-durandin p-durandin removed this pull request from the merge queue due to a manual request Dec 13, 2024
@p-durandin p-durandin enabled auto-merge December 13, 2024 08:50
@tkrupa-intel tkrupa-intel force-pushed the private/tkrupa/cache_encryption branch from aa15586 to fa24a8e Compare December 13, 2024 09:14
@tkrupa-intel
Copy link
Contributor Author

build_jenkins

@tkrupa-intel
Copy link
Contributor Author

build_jenkins

@p-durandin p-durandin added this pull request to the merge queue Dec 13, 2024
Merged via the queue into openvinotoolkit:master with commit e3606a5 Dec 13, 2024
183 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Dec 16, 2024
@tkrupa-intel @sgolebiewski-intel @p-durandin
Private/tkrupa/cache encryption docs (#28035)
63c3047 
### Details:
- Replacement for #27993
(can't modify it, not from fork).
- Docs for #27912. Added
separately because docs addition breaks CI (no GPU Plugin in Python API
tests currently),

### Tickets:
- CVS-158140

---------

Co-authored-by: Sebastian Golebiewski <[email protected]>
Co-authored-by: Pavel Durandin <[email protected]>
11happy pushed a commit to 11happy/openvino that referenced this pull request Dec 23, 2024
@tkrupa-intel @sshlyapn @11happy
[GPU] Enable encryption of cache blob with CacheMode::OPTIMIZE_SIZE (o…
94cf804 
…penvinotoolkit#27912)

### Details:
- Enables encryption of cache blob with CacheMode::OPTIMIZE_SIZE in GPU
Plugin.
- Some additional test coverage already present in
src/plugins/intel_gpu/tests/functional/shared_tests_instances/behavior/ov_plugin/caching_tests.cpp.
Test coverage in this PR is distinct from those tests because it also
checks correctness of the results.
- openvinotoolkit#27742 has to be merged first - it guarantees small cache sizes in
majority of cases which is important for encryption.

### Tickets:
 - CVS-158140

---------

Co-authored-by: Sergey Shlyapnikov <[email protected]>
11happy pushed a commit to 11happy/openvino that referenced this pull request Dec 23, 2024
@tkrupa-intel @sgolebiewski-intel
@p-durandin @11happy
Private/tkrupa/cache encryption docs (openvinotoolkit#28035)
aeab496 
### Details:
- Replacement for openvinotoolkit#27993
(can't modify it, not from fork).
- Docs for openvinotoolkit#27912. Added
separately because docs addition breaks CI (no GPU Plugin in Python API
tests currently),

### Tickets:
- CVS-158140

---------

Co-authored-by: Sebastian Golebiewski <[email protected]>
Co-authored-by: Pavel Durandin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e-ddykim e-ddykim e-ddykim approved these changes

@p-durandin p-durandin p-durandin approved these changes

@olpipi olpipi olpipi approved these changes

@sshlyapn sshlyapn sshlyapn approved these changes

@ilya-lavrenov ilya-lavrenov ilya-lavrenov approved these changes

@akopytko akopytko Awaiting requested review from akopytko akopytko was automatically assigned from openvinotoolkit/openvino-docs-maintainers

Assignees
No one assigned
Labels
category: CPP API OpenVINO CPP API bindings category: GPU OpenVINO GPU plugin category: inference OpenVINO Runtime library - Inference
Projects
None yet
Milestone
2025.0
Development

Successfully merging this pull request may close these issues.
6 participants
@tkrupa-intel @e-ddykim @ilya-lavrenov @sshlyapn @olpipi @p-durandin