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.

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

control/server.py: Enable DSA to offload and accelerate CRC calculation #704

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .env
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ SPDK_URL="https://spdk.io"

SPDK_PKGDEP_ARGS="--rbd"
# check spdk/configure --help
SPDK_CONFIGURE_ARGS="--with-rbd --disable-tests --disable-unit-tests --disable-examples --enable-debug"
SPDK_CONFIGURE_ARGS="--with-rbd --with-idxd --disable-tests --disable-unit-tests --disable-examples --enable-debug"
Copy link
Member

Choose a reason for hiding this comment

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

This won't work on older CPU, please detect it and only is supported enable it

Copy link
Contributor

Choose a reason for hiding this comment

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

As we build right now, it might not work on any CPU. To eliminate crashes on some machine we build the SPDK using x86-64-v2 architecture. This instructs the compiler not to use instructions which are not available on v2. We need to verify that the instructions used here are supported on v2 CPUs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@CongMinYin @gbregman the question is what happens if one one hand we build to x86-64-v2, but on the other hand we specify this flag (--with-idxd). I assume that the SPDK build will ignore the --with-idxd because it is not supported on x86-64-v2?

Copy link
Contributor

Choose a reason for hiding this comment

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

@caroav this can easily verified. We can just disassemble the SPDK binary and see if this instruction was used or not. This is what I did when we had the issue with RDSEED.

Copy link
Contributor Author

@CongMinYin CongMinYin Jun 24, 2024

Choose a reason for hiding this comment

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

@caroav I think we shuold check enable_dsa in ceph-nvmeof.conf, then configure with --with-idxd flag. But I don't how to write something like "if else" in .env or Dockerfile. It seems like you're saying the x86-64-v2 instruction set. But in reality, DSA is a DMA engine, an accelerator only integrated into the new 4th Gen CPU now.

idxd is a kernel driver. I have tried to configure idxd on server with AMD CPU, it will report an error and exit. So it's best to check the enable_dsa option in the ceph-nvmeof.conf. Provide different parameters in .env or Dockerfile. But I couldn't find a similar code block. Do you know what to do?
Run configure in a server with AMD CPU:

root@amd-server4:~/spdk# ./configure  --with-idxd
ERROR: IDXD cannot be used due to CPU incompatibility.

SPDK configure code to detect CPU:
https://github.com/spdk/spdk/blob/dfb2950fd5b13ba0a2a2b533e3e534202a89de74/configure#L741

Choose a reason for hiding this comment

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

I think the IDXD code itself is safe for all hardware platforms:

  1. It is scanning for a specific PCIe device (PCI_DEVICE_ID_INTEL_IDXD) and will not initialize if this device is not found. Older Intel CPUs and AMD CPUs will not have this device.
  2. It needs to use the movdir64b instruction to pass work to the hardware, this is only supported on new Intel CPUs. However this instruction is only ever executed if the PCIe device is found. Due to lack of compiler support this instruction is being generated using a sequence of bytes in assembly code:
static inline void movdir64b(void *dst, const void *src)
{
        asm volatile(".byte 0x66, 0x0f, 0x38, 0xf8, 0x02"
                     : "=m"(*(char *)dst)
                     : "d"(src), "a"(dst));
}

So building the code with --with-idxd will include a new assembly instruction, but it will only be executed on CPUs that support it because of the PCIe device check. Its very hard to prove this other than by code review.

The configure script for SPDK is only permitting the code to be compiled with --with-idxd if it is compiled on an Intel CPU. This means you can't use an AMD server as the build machine. The script is not checking the CPU version - so you can build the code with support for IDXD on an old Intel CPU.

I suggest that the check in the configure script is overly restrictive, and perhaps should be reduce to a warning rather than an error as the code may be built on a system with different capabilities to the system it is run on. It might also be better to check for the presence of the PCIe device rather than the CPU vendor to determine whether the build server supports idxd.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, we need support for the code to be compiled for non-Intel CPU

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@caroav Is our solution to try submitting a PR to modify the SPDK's config script, or check the enable_dsa parameter in this ceph-nvmeof project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @caroav @oritwas, I have submit a pr to solve the platforms errors: https://review.spdk.io/gerrit/c/spdk/spdk/+/24172. It merged. SPDK master branch and later versions won't met this platform errors.

