Re: [PATCH] RDMA/rxe,siw: Restore uverbs_cmd_mask IB_USER_VERBS_CMD_POST_SEND

[Bob Pearson <rpearsonhpe@gmail.com>]

On 10/30/20 9:03 AM, Jason Gunthorpe wrote:
> These two drivers open code the call to POST_SEND and do not use the
> rdma-core wrapper to do it, thus their usages was missed during the audit.
> 
> Both drivers use this as a doorbell to signal the kernel to start DMA.
> 
> Fixes: 628c02bf38aa ("RDMA: Remove uverbs cmds from drivers that don't use them")
> Reported-by: Bob Pearson <rpearsonhpe@gmail.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 3 ++-
>  drivers/infiniband/sw/siw/siw_main.c  | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 7652d53af2c1d9..209c7b3fab97a2 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1142,7 +1142,8 @@ int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name)
>  	dma_set_max_seg_size(&dev->dev, UINT_MAX);
>  	dma_set_coherent_mask(&dev->dev, dma_get_required_mask(&dev->dev));
>  
> -	dev->uverbs_cmd_mask |= BIT_ULL(IB_USER_VERBS_CMD_REQ_NOTIFY_CQ);
> +	dev->uverbs_cmd_mask |= BIT_ULL(IB_USER_VERBS_CMD_POST_SEND) |
> +				BIT_ULL(IB_USER_VERBS_CMD_REQ_NOTIFY_CQ);
>  
>  	ib_set_device_ops(dev, &rxe_dev_ops);
>  	err = ib_device_set_netdev(&rxe->ib_dev, rxe->ndev, 1);
> diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
> index e49faefdee923d..9cf596429dbf7d 100644
> --- a/drivers/infiniband/sw/siw/siw_main.c
> +++ b/drivers/infiniband/sw/siw/siw_main.c
> @@ -347,6 +347,8 @@ static struct siw_device *siw_device_create(struct net_device *netdev)
>  				    addr);
>  	}
>  
> +	base_dev->uverbs_cmd_mask |= BIT_ULL(IB_USER_VERBS_CMD_POST_SEND);
> +
>  	base_dev->node_type = RDMA_NODE_RNIC;
>  	memcpy(base_dev->node_desc, SIW_NODE_DESC_COMMON,
>  	       sizeof(SIW_NODE_DESC_COMMON));
> 
This the right short term fix but seriously why not just completely kill off uverbs_cmd_mask? If a driver sets a non-NULL value in ib_device_ops assume it was done for a reason and allow commands through. Is there any example of a driver using uverbs_cmd_mask to dynamically enable/disable a verb? I thought this was your plan when you mentioned this change a while back.

Your comment about open coding the post send command is valid. I don't recall all the history that got us to where we are now but it looks like calling ibv_cmd_post_send could be made to work. Since wr_count will be zero a lot of extra work will not happen but there is still more work than needed. The only thing I would be worried about is that it assumes the wqes are struct ib_uverbs_send_wr's but at least for rxe they are expanded. Have to make sure there aren't any bad side effects now or in the future from e.g. sending the wrong wqe size to the kernel.

Bob