* [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR
@ 2019-01-07 18:53 Adit Ranadive
2019-01-07 18:59 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Adit Ranadive @ 2019-01-07 18:53 UTC (permalink / raw)
To: jgg@mellanox.com, dledford@redhat.com
Cc: Adit Ranadive, linux-rdma@vger.kernel.org, Pv-drivers,
stable@vger.kernel.org
From: Adit Ranadive <aditr@vmware.com>
Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
opcodes explicitly.
Reported-by: Ruishuang Wang <ruishuangw@vmware.com>
Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
Cc: stable@vger.kernel.org
Reviewed-by: Bryan Tan <bryantan@vmware.com>
Reviewed-by: Ruishuang Wang <ruishuangw@vmware.com>
Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
Signed-off-by: Adit Ranadive <aditr@vmware.com>
---
drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
index 42b8685c997e..c2ed09e66d2b 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
@@ -427,7 +427,26 @@ static inline enum ib_qp_state pvrdma_qp_state_to_ib(enum pvrdma_qp_state state)
static inline enum pvrdma_wr_opcode ib_wr_opcode_to_pvrdma(enum ib_wr_opcode op)
{
- return (enum pvrdma_wr_opcode)op;
+ switch (op) {
+ case IB_WR_LSO:
+ return PVRDMA_WR_LSO;
+ case IB_WR_SEND_WITH_INV:
+ return PVRDMA_WR_SEND_WITH_INV;
+ case IB_WR_RDMA_READ_WITH_INV:
+ return PVRDMA_WR_RDMA_READ_WITH_INV;
+ case IB_WR_LOCAL_INV:
+ return PVRDMA_WR_LOCAL_INV;
+ case IB_WR_REG_MR:
+ return PVRDMA_WR_FAST_REG_MR;
+ case IB_WR_MASKED_ATOMIC_CMP_AND_SWP:
+ return PVRDMA_WR_MASKED_ATOMIC_CMP_AND_SWP;
+ case IB_WR_MASKED_ATOMIC_FETCH_AND_ADD:
+ return PVRDMA_WR_MASKED_ATOMIC_FETCH_AND_ADD;
+ case IB_WR_REG_SIG_MR:
+ return PVRDMA_WR_REG_SIG_MR;
+ default:
+ return (enum pvrdma_wr_opcode)op;
+ }
}
static inline enum ib_wc_status pvrdma_wc_status_to_ib(
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR
2019-01-07 18:53 [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR Adit Ranadive
@ 2019-01-07 18:59 ` Jason Gunthorpe
2019-01-07 19:08 ` Adit Ranadive
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2019-01-07 18:59 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, Pv-drivers,
stable@vger.kernel.org
On Mon, Jan 07, 2019 at 06:53:24PM +0000, Adit Ranadive wrote:
> From: Adit Ranadive <aditr@vmware.com>
>
> Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
> opcodes explicitly.
>
> Reported-by: Ruishuang Wang <ruishuangw@vmware.com>
> Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
> Cc: stable@vger.kernel.org
> Reviewed-by: Bryan Tan <bryantan@vmware.com>
> Reviewed-by: Ruishuang Wang <ruishuangw@vmware.com>
> Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
> Signed-off-by: Adit Ranadive <aditr@vmware.com>
> ---
> drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
So naughty!! How many other places in this driver are assuming the
intenal IB constants are stable?
Did you audit for other cases?
> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> index 42b8685c997e..c2ed09e66d2b 100644
> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
> @@ -427,7 +427,26 @@ static inline enum ib_qp_state pvrdma_qp_state_to_ib(enum pvrdma_qp_state state)
>
> static inline enum pvrdma_wr_opcode ib_wr_opcode_to_pvrdma(enum ib_wr_opcode op)
> {
> - return (enum pvrdma_wr_opcode)op;
> + switch (op) {
> + case IB_WR_LSO:
> + return PVRDMA_WR_LSO;
> + case IB_WR_SEND_WITH_INV:
> + return PVRDMA_WR_SEND_WITH_INV;
> + case IB_WR_RDMA_READ_WITH_INV:
> + return PVRDMA_WR_RDMA_READ_WITH_INV;
> + case IB_WR_LOCAL_INV:
> + return PVRDMA_WR_LOCAL_INV;
> + case IB_WR_REG_MR:
> + return PVRDMA_WR_FAST_REG_MR;
> + case IB_WR_MASKED_ATOMIC_CMP_AND_SWP:
> + return PVRDMA_WR_MASKED_ATOMIC_CMP_AND_SWP;
> + case IB_WR_MASKED_ATOMIC_FETCH_AND_ADD:
> + return PVRDMA_WR_MASKED_ATOMIC_FETCH_AND_ADD;
> + case IB_WR_REG_SIG_MR:
> + return PVRDMA_WR_REG_SIG_MR;
> + default:
> + return (enum pvrdma_wr_opcode)op;
No default, lets write them all out please.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR
2019-01-07 18:59 ` Jason Gunthorpe
@ 2019-01-07 19:08 ` Adit Ranadive
2019-01-07 19:17 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Adit Ranadive @ 2019-01-07 19:08 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, Pv-drivers,
stable@vger.kernel.org
On 1/7/19 10:59 AM, Jason Gunthorpe wrote:
> On Mon, Jan 07, 2019 at 06:53:24PM +0000, Adit Ranadive wrote:
>> From: Adit Ranadive <aditr@vmware.com>
>>
>> Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
>> opcodes explicitly.
>>
>> Reported-by: Ruishuang Wang <ruishuangw@vmware.com>
>> Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Bryan Tan <bryantan@vmware.com>
>> Reviewed-by: Ruishuang Wang <ruishuangw@vmware.com>
>> Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
>> Signed-off-by: Adit Ranadive <aditr@vmware.com>
>> ---
>> drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> So naughty!! How many other places in this driver are assuming the
> intenal IB constants are stable?
>
> Did you audit for other cases?
Lots of places. Though its the values shared with userspace, like QP types,
IB MTUs, etc. Unless you foresee changes to those ..
>
>> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> index 42b8685c997e..c2ed09e66d2b 100644
>> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> @@ -427,7 +427,26 @@ static inline enum ib_qp_state pvrdma_qp_state_to_ib(enum pvrdma_qp_state state)
>>
>> static inline enum pvrdma_wr_opcode ib_wr_opcode_to_pvrdma(enum ib_wr_opcode op)
>> {
>> - return (enum pvrdma_wr_opcode)op;
>> + switch (op) {
>> + case IB_WR_LSO:
>> + return PVRDMA_WR_LSO;
>> + case IB_WR_SEND_WITH_INV:
>> + return PVRDMA_WR_SEND_WITH_INV;
>> + case IB_WR_RDMA_READ_WITH_INV:
>> + return PVRDMA_WR_RDMA_READ_WITH_INV;
>> + case IB_WR_LOCAL_INV:
>> + return PVRDMA_WR_LOCAL_INV;
>> + case IB_WR_REG_MR:
>> + return PVRDMA_WR_FAST_REG_MR;
>> + case IB_WR_MASKED_ATOMIC_CMP_AND_SWP:
>> + return PVRDMA_WR_MASKED_ATOMIC_CMP_AND_SWP;
>> + case IB_WR_MASKED_ATOMIC_FETCH_AND_ADD:
>> + return PVRDMA_WR_MASKED_ATOMIC_FETCH_AND_ADD;
>> + case IB_WR_REG_SIG_MR:
>> + return PVRDMA_WR_REG_SIG_MR;
>> + default:
>> + return (enum pvrdma_wr_opcode)op;
>
> No default, lets write them all out please.
Fair enough.
>
> Jason
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR
2019-01-07 19:08 ` Adit Ranadive
@ 2019-01-07 19:17 ` Jason Gunthorpe
2019-01-07 20:05 ` Adit Ranadive
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2019-01-07 19:17 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, Pv-drivers,
stable@vger.kernel.org
On Mon, Jan 07, 2019 at 07:08:36PM +0000, Adit Ranadive wrote:
> On 1/7/19 10:59 AM, Jason Gunthorpe wrote:
> > On Mon, Jan 07, 2019 at 06:53:24PM +0000, Adit Ranadive wrote:
> >> From: Adit Ranadive <aditr@vmware.com>
> >>
> >> Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
> >> opcodes explicitly.
> >>
> >> Reported-by: Ruishuang Wang <ruishuangw@vmware.com>
> >> Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
> >> Cc: stable@vger.kernel.org
> >> Reviewed-by: Bryan Tan <bryantan@vmware.com>
> >> Reviewed-by: Ruishuang Wang <ruishuangw@vmware.com>
> >> Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
> >> Signed-off-by: Adit Ranadive <aditr@vmware.com>
> >> drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
> >> 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > So naughty!! How many other places in this driver are assuming the
> > intenal IB constants are stable?
> >
> > Did you audit for other cases?
>
> Lots of places. Though its the values shared with userspace, like QP types,
> IB MTUs, etc. Unless you foresee changes to those ..
Constants declared in the include/uapi header should be OK, but you
still need to check that the constants you are using are actually in
that header and either recode them or move them into uapi, as
appropriate.
When constants get moved we may find more mistakes like this that
cause the kernel view to change.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR
2019-01-07 19:17 ` Jason Gunthorpe
@ 2019-01-07 20:05 ` Adit Ranadive
0 siblings, 0 replies; 5+ messages in thread
From: Adit Ranadive @ 2019-01-07 20:05 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, Pv-drivers,
stable@vger.kernel.org
On 1/7/19 11:17 AM, Jason Gunthorpe wrote:
> On Mon, Jan 07, 2019 at 07:08:36PM +0000, Adit Ranadive wrote:
>> On 1/7/19 10:59 AM, Jason Gunthorpe wrote:
>>> On Mon, Jan 07, 2019 at 06:53:24PM +0000, Adit Ranadive wrote:
>>>> From: Adit Ranadive <aditr@vmware.com>
>>>>
>>>> Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
>>>> opcodes explicitly.
>>>>
>>>> Reported-by: Ruishuang Wang <ruishuangw@vmware.com>
>>>> Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
>>>> Cc: stable@vger.kernel.org
>>>> Reviewed-by: Bryan Tan <bryantan@vmware.com>
>>>> Reviewed-by: Ruishuang Wang <ruishuangw@vmware.com>
>>>> Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
>>>> Signed-off-by: Adit Ranadive <aditr@vmware.com>
>>>> drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
>>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> So naughty!! How many other places in this driver are assuming the
>>> intenal IB constants are stable?
>>>
>>> Did you audit for other cases?
>>
>> Lots of places. Though its the values shared with userspace, like QP types,
>> IB MTUs, etc. Unless you foresee changes to those ..
>
> Constants declared in the include/uapi header should be OK, but you
> still need to check that the constants you are using are actually in
> that header and either recode them or move them into uapi, as
> appropriate.
>
> When constants get moved we may find more mistakes like this that
> cause the kernel view to change.
>
> Jason
>
Thanks. Yeah, I was afraid we might have to do something like that.
Okay, I'll send a v1 for this patch and more updates later for 5.1
inclusion if possible.
Thanks,
Adit
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-07 20:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-07 18:53 [PATCH for-rc] RDMA/vmw_pvrdma: Return the correct opcode when creating WR Adit Ranadive
2019-01-07 18:59 ` Jason Gunthorpe
2019-01-07 19:08 ` Adit Ranadive
2019-01-07 19:17 ` Jason Gunthorpe
2019-01-07 20:05 ` Adit Ranadive
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox