[RFC] core: introduce Work Request API

[shijin-aws]

Split the monolithic data transfer post into prepare, queue, and flush stages so an application can build a work request, optionally attach provider-specific metadata, and submit then commit it as separate steps. Add the FI_WR endpoint capability to negotiate support, the FI_OPT_TX_REQ_SIZE / FI_OPT_RX_REQ_SIZE options to size request buffers, the FI_PREPARE_WORK control command (reusing FI_QUEUE_WORK and FI_FLUSH_WORK on an endpoint), the fi_wr_attr descriptor that reuses the fi_op_* operation descriptors, and the fi_prepare_work / fi_queue_work / fi_flush_work inline wrappers.

Add fi_wr(3) documenting the workflow in provider-neutral terms and document the new endpoint calls in fi_endpoint(3).

[shijin-aws]

This is only an RFC PR to get high level feedback. I will present it and get more feedback in the OFIWG meeting next week

[shefty]

I need to understand the broader intent and benefit of this feature. This is my interpretation of the higher-level flow broken out:

• Allocate WR
• Init WR
• Modify WR
• Submit WR
• Repeat the above
• Flush WR queue

Prepare = allocate and init WR
Modify = provider function(s)
Queue = submit WR
Flush

Is there an assumption that the above flow is split between the CPU and XPU, and if so, where does that split occur?

Is the intent that allocate return direct access into HW mapped queues, or just memory in some provider specific format?

The flow here seems similar to libibverb's qp ops, but I think there are intended differences.

Modify WR is some provider specific functions. If that's the case, then why not make all this flow provider specific? Allocate returns a provider specific blob of data. Modify is already provider specific, and queue could easily merge with modify. Under what conditions are provider functions not needed?

