[PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Kamal Heib]

Now that the query_pkey() isn't mandatory by the RDMA core, this
callback can be removed from the usnic provider.

Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_main.c  |  2 --
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 11 -----------
 drivers/infiniband/hw/usnic/usnic_ib_verbs.h |  2 --
 3 files changed, 15 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
index c9abe1c01e4e..515460c4212d 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
@@ -315,7 +315,6 @@ static int usnic_port_immutable(struct ib_device *ibdev, u8 port_num,
 	if (err)
 		return err;
 
-	immutable->pkey_tbl_len = attr.pkey_tbl_len;
 	immutable->gid_tbl_len = attr.gid_tbl_len;
 
 	return 0;
@@ -355,7 +354,6 @@ static const struct ib_device_ops usnic_dev_ops = {
 	.modify_qp = usnic_ib_modify_qp,
 	.query_device = usnic_ib_query_device,
 	.query_gid = usnic_ib_query_gid,
-	.query_pkey = usnic_ib_query_pkey,
 	.query_port = usnic_ib_query_port,
 	.query_qp = usnic_ib_query_qp,
 	.reg_user_mr = usnic_ib_reg_mr,
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
index b8a77ce11590..02a49f661c8d 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
@@ -367,7 +367,6 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
 
 	props->port_cap_flags = 0;
 	props->gid_tbl_len = 1;
-	props->pkey_tbl_len = 1;
 	props->bad_pkey_cntr = 0;
 	props->qkey_viol_cntr = 0;
 	props->max_mtu = IB_MTU_4096;
@@ -437,16 +436,6 @@ int usnic_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
 	return 0;
 }
 
-int usnic_ib_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
-				u16 *pkey)
-{
-	if (index > 0)
-		return -EINVAL;
-
-	*pkey = 0xffff;
-	return 0;
-}
-
 int usnic_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
 {
 	struct usnic_ib_pd *pd = to_upd(ibpd);
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.h b/drivers/infiniband/hw/usnic/usnic_ib_verbs.h
index 2aedf78c13cf..9195f2b901ce 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.h
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.h
@@ -48,8 +48,6 @@ int usnic_ib_query_qp(struct ib_qp *qp, struct ib_qp_attr *qp_attr,
 				struct ib_qp_init_attr *qp_init_attr);
 int usnic_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
 				union ib_gid *gid);
-int usnic_ib_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
-				u16 *pkey);
 int usnic_ib_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata);
 void usnic_ib_dealloc_pd(struct ib_pd *pd, struct ib_udata *udata);
 struct ib_qp *usnic_ib_create_qp(struct ib_pd *pd,
-- 
2.26.2

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Gal Pressman]

On 20/08/2020 15:53, Kamal Heib wrote:
> Now that the query_pkey() isn't mandatory by the RDMA core, this
> callback can be removed from the usnic provider.

Not directly related to this patch, but pyverbs has a test which verifies that
max_pkeys > 0, maybe this check should be removed.

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Kamal Heib]

On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
> On 20/08/2020 15:53, Kamal Heib wrote:
> > Now that the query_pkey() isn't mandatory by the RDMA core, this
> > callback can be removed from the usnic provider.
> 
> Not directly related to this patch, but pyverbs has a test which verifies that
> max_pkeys > 0, maybe this check should be removed.

Or changed to work only for node_type == e.IBV_NODE_CA?

Thanks,
Kamal

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Kamal Heib]

On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
> > On 20/08/2020 15:53, Kamal Heib wrote:
> > > Now that the query_pkey() isn't mandatory by the RDMA core, this
> > > callback can be removed from the usnic provider.
> > 
> > Not directly related to this patch, but pyverbs has a test which verifies that
> > max_pkeys > 0, maybe this check should be removed.
> 
> Or changed to work only for node_type == e.IBV_NODE_CA?
> 
> Thanks,
> Kamal

BTW, do the efa care about pkey?

Thanks,
Kamal

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Gal Pressman]

On 27/08/2020 10:53, Kamal Heib wrote:
> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
>>> On 20/08/2020 15:53, Kamal Heib wrote:
>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
>>>> callback can be removed from the usnic provider.
>>>
>>> Not directly related to this patch, but pyverbs has a test which verifies that
>>> max_pkeys > 0, maybe this check should be removed.
>>
>> Or changed to work only for node_type == e.IBV_NODE_CA?
>>
>> Thanks,
>> Kamal
> 
> BTW, do the efa care about pkey?

Depends.. We only support the default pkey so it doesn't do much in terms of
functionality, but we still need to support it as part of the QP state machine
for modify QP.

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Jason Gunthorpe]

