QAT: update readme for QAT #110

mregmi · 2023-08-17T17:06:27Z

No description provided.

device_plugins/deploy_qat.md

mythi

A few suggestions. I also noticed https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/device_plugins/qat_device_plugin.yaml is not quite OK. maxNumDevices should be 128 and perhaps c6xxxvf can be dropped.

mythi · 2023-08-18T05:16:21Z

device_plugins/deploy_qat.md

+
+## Prerequisite
+Before deploying the QAT plugin, make sure iommu MCO is deployed and iommu is enabled.
+By default, half of the virtual functions are used for compression and the other half is used for cryptography. This default behaviour can be customised via a config map which configures the hardware using sysfs. The config values could be `asym`, `sym` and `dc`, for more details see https://github.com/torvalds/linux/blob/master/Documentation/ABI/testing/sysfs-driver-qat


pointing to the latest sysfs-driver-qat is not accurate because the Linux 6.5 features are not available. Only sym;asym and dc should be valid on OCP today.

mythi · 2023-08-18T05:21:19Z

device_plugins/deploy_qat.md

+```
+oc create configmap --namespace=openshift-operators --from-literal "qat.conf=ServicesEnabled=<option>" qat-config
+```
+The option can be any combination of values described above


see my previous comment. this is also missing how to setup "per node" settings.

mythi · 2023-08-18T05:24:54Z

device_plugins/deploy_qat.md

+```
+
+## verify QAT resource is available
+QAT has two kinds of resource `qat.intel.com/cy` for cryptography and `qat.intel.com/dc` for compression. If the plugin deployment is a success, the resources can be seen using the commands below. Use /dc is compression is configured.


Can this section be written in a way that cyand dc are not explicitly mentioned. We might have to change these in the future when the 6.5 features become available. Maybe just say "look for qat.intel.com resources for compression and crypto"

mythi · 2023-08-18T05:26:36Z

machine_configuration/README.md

+
+```$ oc apply -f https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/machine_configuration/100-intel-qat-intel-iommu-on.yaml```
+
+Note: This will reboot the worker nodes in the Intel QAT machine config pool one by one to turn on intel_iommu kernel parameter. Rebooting the node is not preferred A better solution is in the plan.


Suggested change

Note: This will reboot the worker nodes in the Intel QAT machine config pool one by one to turn on intel_iommu kernel parameter. Rebooting the node is not preferred A better solution is in the plan.

Note: This will reboot the worker nodes in the Intel QAT machine config pool one by one to turn on intel_iommu kernel parameter.

added the suggested change

mythi · 2023-08-18T05:27:23Z

machine_configuration/README.md

+
+For QAT, intel_iommu is a kernel parameter that needs to be turned on using the following command.
+
+```$ oc apply -f https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/machine_configuration/100-intel-qat-intel-iommu-on.yaml```


this could also add loading vfio-pci with the desired ids as parameters since we have to do the booting anyway.

is it possible for us to add the ids as the parameter for the container and set it through the operator? So we don't have to hard code the ids in in the init-container? :-)

hershpa · 2023-08-18T16:05:02Z

A few suggestions. I also noticed https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/device_plugins/qat_device_plugin.yaml is not quite OK. maxNumDevices should be 128 and perhaps c6xxxvf can be dropped.

Hi Mikko, thanks for your review and suggestions. I presume Chaitanya used the sample here for qat device plugin reference.

Are there any implications of setting maxNumDevices to 128 i.e. some device ids do not support that level etc?

mythi · 2023-08-21T06:27:00Z

Are there any implications of setting maxNumDevices to 128 i.e. some device ids do not support that level etc?

maxNumDevices limits how many QAT VF resources are made available from each node to the cluster. Depending on the platform and HW setup, different amount of VFs are available. We don't have a setting that says "use all available" so some big enough value is used to reflect that.

hershpa · 2023-08-21T15:59:04Z

Are there any implications of setting maxNumDevices to 128 i.e. some device ids do not support that level etc?

maxNumDevices limits how many QAT VF resources are made available from each node to the cluster. Depending on the platform and HW setup, different amount of VFs are available. We don't have a setting that says "use all available" so some big enough value is used to reflect that.

Got it, thanks @mythi!

mregmi · 2023-08-22T16:40:33Z

Are there any implications of setting maxNumDevices to 128 i.e. some device ids do not support that level etc?

maxNumDevices limits how many QAT VF resources are made available from each node to the cluster. Depending on the platform and HW setup, different amount of VFs are available. We don't have a setting that says "use all available" so some big enough value is used to reflect that.

Addressed this on a separate PR: #111

uMartinXu · 2023-08-22T17:54:32Z

Are there any implications of setting maxNumDevices to 128 i.e. some device ids do not support that level etc?

maxNumDevices limits how many QAT VF resources are made available from each node to the cluster. Depending on the platform and HW setup, different amount of VFs are available. We don't have a setting that says "use all available" so some big enough value is used to reflect that.

@mythi I think we should have some kind of default value for this to use all the available number of VFs. The end user should not have to know and handle this HW detail.
We also need to do the boundary test to make sure the big enough value is well handled by the device plugin.

uMartinXu · 2023-08-22T18:57:30Z

device_plugins/deploy_qat.md

+For more details see https://intel.github.io/quickassist
+
+## Prerequisite
+Before deploying the QAT plugin, make sure iommu MCO is deployed and iommu is enabled.


We need to work on the PRs in MCO to enable this. and also updated the related readme. And then we can point users to the readme.

Are you referring to enabling IOMMU without a reboot? I've not seen that being possible but it's worth to check.

uMartinXu · 2023-08-22T19:04:36Z

device_plugins/deploy_qat.md

+
+## Prerequisite
+Before deploying the QAT plugin, make sure iommu MCO is deployed and iommu is enabled.
+By default, half of the virtual functions are used for compression and the other half is used for cryptography. This default behaviour can be customised via a config map which configures the hardware using sysfs. The config values could be `asym`, `sym` and `dc`, for more details see [cfg_services](https://github.com/torvalds/linux/blob/42e66b1cc3a070671001f8a1e933a80818a192bf/Documentation/ABI/testing/sysfs-driver-qat)


This should not be the Prerequisite. I think it should be the feature Gap, I suggest to have an individual section in this document to introduce customers to the detail. And the link to this issue should be added in the section. And above link is good for the stand-alone RHEL server. But for OCP, the ender user should not directly operate the individual node.

This should not be the Prerequisite. I think it should be the feature Gap,

Which part you are referring to?

uMartinXu · 2023-08-22T19:06:50Z

device_plugins/deploy_qat.md

+The option can be any combination of values described above.
+When using the operator for deploying the plugin with provisioning config, use provisioningConfig field for the name of the ConfigMap, then the config is passed to initcontainer through the volume mount.
+
+There's also a possibility for a node specific congfiguration through passing a nodename via NODE_NAME into initcontainer's environment and passing a node specific profile (qat-$NODE_NAME.conf) via ConfigMap volume mount.


I am thinking about whether we should expose the configmap detail to the OCP customers.

It's totally fine to have it. The default setting without the configmap is also fine but if the cluster admin wants to opt-in for a custom per-node configuration, they can do so with the configmap and by setting the name of the confimap in QatDevicePlugin custom resource.

I'd like to put it into some kind of Advanced Configuration Section in the readme.

uMartinXu · 2023-08-22T19:08:52Z

machine_configuration/README.md

@@ -87,6 +87,23 @@ $ lsmod | grep ast
 ```
 Ensure that ast driver is not loaded.

