[PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[Kamal Heib]

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

 drivers/infiniband/hw/cxgb4/provider.c       | 1 +
 drivers/infiniband/hw/i40iw/i40iw_verbs.c    | 1 +
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 2 +-
 drivers/infiniband/sw/siw/siw_verbs.c        | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.25.4

[PATCH for-rc v1 1/4] RDMA/siw: Set max_pkeys attribute

[Kamal Heib]

Make sure to set the max_pkeys attribute to indicate the maximum number
of partitions supported by the siw device.

Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
Cc: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_verbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 987e2ba05dbc..bef35d566aee 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -151,6 +151,7 @@ int siw_query_device(struct ib_device *base_dev, struct ib_device_attr *attr,
 	attr->max_srq = sdev->attrs.max_srq;
 	attr->max_srq_sge = sdev->attrs.max_srq_sge;
 	attr->max_srq_wr = sdev->attrs.max_srq_wr;
+	attr->max_pkeys = 1;
 	attr->page_size_cap = PAGE_SIZE;
 	attr->vendor_id = SIW_VENDOR_ID;
 	attr->vendor_part_id = sdev->vendor_part_id;
-- 
2.25.4

[PATCH for-rc v1 2/4] RDMA/cxgb4: Set max_pkeys attribute

[Kamal Heib]

Make sure to set the max_pkeys attribute to indicate the maximum number
of partitions supported by the cxgb4 device.

Fixes: cfdda9d76436 ("RDMA/cxgb4: Add driver for Chelsio T4 RNIC")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
Cc: Potnuri Bharat Teja <bharat@chelsio.com>
---
 drivers/infiniband/hw/cxgb4/provider.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index ba83d942997c..98014beac1c5 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -298,6 +298,7 @@ static int c4iw_query_device(struct ib_device *ibdev, struct ib_device_attr *pro
 	props->local_ca_ack_delay = 0;
 	props->max_fast_reg_page_list_len =
 		t4_max_fr_depth(dev->rdev.lldi.ulptx_memwrite_dsgl && use_dsgl);
+	props->max_pkeys = 1;
 
 	return 0;
 }
-- 
2.25.4

[PATCH for-rc v1 3/4] RDMA/i40iw: Set max_pkeys attribute

[Kamal Heib]

Make sure to set the max_pkeys attribute to indicate the maximum number
of partitions supported by the i40iw device.

Fixes: d37498417947 ("i40iw: add files for iwarp interface")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
Cc: Shiraz Saleem <shiraz.saleem@intel.com>
---
 drivers/infiniband/hw/i40iw/i40iw_verbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 19af29a48c55..ef624fa5f07b 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -84,6 +84,7 @@ static int i40iw_query_device(struct ib_device *ibdev,
 	props->max_qp_init_rd_atom = props->max_qp_rd_atom;
 	props->atomic_cap = IB_ATOMIC_NONE;
 	props->max_fast_reg_page_list_len = I40IW_MAX_PAGES_PER_FMR;
+	props->max_pkeys = 1;
 	return 0;
 }
 
-- 
2.25.4

[PATCH for-rc v1 4/4] RDMA/usnic: Fix reported max_pkeys attribute

[Kamal Heib]

Make sure to report the right max_pkeys attribute value to indicate the
maximum number of partitions supported by the usnic device.

Fixes: e3cf00d0a87f ("IB/usnic: Add Cisco VIC low-level hardware driver")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
Cc: Christian Benvenuti <benve@cisco.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
index b8a77ce11590..0cb2a73d46ee 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
@@ -309,7 +309,7 @@ int usnic_ib_query_device(struct ib_device *ibdev,
 	props->max_pd = USNIC_UIOM_MAX_PD_CNT;
 	props->max_mr = USNIC_UIOM_MAX_MR_CNT;
 	props->local_ca_ack_delay = 0;
-	props->max_pkeys = 0;
+	props->max_pkeys = 1;
 	props->atomic_cap = IB_ATOMIC_NONE;
 	props->masked_atomic_cap = props->atomic_cap;
 	props->max_qp_rd_atom = 0;
-- 
2.25.4

Re: [PATCH for-rc v1 1/4] RDMA/siw: Set max_pkeys attribute

[Bernard Metzler]

-----"Kamal Heib" <kamalheib1@gmail.com> wrote: -----

>To: linux-rdma@vger.kernel.org
>From: "Kamal Heib" <kamalheib1@gmail.com>
>Date: 07/06/2020 11:11AM
>Cc: "Jason Gunthorpe" <jgg@ziepe.ca>, "Doug Ledford"
><dledford@redhat.com>, "Kamal Heib" <kamalheib1@gmail.com>, "Bernard
>Metzler" <bmt@zurich.ibm.com>
>Subject: [EXTERNAL] [PATCH for-rc v1 1/4] RDMA/siw: Set max_pkeys
>attribute
>
>Make sure to set the max_pkeys attribute to indicate the maximum
>number
>of partitions supported by the siw device.
>
>Fixes: 303ae1cdfdf7 ("rdma/siw: application interface")
>Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
>Cc: Bernard Metzler <bmt@zurich.ibm.com>
>---
> drivers/infiniband/sw/siw/siw_verbs.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
>b/drivers/infiniband/sw/siw/siw_verbs.c
>index 987e2ba05dbc..bef35d566aee 100644
>--- a/drivers/infiniband/sw/siw/siw_verbs.c
>+++ b/drivers/infiniband/sw/siw/siw_verbs.c
>@@ -151,6 +151,7 @@ int siw_query_device(struct ib_device *base_dev,
>struct ib_device_attr *attr,
> 	attr->max_srq = sdev->attrs.max_srq;
> 	attr->max_srq_sge = sdev->attrs.max_srq_sge;
> 	attr->max_srq_wr = sdev->attrs.max_srq_wr;
>+	attr->max_pkeys = 1;
> 	attr->page_size_cap = PAGE_SIZE;
> 	attr->vendor_id = SIW_VENDOR_ID;
> 	attr->vendor_part_id = sdev->vendor_part_id;
>-- 
>2.25.4
>
>
Thank you!
Reviewed-by: Bernard Metzler <bmt@zurich.ibm.com>

Re: [PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[Jason Gunthorpe]

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

Re: [PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[Kamal Heib]

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

Re: [PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[Jason Gunthorpe]

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

Re: [PATCH for-rc v1 0/4] RDMA/providers: Set max_pkey attribute

[Kamal Heib]

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