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

qat: improve qat_dpdk_app, openssl-qat-engine #1685

Merged
merged 4 commits into from May 15, 2024
Merged

qat: improve qat_dpdk_app, openssl-qat-engine #1685

merged 4 commits into from May 15, 2024

Conversation

hj-johannes-lee
Copy link
Contributor

@hj-johannes-lee hj-johannes-lee commented Mar 8, 2024 •
edited

  1. drop generic from qat_dpdk_app
  2. add compress-perf to e2e
  3. make openssl-qat-engine use ubuntu 24.04 and drop all custom builds / cherrypicked in pr Fix repo for opencl-icd, workaround build issues and cherry-pick openssl-qat-engine changes #1728
  4. drop e2e tests for qat generic

mythi
mythi reviewed Mar 11, 2024
View reviewed changes
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

Looks OK but need to get e2e-qat4 green

test/e2e/qat/qatplugin_dpdk.go Show resolved Hide resolved
@hj-johannes-lee hj-johannes-lee marked this pull request as draft April 11, 2024 17:55
@hj-johannes-lee
Copy link
Contributor Author

sorry for not changing to draft before you already made comments.
Thanks for the comments though. I was just commiting the changes to this commit not to lose the work.

@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-001 branch 3 times, most recently from a4fb312 to b3ee51d Compare April 11, 2024 20:25
@hj-johannes-lee hj-johannes-lee changed the title e2e: add compress-perf qat: improve qat_dpdk_app and e2e Apr 11, 2024
@hj-johannes-lee
Copy link
Contributor Author

hj-johannes-lee commented Apr 11, 2024 •
edited

I guess because of the last commit of 3 commits cd/validate does not pass.
Please give your opinion about the change of the name of the app crypto-perf that does not make sense to me.

@hj-johannes-lee hj-johannes-lee self-assigned this Apr 11, 2024
@hj-johannes-lee hj-johannes-lee marked this pull request as ready for review April 11, 2024 20:46
@hj-johannes-lee hj-johannes-lee changed the title qat: improve qat_dpdk_app and e2e qat: improve qat_dpdk_app, openssl-qat-engine Apr 11, 2024
@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-001 branch 4 times, most recently from a83db7d to 8f6cf65 Compare April 12, 2024 00:26
@mythi
Copy link
Contributor

mythi commented Apr 12, 2024

Please give your opinion about the change of the name of the app crypto-perf that does not make sense to me.

You're right that the name is a bit misleading but we cannot rename it for now (getting a better name does not justify the amount of work/changes needed outside of this repository).

@hj-johannes-lee
Copy link
Contributor Author

Please give your opinion about the change of the name of the app crypto-perf that does not make sense to me.

You're right that the name is a bit misleading but we cannot rename it for now (getting a better name does not justify the amount of work/changes needed outside of this repository).

I understood, and I somewhat guessed it would not be possible to change. Let me drop the last commit then.

mythi
mythi reviewed Apr 16, 2024
View reviewed changes
cmd/qat_plugin/README.md Outdated Show resolved Hide resolved
mythi
mythi reviewed Apr 16, 2024
View reviewed changes
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

@hj-johannes-lee looking good to me, I added some minor comments. I'd prefer to wait with this until #1699 is available so that the DPDK tests can be tested and enabled too

@mythi
Copy link
Contributor

mythi commented Apr 18, 2024

Fixes: #1687

This isn't fixing all of it. I'm noticing I did not have the issue tasks defined very clearly. I think it would make more sense to rework the commits so that you get to run the tests on e2e-qat until #1699 is available. That is, make the kustomization changes but keep .yamls using generic. What do you think?

@hj-johannes-lee
Copy link
Contributor Author

I am now confused.
Umm, how do you want to keep generic?
As an overlay of each case?

@mythi
Copy link
Contributor

mythi commented Apr 18, 2024

Umm, how do you want to keep generic?

I proposed to make it so that it's possible to keep using e2e-qat (and DPDK tests running) for now but with all the cleanup done. This PR drops generic but it also stops running the tests

@mythi mythi mentioned this pull request Apr 24, 2024
Merged
@mythi
Copy link
Contributor

mythi commented Apr 30, 2024

I'd still suggest we get the big chunk of changes tested + merged on the old setup. There's a lot of unrelated things blocked by e2e-spr gaps (too old kernel for the compress-perf fix)

@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-001 branch 2 times, most recently from 0491bf4 to dd2a488 Compare May 7, 2024 17:44
@hj-johannes-lee
qat_dpdk_app: drop generic
2af37fd 
Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
@hj-johannes-lee hj-johannes-lee marked this pull request as draft May 7, 2024 18:41
@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-001 branch 4 times, most recently from ae6da68 to 70c0bc8 Compare May 13, 2024 12:36
@hj-johannes-lee hj-johannes-lee marked this pull request as ready for review May 13, 2024 14:12
Hyeongju Johannes Lee and others added 3 commits May 13, 2024 17:57
@hj-johannes-lee
e2e: add compress-perf
696877a 
still maintain ci/cd skip test for compress-perf

Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
@hj-johannes-lee
e2e, ci: drop qat generic tests
6ebf093 
Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
@hj-johannes-lee
openssl-qat-engine: add empty line at the end
81b8695 
Signed-off-by: Hyeongju Johannes Lee <hyeongju.lee@intel.com>
tkatila
tkatila approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@tkatila tkatila left a comment

Choose a reason for hiding this comment

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

We should update the SPR kernel soon-ish so we can enable compress-perf on it.

@tkatila tkatila merged commit e753423 into intel:main May 15, 2024
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mythi mythi mythi approved these changes

@tkatila tkatila tkatila approved these changes

@bart0sh bart0sh Awaiting requested review from bart0sh bart0sh is a code owner

@ozhuraki ozhuraki Awaiting requested review from ozhuraki ozhuraki is a code owner

@kad kad Awaiting requested review from kad kad is a code owner

Assignees

@hj-johannes-lee hj-johannes-lee

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hj-johannes-lee @mythi @tkatila