prov/efa: emulated write Rma protocols refactor by charlesstoll · Pull Request #12843 · ofiwg/libfabric

charlesstoll · 2026-09-15T22:49:21Z

This patch series contains:

a few small pre-work changes to prepare for the shared emulated write protocol engine code
adds the emulate write protocol engine
adds the eager rtw protocol to the new framework

sunkuamzn · 2026-09-17T20:48:11Z


 		if (selected_proto->can_use_protocol(
-			    txe, req_pkt_type, header_flags, iface)) {
+			    txe, req_pkt_type, header_flags, iface, false)) {


It's ok for now but this shouldn't always be set to false. See the logic here

libfabric/prov/efa/src/rdm/efa_rdm_msg.c

Lines 197 to 201 in 3799050

err = efa_rdm_ep_use_p2p_for_mr(ep, txe->desc[0]);

if (err < 0)

return err;

use_p2p = err;

I'm implementing it when I add the read based protocols

fixed this so it doesn't hardcode false

sunkuamzn · 2026-09-17T20:58:22Z

+	 * do not need it.
+	 */
+	if (use_p2p &&
+	    txe->total_len >= g_efa_hmem_info[iface].min_read_write_size &&


Should this also be gated behind a wants_mr or needs_mr field?

You could also skip this for now/fix it later because the eager protocol will never register anyway

sunkuamzn · 2026-09-17T21:01:24Z

+ * @param[in,out]	pkt_entry	received EFA_RDM_EAGER_RTW packet
+ *
+ */
+void efa_rdm_pke_handle_eager_rtw_recv(struct efa_rdm_pke *pkt_entry)


Can we rename the prefixes? e.g. efa_rdm_proto_handle_eager_rtw_recv_completion?

sunkuamzn · 2026-09-17T21:09:51Z

 * negative libfabric error code on error.
 */
-static inline
 ssize_t efa_rdm_pke_init_rtw_common(struct efa_rdm_pke *pkt_entry,


Can you move this function to prov/efa/src/rdm/protocols as well and make it static inline? Not worth a function call just to set a few fields

You could make a new header efa_rdm_proto_write.h

sunkuamzn · 2026-09-17T21:11:58Z

+		       ep->efa_rnr_queued_pkt_cnt >
+	       0);
+
+	pkt_entry = efa_rdm_pke_alloc(ep, ep->efa_tx_pkt_pool,


This is now commonized in my PR. I can replace it with the common code.

sounds good. if yours goes in first though, then I'll make sure to commonize when I rebase

yinliaws · 2026-09-17T21:37:55Z

+	 */
+	*pke_send_flags = (txe->fi_flags & FI_MORE) ? FI_MORE : 0;
+
+	assert(ep->efa_max_outstanding_tx_ops - ep->efa_outstanding_tx_ops -


I think we should return -FI_EAGAIN instead of asserting when TX credits are exhausted.

Checking for TX credits should happen in the earlier function

libfabric/prov/efa/src/rdm/efa_rdm_msg.c

Lines 253 to 258 in 3a7bdae

// Handle case when there are no TX packets available

available_tx_pkts = efa_rdm_ep_get_available_tx_pkts(ep);

if (OFI_UNLIKELY(available_tx_pkts == 0)) {

err = -FI_EAGAIN;

goto out;

}

So an assert here should work

added the assert back, added a credit check in the previous function.

yinliaws · 2026-09-17T21:41:16Z

+	for (i = 0; efa_rdm_emulated_write_protocols[i] != NULL; ++i) {
+		selected_proto = efa_rdm_emulated_write_protocols[i];
+
+		req_pkt_type = efa_rdm_proto_req_pkt_type(


The type helper efa_rdm_proto_req_pkt_type forces a non-DC packet whenever FI_INJECT is present, even if FI_DELIVERY_COMPLETE is also requested.

good catch. fixed it in my patch and fixed the common send code as well in a new patch (still this pr).

I don't think FI_INJECT + FI_DELIVERY_COMPLETE is a valid combination. FI_DELIVERY_COMPLETE necessarily requires the buffer to be available for retransmits. It can be implemented with a copy but that defeats the point of FI_INJECT - which is to send the data as quickly as possible.

fixed... again :)

charlesstoll · 2026-09-18T16:15:36Z

bot:aws:retest

sunkuamzn · 2026-09-18T19:31:13Z

+ * 0 on success.
+ * negative libfabric error code on failure
+ */
+static ssize_t efa_rdm_proto_eager_write_init_dc_rtw(struct efa_rdm_pke *pkt_entry,


nit: would it be easier to follow if the TX and RX functions were put together? Right now, the order is

efa_rdm_proto_eager_write_init_rtw
efa_rdm_pke_handle_eager_rtw_send_completion
efa_rdm_proto_eager_write_proc_rtw
efa_rdm_proto_eager_write_handle_rtw_recv
efa_rdm_proto_eager_write_init_dc_rtw
efa_rdm_proto_eager_write_handle_dc_rtw_recv

How about

/* TX path functions */
efa_rdm_proto_eager_write_init_rtw
efa_rdm_proto_eager_write_init_dc_rtw
efa_rdm_pke_handle_eager_rtw_send_completion

/* RX path functions */
efa_rdm_proto_eager_write_proc_rtw
efa_rdm_proto_eager_write_handle_rtw_recv
efa_rdm_proto_eager_write_handle_dc_rtw_recv

A behavior-preserving cleanup ahead of routing emulated writes through the protocol interface. Rename the efa_rdm_proto callback can_use_protocol_for_send to can_use_protocol. The same callback is used by both the send and the emulated write paths, so the send-specific name is misleading. Signed-off-by: Charles Stoll <stollcha@amazon.com>

Add the write engine on the efa_rdm_proto interface. The main differences from send is that the write engine must first check if the device is write capable and use that, if possible. Modifies the protocol selection API to pass in use_p2p since this is relevant to the write protocols. No write protocol is registered yet, so selection always returns none and every write falls through to the existing selection. Signed-off-by: Charles Stoll <stollcha@amazon.com> (cherry picked from commit 925bf12)

Implement the emulated eager write protocol on the efa_rdm_proto interface and register it in the emulated write protocol list. Signed-off-by: Charles Stoll <stollcha@amazon.com> (cherry picked from commit 92984d4)

Move the shared RTW payload helper into a static inline in protocols/efa_rdm_proto_write.h so the eager write protocol and the LONGCTS RTW path share it without exporting it from the pke layer, and rename it to efa_rdm_proto_write_rtw_pke_init_common to match the protocol interface naming. Signed-off-by: Charles Stoll <stollcha@amazon.com>

Group the eager write helpers into TX and RX path sections. No functional change. Signed-off-by: Charles Stoll <stollcha@amazon.com>

charlesstoll force-pushed the rma_refactor branch 3 times, most recently from a054004 to 63e5433 Compare September 17, 2026 16:57

charlesstoll mentioned this pull request Sep 17, 2026

prov/efa: Rma read refactor #12854

Open

sunkuamzn reviewed Sep 17, 2026

View reviewed changes

yinliaws reviewed Sep 17, 2026

View reviewed changes

charlesstoll force-pushed the rma_refactor branch 3 times, most recently from 363953f to 0cbe348 Compare September 18, 2026 15:41

charlesstoll force-pushed the rma_refactor branch from 0cbe348 to 1917721 Compare September 18, 2026 16:26

yinliaws previously approved these changes Sep 18, 2026

View reviewed changes

sunkuamzn reviewed Sep 18, 2026

View reviewed changes

charlesstoll dismissed yinliaws’s stale review via 45a81cf September 18, 2026 20:51

charlesstoll force-pushed the rma_refactor branch from 1917721 to 45a81cf Compare September 18, 2026 20:51

charlesstoll added 4 commits September 18, 2026 14:48

prov/efa: Add the emulated eager write protocol

aa35716

Implement the emulated eager write protocol on the efa_rdm_proto interface and register it in the emulated write protocol list. Signed-off-by: Charles Stoll <stollcha@amazon.com> (cherry picked from commit 92984d4)

prov/efa: Reorder functions in the eager write protocol file

d9dc024

Group the eager write helpers into TX and RX path sections. No functional change. Signed-off-by: Charles Stoll <stollcha@amazon.com>

charlesstoll force-pushed the rma_refactor branch from 45a81cf to d9dc024 Compare September 18, 2026 21:50

yinliaws approved these changes Sep 18, 2026

View reviewed changes

	err = efa_rdm_ep_use_p2p_for_mr(ep, txe->desc[0]);
	if (err < 0)
	return err;

	use_p2p = err;

	// Handle case when there are no TX packets available
	available_tx_pkts = efa_rdm_ep_get_available_tx_pkts(ep);
	if (OFI_UNLIKELY(available_tx_pkts == 0)) {
	err = -FI_EAGAIN;
	goto out;
	}

Conversation

charlesstoll commented Sep 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunkuamzn Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlesstoll commented Sep 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sunkuamzn Sep 17, 2026 •

edited

Loading