| Message ID | 20200706091119.367697-1-kamalheib1@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA/providers: Set max_pkey attribute | expand |
On Mon, Jul 06, 2020 at 12:11:15PM +0300, Kamal Heib wrote: > This patch set makes sure to set the max_pkeys attribute to the providers > that aren't setting it or not setting it correctly. > > v1: Drop the efa patch and target for-rc. > > Kamal Heib (4): > RDMA/siw: Set max_pkeys attribute > RDMA/cxgb4: Set max_pkeys attribute > RDMA/i40iw: Set max_pkeys attribute > RDMA/usnic: Fix reported max_pkeys attribute Why should iwarp have a 1 pkey value? Jason
On Tue, Jul 07, 2020 at 01:12:47PM -0300, Jason Gunthorpe wrote: > On Mon, Jul 06, 2020 at 12:11:15PM +0300, Kamal Heib wrote: > > This patch set makes sure to set the max_pkeys attribute to the providers > > that aren't setting it or not setting it correctly. > > > > v1: Drop the efa patch and target for-rc. > > > > Kamal Heib (4): > > RDMA/siw: Set max_pkeys attribute > > RDMA/cxgb4: Set max_pkeys attribute > > RDMA/i40iw: Set max_pkeys attribute > > RDMA/usnic: Fix reported max_pkeys attribute > > Why should iwarp have a 1 pkey value? > > Jason That is a good question :-) My Idea in this patchset was to match between the reported pkey_tbl_len and the max_pkeys attribute that the providers expose. But after taking a deeper look now, I see that the RDMA core requires from all providers to implement the query_pkey() callback, which before [1] commit that will cause the provider driver not to load. For IB providers the requirement make sense, also for RoCE providers, because there is a requirement by the RoCE Spec to support the default PKey, For iwarp providers, This doesn't make sense and I think that they decided to do the same as RoCE and to avoid the driver load failure. Probably, The requirement from the RDMA core needs to be changed and the query_pkey() callback needs to be removed from the iwarp providers, Thoughts? ------>8------ git grep -n IB_MANDATORY_FUNC drivers/infiniband/ | grep pkey drivers/infiniband/core/device.c:275: IB_MANDATORY_FUNC(query_pkey), ------>8------ [1] - 6780c4fa9d6e ("RDMA: Add indication for in kernel API support to IB device") Thanks, Kamal
On Tue, Jul 07, 2020 at 10:13:24PM +0300, Kamal Heib wrote: > On Tue, Jul 07, 2020 at 01:12:47PM -0300, Jason Gunthorpe wrote: > > On Mon, Jul 06, 2020 at 12:11:15PM +0300, Kamal Heib wrote: > > > This patch set makes sure to set the max_pkeys attribute to the providers > > > that aren't setting it or not setting it correctly. > > > > > > v1: Drop the efa patch and target for-rc. > > > > > > Kamal Heib (4): > > > RDMA/siw: Set max_pkeys attribute > > > RDMA/cxgb4: Set max_pkeys attribute > > > RDMA/i40iw: Set max_pkeys attribute > > > RDMA/usnic: Fix reported max_pkeys attribute > > > > Why should iwarp have a 1 pkey value? > > > > Jason > > That is a good question :-) > > My Idea in this patchset was to match between the reported pkey_tbl_len and > the max_pkeys attribute that the providers expose. > > But after taking a deeper look now, I see that the RDMA core requires > from all providers to implement the query_pkey() callback, which before > [1] commit that will cause the provider driver not to load. For IB > providers the requirement make sense, also for RoCE providers, because > there is a requirement by the RoCE Spec to support the default PKey, For > iwarp providers, This doesn't make sense and I think that they decided to > do the same as RoCE and to avoid the driver load failure. > > Probably, The requirement from the RDMA core needs to be changed and > the query_pkey() callback needs to be removed from the iwarp providers, > Thoughts? Sure But then the pkey table size is 0 right? Jason
On Wed, Jul 08, 2020 at 08:28:15PM -0300, Jason Gunthorpe wrote: > On Tue, Jul 07, 2020 at 10:13:24PM +0300, Kamal Heib wrote: > > On Tue, Jul 07, 2020 at 01:12:47PM -0300, Jason Gunthorpe wrote: > > > On Mon, Jul 06, 2020 at 12:11:15PM +0300, Kamal Heib wrote: > > > > This patch set makes sure to set the max_pkeys attribute to the providers > > > > that aren't setting it or not setting it correctly. > > > > > > > > v1: Drop the efa patch and target for-rc. > > > > > > > > Kamal Heib (4): > > > > RDMA/siw: Set max_pkeys attribute > > > > RDMA/cxgb4: Set max_pkeys attribute > > > > RDMA/i40iw: Set max_pkeys attribute > > > > RDMA/usnic: Fix reported max_pkeys attribute > > > > > > Why should iwarp have a 1 pkey value? > > > > > > Jason > > > > That is a good question :-) > > > > My Idea in this patchset was to match between the reported pkey_tbl_len and > > the max_pkeys attribute that the providers expose. > > > > But after taking a deeper look now, I see that the RDMA core requires > > from all providers to implement the query_pkey() callback, which before > > [1] commit that will cause the provider driver not to load. For IB > > providers the requirement make sense, also for RoCE providers, because > > there is a requirement by the RoCE Spec to support the default PKey, For > > iwarp providers, This doesn't make sense and I think that they decided to > > do the same as RoCE and to avoid the driver load failure. > > > > Probably, The requirement from the RDMA core needs to be changed and > > the query_pkey() callback needs to be removed from the iwarp providers, > > Thoughts? > > Sure > > But then the pkey table size is 0 right? > Correct, also there are required changes in the RDMA core: 1- Avoid exposing the pkeys sysfs entries for the iwarp providers. 2- Avoid allocating the pkey cache for the iwarp providers. I've started this work and I hope to post patches in the upcoming days. Thanks, Kamal > Jason