Re: [RFC PATCH 0/2] RDMA/rxe: Add RDMA Atomic Write operation

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Jan 06, 2022 at 01:54:30AM +0000, yangx.jy@fujitsu.com wrote:
> On 2022/1/6 8:01, Jason Gunthorpe wrote:
> > On Wed, Jan 05, 2022 at 01:00:42AM +0000, yangx.jy@fujitsu.com wrote:
> >
> >> 1) In kernel, current SoftRoCE copies the content of struct rdma to RETH
> >> and copies the content of struct atomic to AtomicETH.
> >> 2) IBTA defines that RDMA Atomic Write uses RETH + payload.
> >> According to these two reasons, I perfer to tweak the existing struct rdma.
> > No this is basically meaningless
> >
> > The wr struct is designed as a 'tagged union' where the op specified
> > which union is in effect.
> >
> > It turns out that the op generally specifies the network headers as
> > well, but that is just a side effect.
> >
> >>>> How about adding a member in struct rdma? for example:
> >>>> struct {
> >>>>        uint64_t    remote_addr;
> >>>>        uint32_t    rkey;
> >>>>        uint64_t    wr_value:
> >>>> } rdma;
> >>> Yes, that's what Tomasz and I were suggesting - a new template for the
> >>> ATOMIC_WRITE request payload. The three fields are to be supplied by
> >>> the verb consumer when posting the work request.
> >> OK, I will update the patch in this way.
> > We are not extending the ib_send_wr anymore anyhow.
> >
> > You should implement new ops inside struct ibv_qp_ex as function
> > calls.
> 
> Hi Jason,
> 
> For SoftRoCE, do you mean that I only need to extend struct rxe_send_wr 
> and add  ibv_wr_rdma_atomic_write() ?
> struct rxe_send_wr {
>      ...
>          struct {
>              __aligned_u64 remote_addr;
> +           __aligned_u64 atomic_wr;
>              __u32   rkey;
>              __u32   reserved;
>          } rdma;

You can't make a change like this to anything in
include/uapi/rdma/rdma_user_rxe.h, it has to remain compatiable.

 
> static inline void ibv_wr_rdma_atomic_write(struct ibv_qp_ex *qp, 
> uint32_t rkey,
>                                       uint64_t remote_addr)
> {
>          qp->wr_rdma_atomic_write(qp, rkey, remote_addr);
> }

Yes, something like that
 
> Besides, could you tell me why we cannot extend struct ibv_send_wr for 
> ibv_post_send()?

The ABI is not allowed to change so what is doable with it is very
limited.

Jason