On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
> On 27/08/2020 10:53, Kamal Heib wrote:
> > On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
> >> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
> >>> On 20/08/2020 15:53, Kamal Heib wrote:
> >>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
> >>>> callback can be removed from the usnic provider.
> >>>
> >>> Not directly related to this patch, but pyverbs has a test which verifies that
> >>> max_pkeys > 0, maybe this check should be removed.
> >>
> >> Or changed to work only for node_type == e.IBV_NODE_CA?
> >>
> >> Thanks,
> >> Kamal
> > 
> > BTW, do the efa care about pkey?
> 
> Depends.. We only support the default pkey so it doesn't do much in terms of
> functionality, but we still need to support it as part of the QP state machine
> for modify QP.

Does the pkey appear on the wire, or is it just some cruft for API sake?

Jason

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Håkon Bugge]

> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
>> On 27/08/2020 10:53, Kamal Heib wrote:
>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
>>>>>> callback can be removed from the usnic provider.
>>>>> 
>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
>>>>> max_pkeys > 0, maybe this check should be removed.
>>>> 
>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
>>>> 
>>>> Thanks,
>>>> Kamal
>>> 
>>> BTW, do the efa care about pkey?
>> 
>> Depends.. We only support the default pkey so it doesn't do much in terms of
>> functionality, but we still need to support it as part of the QP state machine
>> for modify QP.
> 
> Does the pkey appear on the wire, or is it just some cruft for API sake?

On the wire. Included in the BTH (Base Transfer Header).

Håkon


> 
> Jason

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Gal Pressman]

On 27/08/2020 15:20, Håkon Bugge wrote:
>> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
>>> On 27/08/2020 10:53, Kamal Heib wrote:
>>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
>>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
>>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
>>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
>>>>>>> callback can be removed from the usnic provider.
>>>>>>
>>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
>>>>>> max_pkeys > 0, maybe this check should be removed.
>>>>>
>>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
>>>>>
>>>>> Thanks,
>>>>> Kamal
>>>>
>>>> BTW, do the efa care about pkey?
>>>
>>> Depends.. We only support the default pkey so it doesn't do much in terms of
>>> functionality, but we still need to support it as part of the QP state machine
>>> for modify QP.
>>
>> Does the pkey appear on the wire, or is it just some cruft for API sake?
> 
> On the wire. Included in the BTH (Base Transfer Header).

He was probably asking specifically about EFA.
I can't share any details about the wire protocol, does it matter?

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Jason Gunthorpe]

On Thu, Aug 27, 2020 at 03:58:23PM +0300, Gal Pressman wrote:
> On 27/08/2020 15:20, Håkon Bugge wrote:
> >> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
> >>> On 27/08/2020 10:53, Kamal Heib wrote:
> >>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
> >>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
> >>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
> >>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
> >>>>>>> callback can be removed from the usnic provider.
> >>>>>>
> >>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
> >>>>>> max_pkeys > 0, maybe this check should be removed.
> >>>>>
> >>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
> >>>>>
> >>>>> Thanks,
> >>>>> Kamal
> >>>>
> >>>> BTW, do the efa care about pkey?
> >>>
> >>> Depends.. We only support the default pkey so it doesn't do much in terms of
> >>> functionality, but we still need to support it as part of the QP state machine
> >>> for modify QP.
> >>
> >> Does the pkey appear on the wire, or is it just some cruft for API sake?
> > 
> > On the wire. Included in the BTH (Base Transfer Header).
> 
> He was probably asking specifically about EFA.
> I can't share any details about the wire protocol, does it matter?

If it isn't actually used for anything then the driver shouldn't
expose PKEY at all, if you do use it then leave it.

Jason

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Gal Pressman]

On 27/08/2020 16:01, Jason Gunthorpe wrote:
> On Thu, Aug 27, 2020 at 03:58:23PM +0300, Gal Pressman wrote:
>> On 27/08/2020 15:20, Håkon Bugge wrote:
>>>> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>>>> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
>>>>> On 27/08/2020 10:53, Kamal Heib wrote:
>>>>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
>>>>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
>>>>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
>>>>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
>>>>>>>>> callback can be removed from the usnic provider.
>>>>>>>>
>>>>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
>>>>>>>> max_pkeys > 0, maybe this check should be removed.
>>>>>>>
>>>>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Kamal
>>>>>>
>>>>>> BTW, do the efa care about pkey?
>>>>>
>>>>> Depends.. We only support the default pkey so it doesn't do much in terms of
>>>>> functionality, but we still need to support it as part of the QP state machine
>>>>> for modify QP.
>>>>
>>>> Does the pkey appear on the wire, or is it just some cruft for API sake?
>>>
>>> On the wire. Included in the BTH (Base Transfer Header).
>>
>> He was probably asking specifically about EFA.
>> I can't share any details about the wire protocol, does it matter?
> 
> If it isn't actually used for anything then the driver shouldn't
> expose PKEY at all, if you do use it then leave it.

