Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support UD on hip08 #587

Closed
wants to merge 4 commits into from
Closed

Conversation

Li-Weihang
Copy link

This patchset enables hip08 to support Unreliable Datagram.

@@ -68,6 +68,8 @@
#define HNS_ROCE_TPTR_OFFSET 0x1000
#define HNS_ROCE_STATIC_RATE 3 /* Gbps */

#define ETH_ALEN 6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is declared in "linux/if_ether.h"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will modify, thank you.

Comment on lines 484 to 486

wqe_idx = (qp->sq.head + nreq) & (qp->sq.wqe_cnt - 1);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand your problem and solution, but a couple of lines above you have hns_roce_wq_overflow() function which supposes to protect from overflow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus the "a %= b" construction is the better way to write it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand your problem and solution, but a couple of lines above you have hns_roce_wq_overflow() function which supposes to protect from overflow.

Hi, Leon, thanks. You are right, the commit message is uncorrect, I will rewrite it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus the "a %= b" construction is the better way to write it.

Hi Leon, do you mean:
wqe_idx = (qp->sq.head + nreq) % qp->sq.wqe_cnt;

switch (wr->opcode) {
case IBV_WR_SEND:
hr_op = HNS_ROCE_WQE_OP_SEND;
ud_sq_wqe->immtdata = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cleared it a second before.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will remove it, thank you.

sge_info->total_len = 0;

for (i = 0; i < wr->num_sge; i++)
if (likely(wr->sg_list[i].length)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

success-oriented flow, please

if (unlikely(,,,))
continue;

..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thank you for your advice :)


if (wr->send_flags & IBV_SEND_INLINE) {
if (wr->opcode == IBV_WR_RDMA_READ) {
fprintf(stderr, "Not supported inline data!\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No fprints in libraries

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No fprints in libraries

Hi Leon,
Do you means that we should print informations only when debug in libraries? Or it's not reasonable to print message here in inline mode?
I saw some fprintf in other drivers, should I replace all printf/fprintf in hns with a macro that only show message when debug?


if (sge_info->total_len > qp->max_inline_data) {
fprintf(stderr,
"Failed to inline, data len=%d, max inline len=%d!\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Hi Leon,
I have modified code as suggested and pushed a new version, could you please review it again? Thank you :)

@Li-Weihang Li-Weihang requested a review from rleon October 10, 2019 07:12
@Li-Weihang Li-Weihang force-pushed the ud_support branch 2 times, most recently from e347970 to e193d07 Compare October 16, 2019 11:40
oulijun and others added 4 commits October 28, 2019 16:58
This patch achieves two verbs create_ah and destroy_ah to support
allocation and destruction of Address Handle.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
Currently, the wqe idx is calculated repeatly everywhere it is used.
This patch defines wqe_idx and calculated it only once, then just use it
as needed.

Signed-off-by: Yixian Liu <liuyixian@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
This patch refactors the interface of hns_roce_u_v2_post_send, which
is now very complicated. We reduce the complexity with following points:
1. Separate RC server into a function.
2. Simplify and separate the process of sge.
3. Keep the logic and consistence of all operations.

Signed-off-by: Yixian Liu <liuyixian@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
User Application can use Unreliable Datagram on hip08 now.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants