[ofa-general] Re: [PATCH] [RFC] IB/cache: Add ib_cache report for cache in process

[Or Gerlitz]

Roland Dreier wrote:
> It is entirely possible that this change makes the P_Key lookup return
> -ESTALE when it would have returned perfectly correct information (eg
> if a P_Key is being added to the end of the table without affecting
> existing P_Keys).  So this change as it stands introduces a window
> where spurious failures might occur.

Michael S. Tsirkin wrote:
> Anyway, aren't you marking all cache "stale" while most pkeys might be still valid?
> Can't this break valid usage in e.g. SRP?

Roland, Michael,

Please note that there is quite a big difference between UD vs RC based 
IB ULPs with respect to how there are influenced from using a wrong pkey 
index at their QP.

In the RC case, the receiving side transport level would not get any 
packets and hence would not send acks etc, at some point the sending 
side would get completion with error and retry the connection.

In the UD case, nothing other then pkey-violation-counter/traps etc 
would happen unless both side would re-initiate their QP (this is 
exactly what Moni is doing at ipoib in the patch that followed).

Hence, it is extremely important that UD based ULPs would react to the 
async event of pkey change, and would retry reading the pkey from the 
cache when getting ESTALE or any other error code from the cache.

For the RC case, note that a) the connection would not break if the 
change did not involve the index of the pkey used for it b) once the 
connection breaks and re-initiated by the ULP the cache would be very 
much --already updated--.

So the only case which might be problematic with a patch that does not 
change the RC ULPs (and CM) code is when in the exact millisecond you 
set your RC connection the cache changes. I don't think the IB portion 
of the ULP code has to be changed other then sensing the ESTALE error 
and propagating it up. Higher layers would retry the connection and we 
are done.

Anyway, thanks for bringing all this up! while thinking on it i have 
realized that the RDMA CM can (should) be enhanced to register on async 
events and for the pkey change event issue disconnect event on the 
relevant UD unicast IDs and multicast error event on the relevant UD 
multicast IDs.

Or.