Re: [PATCH] RDMA/srpt: Filter out AGN bits

Bart Van Assche
2019-08-19T15:11:25Z

On 8/19/19 5:21 AM, Jason Gunthorpe wrote:
> On Wed, Aug 14, 2019 at 08:15:07AM -0700, Bart Van Assche wrote:
>> The ib_srpt driver derives its default service GUID from the node GUID
>> of the first encountered HCA. Since that service GUID is passed to
>> ib_cm_listen(), the AGN bits must not be set. Since the AGN bits can
>> be set in the node GUID of RoCE HCAs, filter these bits out. This
>> patch avoids that loading the ib_srpt driver fails as follows for the
>> hns driver:
>>
>> ib_srpt srpt_add_one(hns_0) failed.
>>
>> Cc: oulijun <oulijun@huawei.com>
>> Reported-by: oulijun <oulijun@huawei.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> drivers/infiniband/ulp/srpt/ib_srpt.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
>> index e25c70a56be6..114bf8d6c82b 100644
>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
>> @@ -3109,7 +3109,8 @@ static void srpt_add_one(struct ib_device *device)
>> srpt_use_srq(sdev, sdev->port[0].port_attrib.use_srq);
>>
>> if (!srpt_service_guid)
>> - srpt_service_guid = be64_to_cpu(device->node_guid);
>> + srpt_service_guid = be64_to_cpu(device->node_guid) &
>> + ~IB_SERVICE_ID_AGN_MASK;
>
> This seems kind of sketchy, masking bits in the GUID is going to make
> it non-unique.. Should we do this only for roce or something?

Hi Jason and Hal,

The I/O controller GUID can be used in the srp_daemon configuration file
for filtering purposes. The srp_daemon only supports IB networks.

In the IBTA spec I found the following about the I/O controller GUID:
"An EUI-64 GUID used to uniquely identify the controller. This could be
the same one as the Node/Port GUID if there is only one controller."

Does uniqueness of the I/O controller GUID only matter in InfiniBand
networks or does it also matter in other RDMA networks?

How about using 0 as default value for the srpt_service_guid in RoCE
networks?

Thanks,

Bart.