On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> Hello Jason and Doug,
>
> This patch series not only adds RDMA/CM support to the SRP target driver but
> also fixes a number of race conditions in that driver.
>
> The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> module parameter. The default value for that parameter is zero which means
> that RDMA/CM support is disabled.
>
> Note: since this patch series uses the srp_login_req_rdma structure that was
> introduced by the IB/srp RDMA/CM patch series, this series depends on the
> IB/srp RDMA/CM patch series.
>
> This patch series, just like v4 of the IB/srp RDMA/CM patch series, passes
> Laurence Oberman's tests.
>
> Please consider this patch series for inclusion in the upstream kernel.
>
> Thanks,
>
> Bart.
>
> Changes compared to v1:
> - Added patch "Fix a race condition related to wait list processing".
> - Fixed the size of the character arrays used to store the initiator port ID
> and session name. This fixes a login failure that was reported by Laurence
> Oberman.
Overall, this series looks mostly good. I'm still wondering if the
configuration details need more work. In particular, it seems the host
is lacking in the fundamental controls needed for implemented server
side ACLs in regards to RDMA_CM connections. The current code assumes
it is safe to listen on the wildcard address on the target port, and I
don't think that's a safe assumption. We might use different
vlans/pkeys to segment off different namespaces, and in that case we
would want to listen only on the vlans/pkeys that correspond to allowed
namespace clients. So I think that needs correcting. The first 11
patches of this series seem standalone and can go in now at this point.
Would you agree? If so, I'll pull those in while we discuss the
configuration stuff.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDDAttachment:
signature.asc
Description: This is a digitally signed message part