SPDK_TARGET_ARCH="x86-64-v2"
SPDK_MAKEFLAGS=
SPDK_CENTOS_BASE="https://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/Packages/"
Expand Down
66 changes: 66 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,72 @@ sh -c 'echo 4096 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages'

This is automatically done in the `make setup` step. The amount of hugepages can be configured with `make setup HUGEPAGES=512`.

### Enable DSA to offload and accelerate CRC calculation

Intel® Data Streaming Accelerator (Intel® DSA) can generate and test CRC checksum or Data Integrity Field (DIF) on the memory region to support usages typical with storage and networking applications. This feature has already been implemented in SPDK. Enabling this feature allows for offloading and accelerating CRC calculations in NVMe-oF.

#### DSA Configuration

```bash
[spdk]
enable_dsa = True
```

#### Enable DSA in containers using vfio driver

Refer: [SPDK System Configuration User Guide: Device access](https://spdk.io/doc/system_configuration.html#system_configuration_nonroot_device_access).

1. Load `vfio` and `vfio-pci` drivers. Enable `IOMMU`.

2. Use DPDK to bind DSA as `vfio-pci`. In `ceph-nvmeof` directory, execute:

```bash
./spdk/dpdk/usertools/dpdk-devbind.py -s
```

The output may include:

```bash
DMA devices using DPDK-compatible driver
========================================
0000:6a:01.0 'Device 0b25' drv=vfio-pci unused=idxd
0000:e7:01.0 'Device 0b25' drv=vfio-pci unused=idxd
```

If the driver is not `vfio-pci`, bind it to this. Execute:

```bash
./spdk/dpdk/usertools/dpdk-devbind.py -u 0000:6a:01.0 0000:e7:01.0
./spdk/dpdk/usertools/dpdk-devbind.py -b vfio-pci 0000:6a:01.0 0000:e7:01.0
```

3. Check the IOMMU group of DSA devices:

```bash
readlink "/sys/bus/pci/devices/0000:6a:01.0/iommu_group"
```

The output should be e.g. `../../../kernel/iommu_groups/49`

4. update docker-compose.yaml:

```bash
nvmeof-base:
devices:
- /dev/vfio/vfio:/dev/vfio/vfio
- /dev/vfio/49:/dev/vfio/49
- /dev/vfio/250:/dev/vfio/250
volumes:
cap_add:
- IPC_LOCK # DMA pinning
```

Then run `make up` to start container. If DSA is successfully enabled, the following logs can be seen:

```bash
accel_dsa_rpc.c: 50:rpc_dsa_scan_accel_module: *NOTICE*: Enabled DSA user-mode
```

## Development

### Set-up
Expand Down
3 changes: 3 additions & 0 deletions ceph-nvmeof.conf
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ tgt_path = /usr/local/bin/nvmf_tgt
timeout = 60.0
#log_level = WARNING

# True / False to enable dsa
# enable_dsa = False

# Example value: -m 0x3 -L all
# tgt_cmd_extra_args =

Expand Down
9 changes: 9 additions & 0 deletions control/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,10 @@ def _start_spdk(self, omap_state):
spdk_tgt_cmd_extra_args = self.config.get_with_default(
"spdk", "tgt_cmd_extra_args", "")
cmd = [spdk_tgt_path, "-u", "-r", self.spdk_rpc_socket_path]
spdk_tgt_dsa = self.config.getboolean_with_default("spdk", "enable_dsa", False)
if spdk_tgt_dsa:
self.logger.info(f"Start SPDK, but wait for DSA detection before initialization")
cmd = [spdk_tgt_path, "-r", self.spdk_rpc_socket_path, "--wait-for-rpc"]
if spdk_tgt_cmd_extra_args:
cmd += shlex.split(spdk_tgt_cmd_extra_args)
self.logger.info(f"Starting {' '.join(cmd)}")
Expand Down Expand Up @@ -407,6 +411,11 @@ def _start_spdk(self, omap_state):
log_level=protocol_log_level,
conn_retries=conn_retries,
)
if spdk_tgt_dsa:
# init dsa
spdk.rpc.dsa.dsa_scan_accel_module(self.spdk_rpc_client)
spdk.rpc.framework_start_init(self.spdk_rpc_client)
self.logger.info(f"SPDK with DSA started")
except Exception:
self.logger.exception(f"Unable to initialize SPDK")
raise
Expand Down
2 changes: 1 addition & 1 deletion spdk
Loading