Skip to content

cmake: use ExternalProjects to build isa-l and isa-l_crypto libraries#59513

Merged
cbodley merged 2 commits intoceph:mainfrom
cbodley:wip-isal-ext
Oct 3, 2024
Merged

cmake: use ExternalProjects to build isa-l and isa-l_crypto libraries#59513
cbodley merged 2 commits intoceph:mainfrom
cbodley:wip-isal-ext

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Aug 29, 2024 •
edited
Loading

instead of building isa-l source files directly into the ec_isa and ceph_crypto_isal plugins, use cmake ExternalProjects to build the libraries with their own makefiles, and expose the static libraries as cmake targets ISAL::ISAL and ISAL::Crypto

as pointed out in #59494, the current cmake scripts don't include any of the avx512 sources. after switching to ExternalProjects, i've confirmed that the resulting plugins do contain avx512 symbols:

~/ceph/build $ nm lib/libceph_crypto_isal.so | grep avx512 | wc -l
3

~/ceph/build $ nm lib/libec_isa.so | grep avx512 | wc -l
61

Fixes: https://tracker.ceph.com/issues/67943

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@cbodley cbodley requested a review from hualongfeng August 29, 2024 15:22
@cbodley cbodley mentioned this pull request Aug 29, 2024
Closed
14 tasks
@cbodley cbodley requested a review from jamiepryde August 29, 2024 15:22
hualongfeng
hualongfeng reviewed Aug 30, 2024
View reviewed changes
@cbodley cbodley requested a review from a team as a code owner September 5, 2024 16:28
@github-actions github-actions bot added the core label Sep 5, 2024
@cbodley cbodley changed the title cmake/crypto: use ExternalProject to build isal crypto library cmake: use ExternalProjects to build isa-l and isa-l_crypto libraries Sep 5, 2024
@cbodley
cmake/crypto: use ExternalProject to build isal crypto library
3bfc1ae 
instead of building isa-l_crypto source files directly into the
ceph_crypto_isal plugin, use a cmake ExternalProject to build the
library with make and expose the static library as cmake target
ISAL::Crypto

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
cmake/ec: use ExternalProject to build isal library
926ef93 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Sep 6, 2024

@jamiepryde i ended up doing this for the ec plugin too

builds were successful in https://shaman.ceph.com/builds/ceph/wip-isal-ext/

@jamiepryde
Copy link
Contributor

jamiepryde commented Sep 6, 2024

Looks good to me @cbodley
I've tested your branch by building and running unittest_erasure_code_isa, and can see the avx512 symbols in libec_isa.so

@cbodley
Copy link
Contributor Author

cbodley commented Sep 6, 2024

i created https://tracker.ceph.com/issues/67943 to track reef/squid backports

@cbodley
Copy link
Contributor Author

cbodley commented Sep 6, 2024