+# Machine Configuration for Provisioning Intel QAT
+


Let's move this to MCO. And I think we should remove the QAT machine config pool for simplicity purposes.

uMartinXu · 2023-08-22T19:12:07Z

machine_configuration/README.md

+
+For QAT, intel_iommu is a kernel parameter that needs to be turned on using the following command.
+
+```$ oc apply -f https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/machine_configuration/100-intel-qat-intel-iommu-on.yaml```


is it possible for us to add the ids as the parameter for the container and set it through the operator? So we don't have to hard code the ids in in the init-container? :-)

mythi · 2023-08-23T04:42:23Z

is it possible for us to add the ids as the parameter for the container and set it through the operator? So we don't have to hard code the ids in in the init-container?

The PCI IDS of the devices enabled by the cluster admin are passed to the initcontainer so you can have custom logic based on that.

mythi · 2023-08-23T04:49:37Z

@mythi I think we should have some kind of default value for this to use all the available number of VFs. The end user should not have to know and handle this HW detail.

This flag is for purpose: QAT can also be used for "in kernel" functions through the Linux Kernel Crypto Framework. with maxNumDevices, cluster admins have the capability to leave some VFs for the "in kernel" functions (and other functions outside the cluster). Therefore, it cannot be "use everything" unconditionally.

We also need to do the boundary test to make sure the big enough value is well handled by the device plugin.

maxNumDevices is ignored after the plugin has processed all the QAT VFs it finds.

uMartinXu · 2023-08-23T17:57:31Z

@mythi I think we should have some kind of default value for this to use all the available number of VFs. The end user should not have to know and handle this HW detail.

This flag is for purpose: QAT can also be used for "in kernel" functions through the Linux Kernel Crypto Framework. with maxNumDevices, cluster admins have the capability to leave some VFs for the "in kernel" functions (and other functions outside the cluster). Therefore, it cannot be "use everything" unconditionally.

We also need to do the boundary test to make sure the big enough value is well handled by the device plugin.