A hidden batch size is mentioned. That results in the behavior of queue and flush being identical to using FI_MORE. As soon as queue is called, the user must assume that the operation could have been written to the wire. (I can see a benefit of having a flush operation, versus leaving FI_MORE unset, but that's a separate concern.) There's no mention if WRs must be queued in the same order that they are allocated.

Is it expected that an EP would be configured to be in this mode always? Or is there a need to mix prepare-queue flows with standard submissions?

[shijin-aws]

See my answers for each of your comments. Generally speaking, I am open to keep the prepare/queue API as provider specific, while keeping the flush call as common as we proposed in #12041. But it is appreciated for any feedback that I can make such prepare/queue workflow as a friendly Libfabric interface that other provider can easily onboard.

Is there an assumption that the above flow is split between the CPU and XPU, and if so, where does that split occur?
Is the intent that allocate return direct access into HW mapped queues, or just memory in some provider specific format?

One use scenario is application use Libfabric to prepare the WQEs on host buffers (including the prepare call + provider specific setters), then XPU can initiated an DMA to copy these WQEs to the SQ BAR and ring the doorbell. SQ bar and DB are exposed via provider specific queries for the EP. So this is provider specific behavior.

Modify WR is some provider specific functions. If that's the case, then why not make all this flow provider specific? Allocate returns a provider specific blob of data. Modify is already provider specific, and queue could easily merge with modify. Under what conditions are provider functions not needed?

I do not think queue can easily merge with modify .... I would expect queue should be simply a DMA or memcpy to the correct slot in the hw queue. Provider functions are not needed if the WQE can be already formatted by the common fi_op_* struct

A hidden batch size is mentioned. That results in the behavior of queue and flush being identical to using FI_MORE. As soon as queue is called, the user must assume that the operation could have been written to the wire. (I can see a benefit of having a flush operation, versus leaving FI_MORE unset, but that's a separate concern.) There's no mention if WRs must be queued in the same order that they are allocated.

It is not required that WRs must be queued in the same order that they are allocated. But I would make it clear that the msg_order should map to the queue order.

Is it expected that an EP would be configured to be in this mode always? Or is there a need to mix prepare-queue flows with standard submissions?

As I can EP should not need special configure if is a verbs style provider. The prepare-queue flow are mostly allowing provider specific extension for the WQE that the current fi_* API cannot satisfy.

For example, if efa wants to insert a special metadata for WQEs of send, write, read operations which cannot be encapsulated by the fi_rma_msg and fi_msg, instead of proposing provider specific efa_send, efa_write, and efa_read APIs, I want a simple way to allow extending WQEs for any operations. This is the a general workflow that I come up with right now

[shefty]

Don't take my comment wrong, I think the discussion is good, as is having some common framework.

Provider specific behavior and data could be handled by the existing 'msg' APIs. For example, set a provider specific flag (bits 60-63) or define a common flag for this purpose. Then pass in the required changes through the context parameter using a provider specific format.

If we limit the scope to an EP accessible by the CPU, this may be all that's needed. The benefit of separating prepare WR from queue WR, given the FI_MORE flag, is unclear.

The XPU case seems different.

To allow queuing WRs in a different order from how they are allocated implies that the API is NOT working with HW mapped queues. So, for the XPU case, I'm assuming the problem is formatting of the WR is easier on the CPU than a GPU. If that's the case, then the goal is similar to XPU changes. Here the control path is the formatting of the WR and the data path is writing it to the HW queue. This suggests a new device call to queue the WR. It then becomes a question of exporting a WR to an XPU. For that, I considered a couple options, but I think a dedicated 'prepare' function is cleanest, though I might use a name more aligned with exporting the WR to an XPU. Unless there's a use case, I would limit the new API to EPs configured with FI_XPU support with extended fi_xpu_ctx_attr's for max WR sizes.

[shijin-aws]

Provider specific behavior and data could be handled by the existing 'msg' APIs. For example, set a provider specific flag (bits 60-63) or define a common flag for this purpose. Then pass in the required changes through the context parameter using a provider specific format.

Yeah, it is easy to implement a certain bit via provider specific flag. But what if there is some extra values (other than 1 or 0) that application needs to input to the provider, can u share the example of achieving this via context?

[shefty]

For example, see where the context parameter must sometimes be a pointer to struct fi_context, struct fi_context2, struct fi_triggered_context, or struct fi_triggered_context2. fi_triggered_context is closer to your use case.

You could define struct fi_efa_context. If an app sets a flag, say FI_PROV_CONTEXT, then it must provide a provider specific context structure as input into the call.

The contents and lifetime of that struct is EFA specific. Maybe it's only used for input. Maybe it acts like fi_context and must exist until the operation completes. The provider would need to define what gets reported in a completion. E.g. struct fi_efa_context could contain a void *context value which is reported.

[a-szegel]

I think queue/flush are data path operations (fast path) and they should not go through control() provider api

[shijin-aws]

yeah, I wouldn't go control path either ... this was only a PoC approach I did

[a-szegel]

Is it possible that all the different ops require different buffer sizes? Do we want to optimize this further by allowing providers specify the minimum size for each operation?

[shijin-aws]

I would avoid that ... this should be global to per endpoint

[a-szegel]

This statement is saying that the provider/libfabric have no associated state for each WR and that the user never needs to call fi_close(work). All of the state to use the WR API is fully owned by the application. Is this desired?

[shijin-aws]

we shouldn't make work as a fid resource, this is mapping to the mr_desc and av_desc model as we did earlier. It is just a buffer with size specified

[a-szegel]

the user calls fi_close(mr) and fi_close(av_entry)... but they never call fi_close(wr_entry).

[a-szegel]

If the endpoint's transmit resources are exhausted, the provider flushes any pending requests before returning -FI_EAGAIN, and work.wr_buf remains valid for retry.

I don't think we want this sentence in here. We should leave the behavior of what the provider wants to do on TX resource exhaustion up to the provider.

[a-szegel]

I don't think we should have anything after "provider-specific setters are outside the scope of this page. It doesn't make sense to define something that we say we are not defining.

[a-szegel]

I don't love this sentence b/c the next sentence inverts it.

fi_queue_wr never initiates transmission on its own; the application accumulates requests and

[a-szegel]

Lets pick either -FI_ENOSYS or -FI_EOPNOTSUPP but not define them both as options... that is not useful for the user.

Below you say:

A return of -FI_ENOSYS from any call indicates the provider does not implement
the WR API; the application should fall back to fi_write / fi_send / etc.

Not sure why we need both sentences.

[a-szegel]

This is also interesting... if we are flushing, why wouldn't we attempt to repost ourselves after we are flushed?

[a-szegel]

Does this mean the EP needs to be torn down or the work object is no longer valid? What parts are fatal and how does an app recover?

[a-szegel]

@shefty

The benefit of separating prepare WR from queue WR, given the FI_MORE flag, is unclear.

If the user knows all of the WQE's that they want to submit from startup time, they can take all of the WQE building work off the critical path, and move it to startup. If a user wants to submit the same WQE N times, this is also a performance improvement. This is a performance optimization that is valid on both the CPU and the XPU. If a field needs to change within the WQE, the provider can either make a new WQE with fi_prepare_wr() or they can call a provider specific setter to modify an existing WQE (as long as the user tracks that it is not currently submitted to the device), which is a memory optimization vs app complexity trade off.

Another silent advantage to doing this is it makes our threading story much simpler to follow b/c now the only things to worry about is the HW's SQ during queue/flush.

The current proposed plan is as follows, have a host API that only operates on host buffers, and an XPU API that only operates on XPU buffers. We plan on walking back @shijin-aws earlier comments which indicate that an fi_xpu_queue_wr() can operate on a host buffer.

The application owns all the state that exists for the WR API inside the struct fi_wr_attr *wr which is prepared on CPU/GPU from fi_prepare_wr()/fi_xpu_prepare_wr(). The user can choose to do whatever they want to the generated struct fi_wr_attr *wr (free it, modify it, copy it to the XPU), and if they corrupt it, they will get an CQ error on submission. Since all the state for the WR API is owned by the APP, they will never call fi_close(wr) to tell the provider that the WR is no longer valid. It will be provider specific on whether the provider supports copying returned WR's to different memory regions, or if the WR's need to be used in the memory region where they were created.

[a-szegel]

We also plan on dropping provider specific setters from the API docs and only going with the common prepare/queue/flush. If the provider wants WR's that are 99% the same but different, they will need to make multiple copies of them.

[shefty]

@a-szegel - based on your near to last comment, this is an approach I would consider, which I don't think is that far off from what's conceptually presented:

(Side note, this feature has similarities to both FI_MORE and fi_deferred_work, but not an exact fit for either.)

FI_WR -- is both a capability bit and a flag.

struct fi_wr_context -- replaces the proposed struct fi_wr_attr. I would consider defining it like this:

struct fi_wr_context {
    union {
        struct fi_context context;
        struct fi_context2 context2;
    };
};

struct fi_wr_context may be used interchangeably with fi_context or fi_context2, preserving that semantic. There are no user accessible fields.

Add fi_ep_attr::wr_size -- this is the (maximum) required size that struct fi_wr_context must reference. The provider should set wr_size large enough to hold any WR which may be submitted by on the EP. E.g. this could be the size of a QP entry plus max SGL. (I'm strongly tempted to call this ctx_size instead to allow a provider to define some arbitrarily sized context structure, which is basically what I'm doing.)

If FI_WR flag is passed to an existing API, the input context must be struct fi_wr_context of size wr_size. For practical purposes, this means that FI_WR may be passed as input into fi_sendmsg, fi_recvmsg, fi_writemsg, fi_readmsg, etc. This allows reusing the existing APIs for the 'prepare' and 'modify' functionality. Any provider set mode bits apply (e.g. FI_ASYNC_IOV).

The base behavior for FI_WR is 'prepare'. If FI_WR is set, the provider will format WR data, write it into struct fi_wr_context, and return. The called function (fi_xxxmsg) only returns a pre-formatted WR. (Note that SW providers, like the tcp, could simply capture the fi_xxxmsg parameters as provided, making FI_WR trivial to support.)

If we want to support a 'modify' function, that's more involved. For modify to be faster than prepare, we would need to document which fields of the WR are allowed to change. For example, the operation itself must remain the same. Maybe fi_writemsg only allows changing the target MR and destination address. We could probably defer modify for now, but it would require some sort of flag (FI_RESCAN?) being used to indicate modify from prepare.

A flush operation makes sense generically given FI_MORE. I would just define that in some reasonable way.

I'm not sure about the best way to queue the WR. A single 'queue' operation? Separate operations to 'queue_rx' and 'queue_tx'? Separate queue functions per operation, such as queue_msg, queue_rma, queue_tag, etc.? Or something more refined, such as queue_send, queue_recv, queue_write, queue_read, queue_tsend, queue_trecv, etc.? I can envision HW wanting more-refined queuing to avoid internal branches.

[shefty]

Regarding queuing... If queuing were done through the existing APIs via a flag (FI_READY? -- I don't see a good existing one), that would allow for advanced use cases, such as 'prepare & queue' or 'modify & queue', where the fi_wr_context could be formatted and queued, with the formatted WR returned. This approach would align with the explicit queuing calls (fi_sendmsg, fi_recvmsg, etc.).

If we wanted to tie this in with FI_MORE, FI_WR | FI_READY could prepare + queue + flush, but FI_WR | FI_READY | FI_MORE would only prepare + queue.

In all cases, the updated WR would be returned for further use.

This would be flexible, but does result in a slightly more difficult API to use.

[j-xiong]

Is the definition of fi_wr_context intended to provide the space needed for FI_CONTEXT and FI_CONTEXT2 mode in addition to the space needed to hold the WR? Would it be wasted space if these modes are not used?

[shefty]

Ultimately, the app must back fi_wr_context with a buffer of size wr_size.

The intent of embedding fi_context / fi_context2 within is to ensure that the structure can always act as either of those structures. If wr_size is < sizeof(fi_context2), then, yes, it would consume more space than needed. We could define fi_wr_context as empty:

struct fi_wr_context {
    uint64_t data[];
}

Then it's on the provider to set wr_size accordingly if it needs fi_context2.

[a-szegel]

Hey @shefty,

There are a few reasons why I think the new WR API is a better idea than re-using the existing fi_msg/fi_write/fi_read/fi_atomic API's:

1. fi_getinfo(3) defines struct fi_context as the per operation context, which must remain valid until that operation completes. It is provider scratch for one in-flight operation. The case motivating this feature is prepare
   once, queue the same WR N times. Those N concurrent operations share a single scratch area, and the CQ reports the same op_context for all of them.
2. Simplicity: Having a new dedicated API for a new feature allows us to define what flags we support, and what flags we do not support. If we re-use the msg/rma/atomic API's, it will be less obvious to the user what flags are/are not supported when passing in fi_context_wr
3. Abstraction: The prepare function can be thought of as control path, and the queue/flush operations are data path. When we merge them all into the data path calls, it breaks the abstraction of the API.
4. Branching: Keeping the data path as simple as possible makes it easier for one to optimize it, and requires less branching.

Can you tell me your reasons for prefering to re-use the existing API's over creating new API's?

[shefty]

The question is whether we conceptually have:

prepare(opcode, ...) // opcode may be embedded into an input structure
vs
prepare-opcode(...) // one call per opcode

The corresponding API models are: triggered ops (top case) and deferred work (bottom case). In both cases, queuing is the 'trigger'.

I view the proposal as an optimization over the existing APIs. The transport operation and other behavior are unchanged. That is, this flow should not reduce any functionality that an endpoint is expected to provide. prepare-queue-flush should be interchangeable with fi_.

The input into prepare should be separate from the output. The output should have no user interpreted data (e.g. fi_wr_context). If using FI_XPU, the fi_wr_context must be exportable to the device.

We should also figure out modify. I'm thinking of this as a starting point:

modify-field1(wr, ...) // e.g. modify-dest_addr(wr, dest_addr)
modify-field2(wr, ...) // e.g. modify-flags(wr, flags)
modify-field3(wr, ...) // e.g. modify-tag(wr, tag, ignore)
modify-field4(wr, ...) // e.g. modify-src_iov(wr, iov, desc, iov_count)
etc.

This results in a large number of API calls but allows for refined changes. A provider could still support vendor optimized helpers as alternatives. Given the above approach, expanding to modify-opcode-fieldX(wr, ...) would be unwieldy. This option likely eliminates a single modify+queue type function.

IMO, both prepare and modify should be designed as fast path operations. Modify support from a device seems nice.

Queue may or may be align with prepare, but has similar options:

queue(opcode, ...) // opcode embedded into WR
or
queue_opcode(wr, ...) // assert wr-opcode matches call

There are potentially multiple queues behind a single EP. Sends and receives go to separate queues. Untagged and tagged buffers are separated. Other transports (e.g. UET) may have even more. The decision is whether the app should indicate which queue to use (e.g. queue_tsend), or if the provider must rediscover it.

Regardless of which option is chosen, we end up with branches. queue(wr,) may require the provider to store extra metadata or extract the operation from some HW specific format. queue_opcode(wr) gives the app a chance to specify the wrong queue.

[a-szegel]

How will we support providers that requires FI_CONTEXT[2] for generating completions when reusing existing API's?

FI_CONTEXT
Specifies that the provider requires that applications use struct fi_context as their per operation context parameter for operations that generated full completions.

If we are overloading the unique per operation FI_CONTEXT[2] field with a pointer to a provider fi_wr_context (WQE), we no longer can use that uint64_t for the context. The provider can allocate an additional uint64_t at the end of their fi_wr_context, but that requires that only 1 instance of the fi_wr_context can be submitted waiting completion at any one time. The user should be able to modify fi_wr_context after queue returns because the WR should be copied to the providers hardware queue immediately, and the fi_wr_context should no longer be needed by the device. This means we would need 1 fi_wr_context per in-progress operation, which would not be efficent for memory usage

Example:

// Broken
fi_prepare(wr)
fi_queue(wr)   //  Copies wr from FI_HMEM_XXXX to device WQ
fi_modify(wr, dst=1) 
fi_queue(wr)    //  BROKEN if FI_CONTEXT[2] is required and is part of fi_wr_context
fi_flush(ep)
fi_cq_read(cq)


// Solution 1 - 2 WR's
fi_prepare(wr)
fi_queue(wr)   //  Copies wr from FI_HMEM_XXXX to device WQ
fi_prepare(wr2, dst=1) 
fi_queue(wr2)   
fi_flush(ep)
fi_cq_read(cq)


// Solution 2 - 1 WR active at a time
fi_prepare(wr)
fi_queue(wr)   //  Copies wr from FI_HMEM_XXXX to device WQ
fi_flush(ep)
while (not yet received completion for wr)
      fi_cq_read(cq)
fi_modify(wr, dst=1) 
fi_queue(wr)    
fi_flush(ep)

[shefty]

My last comment was not a strong endorsement for reusing the existing APIs. Actually, if we include modify, we may want an entire new set of ep calls.

That said, I still think this works. The fi_wr_context cannot be used with a second queue() operation until the first one completes. However, nothing prevents us from allowing modify() after queue(), but before a completion is generated. That would need to be part of the definition for how fi_wr_context may be used.

Internally, a provider could handle this by treating fi_wr_context as having 2 separate components. One is directly related to the WR and may be modified. The other is related to the active transfer. The provider may need to copy information from the "WR" to the active component within queue() to make this work.

The alternative definition would be to disallow modifying the fi_wr_context until a completion is generated. This may be more efficient for the provider, but less efficient for the app.

In either case, I would still have the app only deal with passing in fi_wr_context and not need both fi_wr_context and fi_context2.

[shefty]

Okay, I see. You want to call modify + queue in a loop prior to flush. Yes, in that case, we need the WR structure separate from the transfer tracking.

[a-szegel]

ooops sorry forgot to add flush to my example... edited

[shefty]

Based on the prior examples, I think that leads to:

prepare(ep, wr_attr, *wr);
modify-X(wr, X,)
queue(ep, wr, context)
flush(ep)

modify() should be fast path. prepare() doesn't seem as critical -- needs to be about the same performance as the fi_*msg calls. queue() needs the ep, so the call can route to the provider.

A single prepare() call seems sufficient then. But should queue functionality be optimized? E.g. queue_send, queue_tsend, etc. Otherwise, queue will likely have an immediate switch() (Queue may be able to get away with only tx, rx, and tagged rx varieties).