| Message ID | 20221015063648.52285-1-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA/rxe: Add atomic write operation | expand |
Hi Jason, Bob Thanks a lot for your review. I have updated the patch set according to your all comments except the missing kmap issue. I didn't understand why current iova_to_vaddr() has been broken so I hope we can discuss the issue fully and then find a suitable solution. Best Regards, Xiao Yang On 2022/10/15 14:37, Yang, Xiao/杨 晓 wrote: > The IB SPEC v1.5[1] defined new atomic write operation. This patchset > makes SoftRoCE support new atomic write on RC service. > > On my rdma-core repository[2], I have introduced atomic write API > for libibverbs and Pyverbs. I also have provided a rdma_atomic_write > example and test_qp_ex_rc_atomic_write python test to verify > the patchset. > > The steps to run the rdma_atomic_write example: > server: > $ ./rdma_atomic_write_server -s [server_address] -p [port_number] > client: > $ ./rdma_atomic_write_client -s [server_address] -p [port_number] > > The steps to run test_qp_ex_rc_atomic_write test: > run_tests.py --dev rxe_enp0s3 --gid 1 -v test_qpex.QpExTestCase.test_qp_ex_rc_atomic_write > test_qp_ex_rc_atomic_write (tests.test_qpex.QpExTestCase) ... ok > > ---------------------------------------------------------------------- > Ran 1 test in 0.008s > > OK > > [1]: https://www.infinibandta.org/wp-content/uploads/2021/08/IBTA-Overview-of-IBTA-Volume-1-Release-1.5-and-MPE-2021-08-17-Secure.pptx > [2]: https://github.com/yangx-jy/rdma-core/tree/new_api_with_point > > v5->v6: > 1) Rebase on current for-next > 2) Split the implementation of atomic write into 7 patches > 3) Replace all "RDMA Atomic Write" with "atomic write" > 4) Save 8-byte value in struct rxe_dma_info instead > 5) Remove the print in atomic_write_reply() > > v4->v5: > 1) Rebase on current wip/jgg-for-next > 2) Rewrite the implementation on responder > > v3->v4: > 1) Rebase on current wip/jgg-for-next > 2) Fix a compiler error on 32-bit arch (e.g. parisc) by disabling RDMA Atomic Write > 3) Replace 64-bit value with 8-byte array for atomic write > > V2->V3: > 1) Rebase > 2) Add RDMA Atomic Write attribute for rxe device > > V1->V2: > 1) Set IB_OPCODE_RDMA_ATOMIC_WRITE to 0x1D > 2) Add rdma.atomic_wr in struct rxe_send_wr and use it to pass the atomic write value > 3) Use smp_store_release() to ensure that all prior operations have completed > > Xiao Yang (8): > RDMA: Extend RDMA user ABI to support atomic write > RDMA: Extend RDMA kernel ABI to support atomic write > RDMA/rxe: Extend rxe user ABI to support atomic write > RDMA/rxe: Extend rxe packet format to support atomic write > RDMA/rxe: Make requester support atomic write on RC service > RDMA/rxe: Make responder support atomic write on RC service > RDMA/rxe: Implement atomic write completion > RDMA/rxe: Enable atomic write capability for rxe device > > drivers/infiniband/sw/rxe/rxe_comp.c | 4 ++ > drivers/infiniband/sw/rxe/rxe_opcode.c | 18 ++++++ > drivers/infiniband/sw/rxe/rxe_opcode.h | 3 + > drivers/infiniband/sw/rxe/rxe_param.h | 5 ++ > drivers/infiniband/sw/rxe/rxe_req.c | 15 ++++- > drivers/infiniband/sw/rxe/rxe_resp.c | 84 ++++++++++++++++++++++++-- > include/rdma/ib_pack.h | 2 + > include/rdma/ib_verbs.h | 3 + > include/uapi/rdma/ib_user_verbs.h | 4 ++ > include/uapi/rdma/rdma_user_rxe.h | 1 + > 10 files changed, 132 insertions(+), 7 deletions(-) >
On Sat, Oct 15, 2022 at 06:37:03AM +0000, yangx.jy@fujitsu.com wrote: > The IB SPEC v1.5[1] defined new atomic write operation. This patchset > makes SoftRoCE support new atomic write on RC service. > > On my rdma-core repository[2], I have introduced atomic write API > for libibverbs and Pyverbs. I also have provided a rdma_atomic_write > example and test_qp_ex_rc_atomic_write python test to verify > the patchset. > > The steps to run the rdma_atomic_write example: > server: > $ ./rdma_atomic_write_server -s [server_address] -p [port_number] > client: > $ ./rdma_atomic_write_client -s [server_address] -p [port_number] > > The steps to run test_qp_ex_rc_atomic_write test: > run_tests.py --dev rxe_enp0s3 --gid 1 -v test_qpex.QpExTestCase.test_qp_ex_rc_atomic_write > test_qp_ex_rc_atomic_write (tests.test_qpex.QpExTestCase) ... ok > > ---------------------------------------------------------------------- > Ran 1 test in 0.008s > > OK > > [1]: https://www.infinibandta.org/wp-content/uploads/2021/08/IBTA-Overview-of-IBTA-Volume-1-Release-1.5-and-MPE-2021-08-17-Secure.pptx > [2]: https://github.com/yangx-jy/rdma-core/tree/new_api_with_point > > v5->v6: > 1) Rebase on current for-next > 2) Split the implementation of atomic write into 7 patches > 3) Replace all "RDMA Atomic Write" with "atomic write" > 4) Save 8-byte value in struct rxe_dma_info instead > 5) Remove the print in atomic_write_reply() I think this looked OK, please fix the enum thing and also resolve all the remarks on the github and rebase/repost/retest both series. Thanks, Jason
On 2022/11/23 3:54, Jason Gunthorpe wrote: > On Sat, Oct 15, 2022 at 06:37:03AM +0000, yangx.jy@fujitsu.com wrote: >> The IB SPEC v1.5[1] defined new atomic write operation. This patchset >> makes SoftRoCE support new atomic write on RC service. >> >> On my rdma-core repository[2], I have introduced atomic write API >> for libibverbs and Pyverbs. I also have provided a rdma_atomic_write >> example and test_qp_ex_rc_atomic_write python test to verify >> the patchset. >> >> The steps to run the rdma_atomic_write example: >> server: >> $ ./rdma_atomic_write_server -s [server_address] -p [port_number] >> client: >> $ ./rdma_atomic_write_client -s [server_address] -p [port_number] >> >> The steps to run test_qp_ex_rc_atomic_write test: >> run_tests.py --dev rxe_enp0s3 --gid 1 -v test_qpex.QpExTestCase.test_qp_ex_rc_atomic_write >> test_qp_ex_rc_atomic_write (tests.test_qpex.QpExTestCase) ... ok >> >> ---------------------------------------------------------------------- >> Ran 1 test in 0.008s >> >> OK >> >> [1]: https://www.infinibandta.org/wp-content/uploads/2021/08/IBTA-Overview-of-IBTA-Volume-1-Release-1.5-and-MPE-2021-08-17-Secure.pptx >> [2]: https://github.com/yangx-jy/rdma-core/tree/new_api_with_point >> >> v5->v6: >> 1) Rebase on current for-next >> 2) Split the implementation of atomic write into 7 patches >> 3) Replace all "RDMA Atomic Write" with "atomic write" >> 4) Save 8-byte value in struct rxe_dma_info instead >> 5) Remove the print in atomic_write_reply() > > I think this looked OK, please fix the enum thing and also resolve all > the remarks on the github and rebase/repost/retest both series. Hi Jason, Thanks for your reminder. I will do it soon. In addition, I have resolved all remarks except the following one on github: EdwardSro: "keep an empty line at EoF" I: "I wonder why we need to add an empty line at EoF? I think there is no empty line at EOF in other files." I hope you or EdwardSro can answer my question. ^_^ Best Regards, Xiao Yang > > Thanks, > Jason
On Thu, Dec 01, 2022 at 07:58:44PM +0800, Xiao Yang wrote: > On 2022/11/23 3:54, Jason Gunthorpe wrote: > > On Sat, Oct 15, 2022 at 06:37:03AM +0000, yangx.jy@fujitsu.com wrote: > > > The IB SPEC v1.5[1] defined new atomic write operation. This patchset > > > makes SoftRoCE support new atomic write on RC service. > > > > > > On my rdma-core repository[2], I have introduced atomic write API > > > for libibverbs and Pyverbs. I also have provided a rdma_atomic_write > > > example and test_qp_ex_rc_atomic_write python test to verify > > > the patchset. > > > > > > The steps to run the rdma_atomic_write example: > > > server: > > > $ ./rdma_atomic_write_server -s [server_address] -p [port_number] > > > client: > > > $ ./rdma_atomic_write_client -s [server_address] -p [port_number] > > > > > > The steps to run test_qp_ex_rc_atomic_write test: > > > run_tests.py --dev rxe_enp0s3 --gid 1 -v test_qpex.QpExTestCase.test_qp_ex_rc_atomic_write > > > test_qp_ex_rc_atomic_write (tests.test_qpex.QpExTestCase) ... ok > > > > > > ---------------------------------------------------------------------- > > > Ran 1 test in 0.008s > > > > > > OK > > > > > > [1]: https://www.infinibandta.org/wp-content/uploads/2021/08/IBTA-Overview-of-IBTA-Volume-1-Release-1.5-and-MPE-2021-08-17-Secure.pptx > > > [2]: https://github.com/yangx-jy/rdma-core/tree/new_api_with_point > > > > > > v5->v6: > > > 1) Rebase on current for-next > > > 2) Split the implementation of atomic write into 7 patches > > > 3) Replace all "RDMA Atomic Write" with "atomic write" > > > 4) Save 8-byte value in struct rxe_dma_info instead > > > 5) Remove the print in atomic_write_reply() > > > > I think this looked OK, please fix the enum thing and also resolve all > > the remarks on the github and rebase/repost/retest both series. > Hi Jason, > > Thanks for your reminder. I will do it soon. > In addition, I have resolved all remarks except the following one on github: > EdwardSro: "keep an empty line at EoF" > I: "I wonder why we need to add an empty line at EoF? I think there is no > empty line at EOF in other files." It is not really "empty line" it is that the last character in the file should be '\n', and all files are like that. Jason
On 2022/12/1 20:58, Jason Gunthorpe wrote: > It is not really "empty line" it is that the last character in the > file should be '\n', and all files are like that. Hi Jason, Thanks for your explanation. I think my latest patch has added an "empty line", right? diff --git a/pyverbs/libibverbs_enums.pxd b/pyverbs/libibverbs_enums.pxd index 6a875fdd..c78a2284 100644 --- a/pyverbs/libibverbs_enums.pxd +++ b/pyverbs/libibverbs_enums.pxd ... @@ -507,4 +510,4 @@ cdef extern from "<infiniband/tm_types.h>": IBV_TMH_NO_TAG IBV_TMH_RNDV IBV_TMH_FIN - IBV_TMH_EAGER \ No newline at end of file + IBV_TMH_EAGER # cat -A pyverbs/libibverbs_enums.pxd | tail -2 IBV_TMH_FIN$ IBV_TMH_EAGER$ Best Regards, Xiao Yang > > Jason
On Thu, Dec 01, 2022 at 10:16:15PM +0800, Xiao Yang wrote: > On 2022/12/1 20:58, Jason Gunthorpe wrote: > > It is not really "empty line" it is that the last character in the > > file should be '\n', and all files are like that. > Hi Jason, > > Thanks for your explanation. > > I think my latest patch has added an "empty line", right? yes Jason