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

tests_l2: Added qat workload #117

Merged
merged 1 commit into from
Sep 13, 2023
Merged

tests_l2: Added qat workload #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions tests/l2/qat/qatlib_build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright (c) 2023 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change the commit log to:

tests_l2: Add qatlib based Intel QAT sample workload

qatlib is built from source in https://github.com/intel/qatlib
The qatlib version is specified by QATLIB_VERSION: 23.08.0
cpa_sample_code shipped with qatlib is used to verify the QAT provisioning on OCP

Resource:
qat.intel.com/dc is used for QAT compression/decompression
qat.intel.com/cy is used for QAT asymmetric/asymmetric cryptography

serviceAccount: intel-qat uses the user defined SCC intel-qat-scc which enables the IPC_LOCK capability for Qat-lib based workload see #122

Signed-off-by: veenadhari.bedida@intel.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also add the detailed commit log in the git. The PR description can only be accessed from the online GitHub service. Correct me @mythi @vbedida79

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can update commit message. do we plan to do this for every PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please. :-)

# SPDX-License-Identifier: Apache-2.0

apiVersion: image.openshift.io/v1
kind: ImageStream
metadata:
name: intel-qat-workload
namespace: intel-qat
spec: {}
---
apiVersion: build.openshift.io/v1
kind: BuildConfig
metadata:
name: intel-qat-workload
namespace: intel-qat
spec:
triggers:
- type: "ConfigChange"
- type: "ImageChange"
runPolicy: "Serial"
source:
type: Dockerfile
dockerfile: |

FROM registry.access.redhat.com/ubi8/ubi
ARG QATLIB_VERSION

RUN dnf -y update && \
dnf install -y gcc \
make \
automake \
autoconf \
libtool \
http://mirror.centos.org/centos/8-stream/PowerTools/x86_64/os/Packages/nasm-2.15.03-3.el8.x86_64.rpm \
Copy link

Choose a reason for hiding this comment

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

for the purpose of this build, I'd think we should go without nasm. See https://github.com/intel/qatlib/blob/39e19d4fc09345bb857ee8745619fa0f83f6f824/INSTALL#L224-L225

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, thanks. will update

Copy link
Contributor

Choose a reason for hiding this comment

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

for the purpose of this build, I'd think we should go without nasm. See https://github.com/intel/qatlib/blob/39e19d4fc09345bb857ee8745619fa0f83f6f824/INSTALL#L224-L225

Looks like the option is for the situation nasm compiler is not present in the build environment. If we have the nasm compiler, we should install the nasm and build the lib with fast-crc-in-assembler, otherwise, there might be some performance penalty.

Copy link

Choose a reason for hiding this comment

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

For this "test application", it's acceptable but pulling rpms from random urls is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can look for official links of nasm. nasm & qatlib is on codeready-builder-for-rhel-8-x86_64-rpms repo for ubi images. With a dockerfile this repo can be accessed and built on RHEL systems enabled with subscription manager. Via buildconfigs, subscription manager and its supported repos can be enabled only via RH account personal keys. Plan to enable for future releases.

Copy link
Contributor

@uMartinXu uMartinXu Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For this "test application", it's acceptable but pulling rpms from random urls is not.

@mythi I agree with your point. For the qatlib, the user should use RH distributed qatblib rpm package and shoul not build the lib by themselves. However, currently, it is not very easily for user to do that.The enhancement is filed for Milestone 1.1.0. to address the gap.

All in all, for the testing of the first QAT release(1.0.1), this test case is good enough.

@mythi Thank you very much for your comments and help.

openssl-devel \
zlib-devel \
git && \
git clone -b $QATLIB_VERSION https://github.com/intel/qatlib

RUN cd /qatlib && \
./autogen.sh && \
./configure \
--prefix=/usr \
--enable-systemd=no && \
make -j && \
make install samples-install

WORKDIR /usr/bin
ENTRYPOINT ["/usr/bin/cpa_sample_code"]
strategy:
type: Docker
noCache: true
dockerStrategy:
buildArgs:
- name: "QATLIB_VERSION"
value: "23.08.0"

output:
to:
kind: ImageStreamTag
name: intel-qat-workload:latest
29 changes: 29 additions & 0 deletions tests/l2/qat/qatlib_job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright (c) 2023 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

apiVersion: batch/v1
kind: Job
metadata:
name: intel-qat-workload
namespace: intel-qat
spec:
template:
spec:
restartPolicy: Never
containers:
- name: intel-qat-job
image: image-registry.openshift-image-registry.svc:5000/intel-qat/intel-qat-workload:latest
imagePullPolicy: IfNotPresent
command: ["./cpa_sample_code"]
securityContext:
capabilities:
add:
[IPC_LOCK]
resources:
requests:
qat.intel.com/dc: '1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we tried the case when we have more than one qat resource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. /dev/vfio will have 4 dev files instead of 2. since 2 of dc and 2 of cy resources are requested

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean have we ever tried to use multiple resources/VFs to do the dc or/and cy simultaneously in a single pod/container?

Copy link
Contributor Author

@vbedida79 vbedida79 Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

yes difference is what /dev/vfio would have. changed this job example, 2 cy and 2 dc. anything else to be checked here?

Copy link
Contributor

Choose a reason for hiding this comment

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

The provisioning of multiple resources works.
But we might need to figure out whether the multiple resources can actually be used by the test case to do dc or/and cy offloading to QAT device simultaneously in a single pod/container.
For this PR, I think it is good enough to merge.

qat.intel.com/cy: '1'
limits:
qat.intel.com/dc: '1'
qat.intel.com/cy: '1'
serviceAccount: intel-qat