On Thu, Jan 06, 2022 at 01:54:30AM +0000, yangx.jy@xxxxxxxxxxx wrote: > On 2022/1/6 8:01, Jason Gunthorpe wrote: > > On Wed, Jan 05, 2022 at 01:00:42AM +0000, yangx.jy@xxxxxxxxxxx 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