maxNumDevices is ignored after the plugin has processed all the QAT VFs it finds.
I am OK with manNumDevices parameter, my point is we should have a default value for users if they do not set it. The default value of maxNumber can be the max VF numbers the device can support. Besides, if we export this parameter and let user set this up, we should also let users know how many VF devices this device can support, something like https://doc.dpdk.org/guides/cryptodevs/qat.html Table 21.2 QAT device generations, devices and drivers.

uMartinXu · 2023-08-23T18:01:10Z

@mythi I think we should have some kind of default value for this to use all the available number of VFs. The end user should not have to know and handle this HW detail.

This flag is for purpose: QAT can also be used for "in kernel" functions through the Linux Kernel Crypto Framework. with maxNumDevices, cluster admins have the capability to leave some VFs BTW, this implies that we will also continue to support he "Kernel Mode"? :-)

mythi · 2023-08-23T18:04:25Z

BTW, this implies that we will also continue to support he "Kernel Mode"? :-)

no, that is different case.

mythi · 2023-08-23T18:08:03Z

The default value of maxNumber can be the max VF numbers the device can support.

it's not always that simple. There's VFs/PF but there's also how many PFs are available. I understand what you are saying but we'll go with this for now.

uMartinXu · 2023-08-23T23:17:01Z

The default value of maxNumber can be the max VF numbers the device can support.

it's not always that simple. There's VFs/PF but there's also how many PFs are available. I understand what you are saying but we'll go with this for now.

I think for this release it is good enough. Just hope we can optimize it in the future. We can file another issue and track it. :-)

mythi · 2023-08-24T06:12:03Z

The default value of maxNumber can be the max VF numbers the device can support.

it's not always that simple. There's VFs/PF but there's also how many PFs are available. I understand what you are saying but we'll go with this for now.

I think for this release it is good enough. Just hope we can optimize it in the future. We can file another issue and track it. :-)

Yes, future is fine when we also get to hear end-user feedback.

hershpa · 2023-08-30T21:57:42Z

@mregmi, the machine config readme documentation needs to be updated since we are now not using QAT custom machine config pool. Also, please add a section about the new machine config to load vfio-pci. Thanks!

hershpa · 2023-08-30T21:57:57Z

@chaitanya1731, can you review the device plugins readme. Thanks!

Signed-off-by: Manish Regmi <[email protected]>

uMartinXu · 2023-09-13T23:45:00Z

some other PRS will be submitted to replace this PR. So close this one.

hershpa added documentation Improvements or additions to documentation qat QAT feature labels Aug 17, 2023

vbedida79 reviewed Aug 17, 2023

View reviewed changes

device_plugins/deploy_qat.md Outdated Show resolved Hide resolved

mregmi force-pushed the main branch from cc1f13e to 6665333 Compare August 17, 2023 19:53

mythi reviewed Aug 18, 2023

View reviewed changes

mregmi force-pushed the main branch from 6665333 to 25b6aad Compare August 22, 2023 16:41

hershpa added this to the v1.0.1 milestone Aug 22, 2023

uMartinXu reviewed Aug 22, 2023

View reviewed changes

hershpa added the machine_config Machine Configuration label Aug 30, 2023

QAT: update readme for QAT

716a359

Signed-off-by: Manish Regmi <[email protected]>

mregmi force-pushed the main branch 2 times, most recently from b1b2a57 to 716a359 Compare September 11, 2023 18:04

uMartinXu closed this Sep 13, 2023


		```$ oc apply -f https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/machine_configuration/100-intel-qat-intel-iommu-on.yaml```

		Note: This will reboot the worker nodes in the Intel QAT machine config pool one by one to turn on intel_iommu kernel parameter. Rebooting the node is not preferred A better solution is in the plan.


		For QAT, intel_iommu is a kernel parameter that needs to be turned on using the following command.

		```$ oc apply -f https://github.com/intel/intel-technology-enabling-for-openshift/blob/main/machine_configuration/100-intel-qat-intel-iommu-on.yaml```

QAT: update readme for QAT #110

QAT: update readme for QAT #110

Conversation

mregmi commented Aug 17, 2023

mythi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hershpa commented Aug 18, 2023 • edited Loading

mythi commented Aug 21, 2023

hershpa commented Aug 21, 2023

mregmi commented Aug 22, 2023 • edited Loading

uMartinXu commented Aug 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mythi commented Aug 23, 2023 • edited Loading

mythi commented Aug 23, 2023

uMartinXu commented Aug 23, 2023

uMartinXu commented Aug 23, 2023

mythi commented Aug 23, 2023

mythi commented Aug 23, 2023

uMartinXu commented Aug 23, 2023

mythi commented Aug 24, 2023

hershpa commented Aug 30, 2023

hershpa commented Aug 30, 2023

uMartinXu commented Sep 13, 2023

hershpa commented Aug 18, 2023 •

edited

Loading

mregmi commented Aug 22, 2023 •

edited

Loading

mythi commented Aug 23, 2023 •

edited

Loading