Re: [PATCH for-next 0/2] RDMA/erdma: Introduce custom implementation of drain_sq and drain_rq

[Tom Talpey <tom@talpey.com>]

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.

Both IB and iWARP allow new post-WR calls to fail synchronously if
the QP is in the ERROR state. 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.

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.

>> 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?
The post calls will fail unexpectedly, because the QP is not yet in
ERROR state.

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...

Tom.


> 
> So, it seems ok in practice.
> 
> Thanks,
> Cheng Xu
> 
> 
> [1] http://www.rdmaconsortium.org/home/draft-hilland-iwarp-verbs-v1.0-RDMAC.pdf
> 
>