allow to disable dlopen for rdma libs

[remicollet]

In 9.1 libvalkey has support to use librdmacm and libibverbs directly or to dlopen them. The dlopen method was introduced in #3072.

The build flags USE_DLOPEN_RDMA is always set when building libvalkey bundled in valkey

This PR allow to build using
make BUILD_RDMA=module USE_DLOPEN_RDMA=0

Keeping the same default with
make BUILD_RDMA=module

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.43%. Comparing base (afe6ee1) to head (84275ab).

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3383      +/-   ##
============================================
- Coverage     74.46%   74.43%   -0.04%     
============================================
  Files           130      130              
  Lines         72730    72730              
============================================
- Hits          54160    54135      -25     
- Misses        18570    18595      +25     

see 21 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[zuiderkwast]

The build flags USE_DLOPEN_RDMA is always set when building libvalkey bundled in valkey

Not exactly always. When valkey is built with BUILD_RDMA=yes, libvalkey is built with statically linked rdma libs.

valkey BUILD_RDMA	libvalkey
yes	static
module	dlopen
no	none

What's the need for building without the dlopen support?

[remicollet]

What's the need for building without the dlopen support?

Benefit of dlopen ?

• save a few KB

Problems of dlopen

• fragile (rely on stable ABI)
• hardcoded library soname
• breaks RPM auto-dependencies

So, I prefer to not use this feature.

[zuiderkwast]

• fragile (rely on stable ABI)
• hardcoded library soname

This isn't any different than dynamic linking in general, right? Dynamic linking is used extensively by many programs.

• breaks RPM auto-dependencies

So the server-RDMA-module uses statically linked with RDMA libs but the cli and benchmark need dynamic RDMA libs, and they're not listed in the ELF so they're not resolved as dependencies? I'm starting to see the problem now.

If you don't care about a few extra KB, why not just build (server and tools) with USE_RDMA=yes?

I'd like to avoid an explosion of different ways to build. We can revise #3072 if needed and do something in a different way before 9.1 GA.

@Conan-Kudo @natoscott @Ada-Church-Closure it'd be good if you packagers(?) could discuss and try to unify on how you'd like to package these things.

[remicollet]

So the server-RDMA-module uses statically linked with RDMA libs but the cli and benchmark need dynamic RDMA libs, and they're not listed in the ELF so they're not resolved as dependencies? I'm starting to see the problem now.

Yes

And RPM dependencies are a bit more precise than only the soname (thanks to versioned symbols)

From package requirement:

libibverbs.so.1()(64bit)
libibverbs.so.1(IBVERBS_1.0)(64bit)
libibverbs.so.1(IBVERBS_1.1)(64bit)
librdmacm.so.1()(64bit)
librdmacm.so.1(RDMACM_1.0)(64bit)

Also, the fallback on librdmacm.so/libibverbs.so (not the soname) can be problematic

[Ada-Church-Closure]

Hi @remicollet and @zuiderkwast, thanks for explaining the details!

Since I mainly test on Arch Linux (where dependencies are usually declared manually), I didn't realize the dlopen approach would break RPM's auto-dependency scanning and cause ABI versioning issues. My original goal was simply to keep the CLI tools as lightweight as possible for users who don't actually use RDMA.

PR #3383 makes perfect sense to me. Adding USE_DLOPEN_RDMA=0 is a great way to keep the default lightweight, while giving downstream packagers the flexibility to use standard dynamic linking when they need it.

I fully support this change. Thanks for catching my blind spot and providing the fix!