On 2022/1/4 23:17, Tom Talpey wrote:
>
> On 1/4/2022 4:28 AM, yangx.jy@xxxxxxxxxxx wrote:
>> On 2021/12/31 14:30, yangx.jy@xxxxxxxxxxx wrote:
>>> On 2021/12/31 5:42, Tom Talpey wrote:
>>>> On 12/30/2021 2:21 PM, Gromadzki, Tomasz wrote:
>>>>> 1)
>>>>>> rdma_post_atomic_writev(struct rdma_cm_id *id, void *context, struct
>>>>>> ibv_sge *sgl,
>>>>>> int nsge, int flags, uint64_t remote_addr, uint32_t
>>>>>> rkey)
>>>>> Do we need this API at all?
>>>>> Other atomic operations (compare_swap/add) do not use struct ibv_sge
>>>>> at all but have a dedicated place in
>>>>> struct ib_send_wr {
>>>>> ...
>>>>> struct {
>>>>> uint64_t remote_addr;
>>>>> uint64_t compare_add;
>>>>> uint64_t swap;
>>>>> uint32_t rkey;
>>>>> } atomic;
>>>>> ...
>>>>> }
>>>>>
>>>>> Would it be better to reuse (extend) any existing field?
>>>>> i.e.
>>>>> struct {
>>>>> uint64_t remote_addr;
>>>>> uint64_t compare_add;
>>>>> uint64_t swap_write;
>>>>> uint32_t rkey;
>>>>> } atomic;
>>>> Agreed. Passing the data to be written as an SGE is unnatural
>>>> since it is always exactly 64 bits. Tweaking the existing atomic
>>>> parameter block as Tomasz suggests is the best approach.
>>> Hi Tomasz, Tom
>>>
>>> Thanks for your quick reply.
>>>
>>> If we want to pass the 8-byte value by tweaking struct atomic on user
>>> space, why don't we
>>> tranfer the 8-byte value by ATOMIC Extended Transport Header
>>> (AtomicETH)
>>> on kernel space?
>>> PS: IBTA defines that the 8-byte value is tranfered by RDMA Extended
>>> Transport Heade(RETH) + payload.
>>>
>>> Is it inconsistent to use struct atomic on user space and RETH +
>>> payload
>>> on kernel space?
>> Hi Tomasz, Tom
>>
>> I think the following rules are right:
>> RDMA READ/WRITE should use struct rdma in libverbs and RETH + payload in
>> kernel.
>> RDMA Atomic should use struct atomic in libverbs and AtomicETH in
>> kernel.
>>
>> According to IBTA's definition, RDMA Atomic Write should use struct rdma
>> in libibverbs.
>
> I don't quite understand this statement, the IBTA defines the protocol
> but does not define the API at such a level.
Hi Tom,
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.
>
> I do however agree with this:
>
>> 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.
Best Regards,
Xiao Yang
>
> Tom.
