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

["yangx.jy@fujitsu.com" <yangx.jy@fujitsu.com>]

On 2022/1/10 23:42, Jason Gunthorpe wrote:
> 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.
Hi Jason,

How about adding atomic_wr member at the end of struct rdma? like this:
------------------------------------------------------
struct rxe_send_wr {
     ...
         struct {
             __aligned_u64 remote_addr;
             __u32   rkey;
             __u32   reserved;
+          __aligned_u64 atomic_wr;
         } rdma;
     ...
}
------------------------------------------------------
If it is also wrong, how to extend struct rdma?
>
>> 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.
Thanks for your explanation.

Best Regards,
Xiao Yang
> Jason