(cc @rzarzynski since you were the default assignee on https://bugzilla.redhat.com/show_bug.cgi?id=2310433)

@cbodley
Copy link
Contributor Author

cbodley commented Sep 6, 2024

passed rgw qa in https://pulpito.ceph.com/cbodley-2024-09-06_13:43:33-rgw-wip-isal-ext-distro-default-smithi/ (qa/suites/rgw/crypt exercises the ceph_crypto_isal plugin)

rzarzynski
rzarzynski approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Thank you, @cbodley!

@mkogan1 mkogan1 self-requested a review September 10, 2024 13:58
mkogan1
mkogan1 approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@mkogan1 mkogan1 left a comment

Choose a reason for hiding this comment

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

To test, a relatively new machine which CPU supports vaes capability is required:

❯ lscpu | grep vaes
Flags:                          	fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3 cat_l2 cdp_l3 cdp_l2 ssbd mba ibrs ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid cqm rdt_a avx512f avx512dq rdseed adx smap avx512ifma clflushopt clwb intel_pt avx512cd sha_ni avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local split_lock_detect avx_vnni avx512_bf16 wbnoinvd dtherm ida arat pln pts hwp hwp_act_window hwp_epp hwp_pkg_req vnmi avx512vbmi umip pku ospke waitpkg avx512_vbmi2 gfni vaes vpclmulqdq avx512_vnni avx512_bitalg tme avx512_vpopcntdq la57 rdpid bus_lock_detect cldemote movdiri movdir64b enqcmd fsrm md_clear serialize tsxldtrk pconfig arch_lbr ibt amx_bf16 avx512_fp16 amx_tile amx_int8 flush_l1d arch_capabilitie

like:

❯ lscpu
  ...
  Model name:        	Intel(R) Xeon(R) Silver 4416+
  ...

verified by runing on an vstart env with -o plugin_crypto_accelerator=crypto_isal -o rgw_crypt_default_encryption_key=4YSmvJtBv0aZ7geVgAsdpRnLBEwWSWlMIGnRS8a9TSA=

and monitored the avx512 instruction:

❯  nm lib/libceph_crypto_isal.so | grep avx512
0000000000012dc0 t aes_cbc_dec_128_vaes_avx512
0000000000013e41 t aes_cbc_dec_192_vaes_avx512
00000000000150b0 t aes_cbc_dec_256_vaes_avx512

using the tool: https://www.intel.com/content/www/us/en/developer/articles/tool/software-development-emulator.html -- Intel® Software Development Emulator (Intel® SDE)

with the command:
sudo ./sde64 -mix -attach-pid $(pidof radosgw)

to exercise the aes_cbc_dec_..._vaes_avx512 instructions an S3 GET operation is performed (b/c ..._dec_...)

after stopping RGW, searching the sde64 output file for aes_cbc_dec_..._vaes_avx512 instructions:

grep -i 'aes_cbc_dec_..._vaes_avx512' ./sde-mix-out.txt
…
   0:   1857917952  55.777  55.777   	1373184  	7fff0ea2f0b0 aes_cbc_dec_256_vaes_avx512  IMG: /home/ceph/src-git/ceph--up--main/build/lib/libceph_crypto_isal.so
# $dynamic-counts-for-function: aes_cbc_dec_256_vaes_avx512  IMG: /home/ceph/src-git/ceph--up--main/build/lib/libceph_crypto_isal.so at [0x7fff0ea2f0b0, 0x7fff0ea30510)  55.777%
BLOCK: 	0   PC: 00007fff0ea2f13a   ICOUNT: 1828749312   EXECUTIONS:  22577152   #BYTES: 498   %:  53.8   cumltv%:  53.8  FN: aes_cbc_dec_256_vaes_avx512  IMG: /home/ceph/src-git/ceph--up--main/build/lib/libceph_crypto_isal.so  OFFSET: 1513a
BLOCK: 	3   PC: 00007fff0ea2f12d   ICOUNT:  47976448   EXECUTIONS:  23988224   #BYTES: 13   %:  1.41   cumltv%:  63.4  FN: aes_cbc_dec_256_vaes_avx512  IMG: /home/ceph/src-git/ceph--up--main/build/lib/libceph_crypto_isal.so  OFFSET: 1512d
BLOCK: 	5   PC: 00007fff0ea2f0be   ICOUNT:  22577152   EXECUTIONS:   1411072   #BYTES: 111   %: 0.665   cumltv%:  64.7  FN: aes_cbc_dec_256_vaes_avx512  IMG: /home/ceph/src-git/ceph--up--main/build/lib/libceph_crypto_isal.so  OFFSET: 150be
…

image

With this PR the aes_cbc_dec_256_vaes_avx512 instructions are present, before this PR they are NOT present.

@cbodley
Copy link
Contributor Author

cbodley commented Oct 1, 2024

hey @SrinivasaBharath, any updates on testing here?

@ronen-fr
Copy link
Contributor

ronen-fr commented Oct 3, 2024

hey @SrinivasaBharath, any updates on testing here?

Rados approved. https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrackercephcomissues68124

@cbodley cbodley merged commit f185578 into ceph:main Oct 3, 2024
@cbodley cbodley deleted the wip-isal-ext branch October 3, 2024 15:31
This was referenced Oct 3, 2024
Merged
Merged
@jamiepryde jamiepryde mentioned this pull request Oct 14, 2024
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hualongfeng hualongfeng hualongfeng left review comments

@rzarzynski rzarzynski rzarzynski approved these changes

@mkogan1 mkogan1 mkogan1 approved these changes

@jamiepryde jamiepryde jamiepryde approved these changes

Assignees

No one assigned

Labels

build/ops core performance ready-to-merge TESTED

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@cbodley @jamiepryde @ronen-fr @rzarzynski @mkogan1 @hualongfeng @SrinivasaBharath