On 8/25/2022 11:21 PM, Cheng Xu wrote:
On 8/26/22 12:37 AM, Tom Talpey wrote:
On 8/24/2022 9:54 PM, Cheng Xu wrote:
On 8/24/22 10:08 PM, Tom Talpey wrote:
On 8/24/2022 5:42 AM, Cheng Xu wrote:
Hi,
This series introduces erdma's implementation of drain_sq and drain_rq.
Our hardware will stop processing any new WRs if QP state is error.
Doesn't this violate the IB specification? Failing newly posted WRs
before older WRs have flushed to the CQ means that ordering is not
preserved.
I agree with Bernard's point.
I'm not very familiar with with IB specification. But for RNIC/iWarp [1],
post WR in Error state has two optional actions: "Post WQE, and then Flush it"
or "Return an Immediate Error" (Showed in Figure 10). So, I think failing
newly posted WRs is reasonable.
<...> But the QP can only enter ERROR once the
SQ and RQ are fully drained to the CQ(s). Until that happens, the
WRs need to flush through.
Emm, let's put erdma aside first, it seems that specification does not require
this. According to "6.2.4 Error State" in the document [1]:
The following is done on entry into the Error state:
* The RI MUST flush any incomplete WRs on the SQ or RQ.
.....
* At some point in the execution of the flushing operation, the RI
MUST begin to return an Immediate Error for any attempt to post
a WR to a Work Queue;
....
As the second point says, The flushing operation and the behavior of returning
Immediate Error are asynchronous. what you mentioned is not guaranteed. Failing
the post_send/post_recv may happens at any time during modify_qp to error.
[1] http://www.rdmaconsortium.org/home/draft-hilland-iwarp-verbs-v1.0-RDMAC.pdf
Well, that language is very imprecise, "at some point" is not exactly
definitive. I'll explain one scenario that makes it problematic.
Your code seems to start failing WR's when the TX_STOPPED or RX_STOPPED
bits are set. But that bit is being set when the drain *begins*, not
when it flushes through. That seems wrong, to me.
Back to erdma's scenario, As I explains above, I think failing immediately when
flushing begins does not violate the specification.
Consider a consumer which posts with a mix of IB_SEND_SIGNALED and
also unsignaled WRs, for example, fast-memory registration followed
by a send, a very typical storage consumer operation.
- post_wr(memreg, !signaled) => post success
- => operation success, no completion generated
- ... <= provider detects error here
- post_wr(send, signaled) => post fail (new in your patch)
- ... <= provider notifies async error, etc.
The consumer now knows there's an error, and needs to tear down.
It must remove the DMA mapping before proceeding, but the hardware
may still be using it. How does it determine the status of that
first post_wr, so it may proceed?
The IB spec explicitly states that the post verb can only return
the immediate error after the QP has exited the ERROR state, which
includes all pending WRs having been flushed and made visible on
the CQ. Here is an excerpt from the Post Send Request section
11.4.1.1 specifying its output modifiers:
-> Invalid QP state.
-> Note: This error is returned only when the QP is in the Reset,
-> Init, or RTR states. It is not returned when the QP is in the Error
-> or Send Queue Error states due to race conditions that could
-> result in indeterminate behavior. Work Requests posted to the
-> Send Queue while the QP is in the Error or Send Queue Error
-> states are completed with a flush error.
So, the consumer will post a new, signaled, work request, and wait
for it to "flush through" by polling the CQ. Because WR's always
complete in-order, this final completion must appear after *all*
prior WR's, and this gives the consumer the green light to proceed.
With your change, ERDMA will pre-emptively fail such a newly posted
request, and generate no new completion. The consumer is left in limbo
on the status of its prior requests. Providers must not override this.
Tom.
Many upper layers depend on this.
For kernel verbs, erdma currently supports NoF. we tested the NoF cases,
and found that it's never happened that newly WRs were posted after QP
changed to error, and the drain_qp should be the terminal of IO process.
Also, in userspace, I find that many providers prevents new WRs if QP state is
not right.
Sure, but your new STOPPED bits don't propagate up to userspace, right?
Yes. they are only used for kernel QPs. The bits are used for setting an accurate
time point to prevent newly WRs when modify qp to error.
The post calls will fail unexpectedly, because the QP is not yet in
ERROR state.
Do you mean this happens in userspace or kernel? The new bits do not influence
userspace, and erdma will have the same behavior with other providers in userspace.
For kernel, this is only used in drain_qp scenario. posting WRs and drain qp
concurrently will lead uncertain results. This is also mentioned in the comment
of ib_drain_qp:
* ensure that there are no other contexts that are posting WRs concurrently.
* Otherwise the drain is not guaranteed.
I'm also concerned how consumers who are posting their own "drain" WQEs
will behave. This is a common approach that many already take. But now
they will see different behavior when posting them...
For userspace, erdma is not special.
For kernel, I think the standard way to drain WR is using ib_drain_qp, not custom
implementation. I'm not sure that there is some ULPs in kernel has their own drain
flow.
Thanks,
Cheng Xu
Tom.
So, it seems ok in practice.
Thanks,
Cheng Xu
[1] http://www.rdmaconsortium.org/home/draft-hilland-iwarp-verbs-v1.0-RDMAC.pdf