How would that work? How can you not expose pkeys and still have ibv_ud_pingpong
work?
You mean remove query_pkey, but still support IBV_QP_PKEY_INDEX in modify_qp?

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Jason Gunthorpe]

On Thu, Aug 27, 2020 at 04:10:27PM +0300, Gal Pressman wrote:
> On 27/08/2020 16:01, Jason Gunthorpe wrote:
> > On Thu, Aug 27, 2020 at 03:58:23PM +0300, Gal Pressman wrote:
> >> On 27/08/2020 15:20, Håkon Bugge wrote:
> >>>> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >>>> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
> >>>>> On 27/08/2020 10:53, Kamal Heib wrote:
> >>>>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
> >>>>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
> >>>>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
> >>>>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
> >>>>>>>>> callback can be removed from the usnic provider.
> >>>>>>>>
> >>>>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
> >>>>>>>> max_pkeys > 0, maybe this check should be removed.
> >>>>>>>
> >>>>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Kamal
> >>>>>>
> >>>>>> BTW, do the efa care about pkey?
> >>>>>
> >>>>> Depends.. We only support the default pkey so it doesn't do much in terms of
> >>>>> functionality, but we still need to support it as part of the QP state machine
> >>>>> for modify QP.
> >>>>
> >>>> Does the pkey appear on the wire, or is it just some cruft for API sake?
> >>>
> >>> On the wire. Included in the BTH (Base Transfer Header).
> >>
> >> He was probably asking specifically about EFA.
> >> I can't share any details about the wire protocol, does it matter?
> > 
> > If it isn't actually used for anything then the driver shouldn't
> > expose PKEY at all, if you do use it then leave it.
> 
> How would that work? How can you not expose pkeys and still have ibv_ud_pingpong
> work?
> You mean remove query_pkey, but still support IBV_QP_PKEY_INDEX in modify_qp?

iWarp doesn't use PKEY at all, so you'd have to follow their
model. Some of the userspace that assumes IB would probably need
fixing (eg ud pingpong does not support iwarp)

Jason

Re: [PATCH for-next] RDMA/usnic: Remove the query_pkey callback

[Gal Pressman]

On 27/08/2020 16:55, Jason Gunthorpe wrote:
> On Thu, Aug 27, 2020 at 04:10:27PM +0300, Gal Pressman wrote:
>> On 27/08/2020 16:01, Jason Gunthorpe wrote:
>>> On Thu, Aug 27, 2020 at 03:58:23PM +0300, Gal Pressman wrote:
>>>> On 27/08/2020 15:20, Håkon Bugge wrote:
>>>>>> On 27 Aug 2020, at 14:01, Jason Gunthorpe <jgg@ziepe.ca> wrote:
>>>>>> On Thu, Aug 27, 2020 at 11:20:16AM +0300, Gal Pressman wrote:
>>>>>>> On 27/08/2020 10:53, Kamal Heib wrote:
>>>>>>>> On Thu, Aug 20, 2020 at 04:53:38PM +0300, Kamal Heib wrote:
>>>>>>>>> On Thu, Aug 20, 2020 at 04:11:23PM +0300, Gal Pressman wrote:
>>>>>>>>>> On 20/08/2020 15:53, Kamal Heib wrote:
>>>>>>>>>>> Now that the query_pkey() isn't mandatory by the RDMA core, this
>>>>>>>>>>> callback can be removed from the usnic provider.
>>>>>>>>>>
>>>>>>>>>> Not directly related to this patch, but pyverbs has a test which verifies that
>>>>>>>>>> max_pkeys > 0, maybe this check should be removed.
>>>>>>>>>
>>>>>>>>> Or changed to work only for node_type == e.IBV_NODE_CA?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Kamal
>>>>>>>>
>>>>>>>> BTW, do the efa care about pkey?
>>>>>>>
>>>>>>> Depends.. We only support the default pkey so it doesn't do much in terms of
>>>>>>> functionality, but we still need to support it as part of the QP state machine
>>>>>>> for modify QP.
>>>>>>
>>>>>> Does the pkey appear on the wire, or is it just some cruft for API sake?
>>>>>
>>>>> On the wire. Included in the BTH (Base Transfer Header).
>>>>
>>>> He was probably asking specifically about EFA.
>>>> I can't share any details about the wire protocol, does it matter?
>>>
>>> If it isn't actually used for anything then the driver shouldn't
>>> expose PKEY at all, if you do use it then leave it.
>>
>> How would that work? How can you not expose pkeys and still have ibv_ud_pingpong
>> work?
>> You mean remove query_pkey, but still support IBV_QP_PKEY_INDEX in modify_qp?
> 
> iWarp doesn't use PKEY at all, so you'd have to follow their
> model. Some of the userspace that assumes IB would probably need
> fixing (eg ud pingpong does not support iwarp)

Thanks, I'd rather keep supporting the default pkey only.