librdmacm: extend rsocket for Redis, iperf3, memcached and more Linux APIsRsocket upstream

[BatshevaBlack]

Summary

Extend the rsocket implementation in librdmacm so that applications such as Redis, iperf3, and memcached can use rsocket transparently via LD_PRELOAD (librspreload), and so rsocket aligns with more standard Linux socket and I/O behavior.

Motivation

The rsocket library did not fully support several POSIX/Linux interfaces (epoll, select, accept4, sendfile, fcntl64, and various socket options). Applications that rely on these either failed or fell back to TCP. This change extends the rsocket implementation to implement or fix those interfaces, so the preload can intercept them and route traffic over RDMA.

Changes

• Add/fix epoll (epoll_create, epoll_create1, epoll_ctl, epoll_wait)
• fix rpoll timeout handling and select,
• fix cm_svc_run
• add accept4, dup, fcntl64, sendfile64;
• fix rfcntl
• extend getsockopt/setsockopt;
• fix SOCK_STREAM/SOCK_DGRAM handling and connect service/TCP behavior;
• adjust wake-up timeout from rpoll.

[shefty]

poll support is non-trivial. Is there a reason why epoll support was implemented over rpoll?

[shefty]

See comments. I didn't review the epoll code in detail, as I would have expected the implementation to leverage to rpoll() path.

[BatshevaBlack]

poll support is non-trivial. Is there a reason why epoll support was implemented over rpoll?

@shefty
Thanks for your comments.
I didn’t use rpoll in the epoll path because the epoll thread would have to build a new pollfd array from the instance’s fds on every iteration, and I wanted to avoid that overhead.
So the thread does the same readiness work rpoll does (via rs_poll_rs) per fd instead of constructing the set and calling rpoll().
Semantics match rpoll, it’s just a different call path. I’m happy to refactor so the thread calls rpoll() if you’d rather have a single path.

[shefty]

If you can separate the epoll changes into another PR, that should help merge the other changes quicker.

This epoll implementation doesn't behave the same as poll. For example, if you look at rpoll(), loops are required for proper event handling. Similar loops are missing from repoll_wait(). I'll walk through what rpoll() is doing below.

Here are links to an epoll implementation over poll (called ofi_pollfds). The license is suitable for rdma-core. Note that the implementation targets windows, mac os, and freebsd, so there are extra abstractions that linux only support may not need.

https://github.com/ofiwg/libfabric/blob/main/include/ofi_epoll.h#L276
https://github.com/ofiwg/libfabric/blob/main/src/common.c#L1510

I can walk through what this code is doing if needed. The abstraction itself is NOT full epoll support. It does not handle attaching an epoll fd to another epoll fd, for example. Trying to support that would add significant complexity.

The rpoll() implementation has 2 while loops.

The first loop busy waits for a small period of time before moving to a blocking call. Technically, the use of a loop here isn't needed, as it's an optimization. However, the loop drives progress across the rsockets and checks for events, which is needed. We need to drive progress because an rocket may already have data to read, sitting in the local buffer. If that's the case, the rsocket must be marked for POLLOUT. There's no guarantee additional completions will be written to the rsocket's CQ or events added to its completion channel. The call to rs_poll_check() from rpoll() drives progress on the rsocket, so events aren't lost. rs_poll_check() checks all fd's, including non-rsocket fd's. This ensures that if we find an event on an rsocket, we don't miss also reporting events on non-rsocket fd's.

The second loop goes through the motions are arming the rsocket's CQ prior to blocking in poll(). Note that we call rs_poll_check() again to avoid dropping completions. We have to recheck the CQ for completions after it's been armed to avoid leaving entries on the CQ. If we have any completions to report, we return them, so we don't end up blocked in poll(). Once we're in poll(), if an event occurs, we need to process it. The occurrence of an event doesn't mean that it's an event that should be reported to a user. For example, the user could be waiting for POLLOUT, but we instead receive data. Because the user isn't checking POLLIN, we queue the data, then continue looping until POLLOUT is satisfied.

Looking at repoll_wait() and epoll_wait(), I don't see the above functionality. There's no attempt to drive progress. I don't see where CQ completion channels are armed or where the CQs are being polled. I don't see how multiple threads calling repoll_wait is handled (e.g. see rs_poll_enter / rs_poll_exit). There's a lot of complexity being handled by rpoll() which isn't there for repoll, which makes me question the implementation. That's why layering epoll over rpoll may be a better option, so we don't have to duplicate that complexity. (Yes, layering epoll over poll isn't exactly trivial either...)

Any epoll implementation we have will likely be limited relative to true epoll support. Those limits should be documented.

[shefty]

If I'm understand the code, epoll spawns a thread that spins in epoll_thread(). Is that correct?

repoll_wait() also never blocks, effectively ignoring the timeout, which could result in spinning the application thread.

If the above is correct, then I think the changes needed for epoll are to block the thread calling epoll_wait and avoid the epoll thread from spinning.

If we implement epoll using a thread, that thread should probably be idle until epoll_wait is called (to avoid contention driving progress on the same rsocket from 2 threads). The epoll thread should reuse more of the rpoll calls, rather than duplicating a large function like rs_poll_rs() with epoll_rs(). Repacing epoll_rs() with rs_poll_rs() is likely trivial. But I suspect we'll want to leverage additional calls to avoid spinning. It's okay if the rpoll flow needs to change some to make this happen.

[BatshevaBlack]

@shefty thanks!
I will go through your input about epoll
in the meantime I opened a new PR not including epoll changes #1717
I will remove the non epoll commits from this branch