Re: [PATCH 1/5] RDMA/rxe: Change user/kernel API to allow indexing AH

From: Jason Gunthorpe <jgg@nvidia.com>
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: zyjzyj2000@gmail.com, linux-rdma@vger.kernel.org
Subject: Re: [PATCH 1/5] RDMA/rxe: Change user/kernel API to allow indexing AH
Date: Fri, 16 Jul 2021 15:03:17 -0300	[thread overview]
Message-ID: <20210716180317.GM543781@nvidia.com> (raw)
In-Reply-To: <ede35d21-2af7-92c3-7289-ba14c8a6bb5a@gmail.com>

On Fri, Jul 16, 2021 at 12:55:05PM -0500, Bob Pearson wrote:
> On 7/16/21 12:44 PM, Jason Gunthorpe wrote:
> > On Mon, Jun 28, 2021 at 05:00:40PM -0500, Bob Pearson wrote:
> >> Make changes to rdma_user_rxe.h to allow indexing AH objects, passing
> >> the index in UD send WRs to the driver and returning the index to the rxe
> >> provider. This change will allow removing handling of the AV in the user
> >> space provider. This change is backwards compatible with the current API
> >> so new or old providers and drivers can work together.
> >>
> >> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> >>  include/uapi/rdma/rdma_user_rxe.h | 14 +++++++++++++-
> >>  1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h
> >> index e283c2220aba..e544832ed073 100644
> >> +++ b/include/uapi/rdma/rdma_user_rxe.h
> >> @@ -98,6 +98,8 @@ struct rxe_send_wr {
> >>  			__u32	remote_qpn;
> >>  			__u32	remote_qkey;
> >>  			__u16	pkey_index;
> >> +			__u16	reserved;
> >> +			__u32	ah_num;
> >>  		} ud;
> >>  		struct {
> >>  			__aligned_u64	addr;
> >> @@ -148,7 +150,12 @@ struct rxe_dma_info {
> >>  
> >>  struct rxe_send_wqe {
> >>  	struct rxe_send_wr	wr;
> >> -	struct rxe_av		av;
> >> +	union {
> >> +		struct rxe_av av;
> >> +		struct {
> >> +			__u32		reserved[0];
> >> +		} ex;
> >> +	};
> > 
> > What is this for? I didn't notice a usage?
> > 
> > Jason
> > 
> 
> Nothing yet. Was just pointing out that this is where we can extend the wqe without breaking ABI.
> I came back to this issue because I started working on implementing XRC and realized that I had to find someplace to put the xrc extended header info (the srq number) and the wqe was full up. Being dense
> I didn't figure out until later that the AV is only used for UD so this space is free anyway.
> Never the less this the patch set is still useful because IMO.

I think you should move the rxe_av memory from the struct rxe_send_wqe
to the rxe_send_wr.ud struct by padding out the ud struct and placing
the av in the proper location.

Then you don't need this confusing union and the whole thing is much
clearer..

Jason