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

libhns: Two bugfixes in post_send flow #1513

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Conversation

hginjgerx
Copy link

Two bugfixes in post_send flow

The FENCE indicator in hns WQE doesn't ensure that response data from
a previous Read/Atomic operation has been written to the requester's
memory before the subsequent Send/Write operation is processed. This
may result in the subsequent Send/Write operation accessing the original
data in memory instead of the expected response data.

Unlike FENCE, the SO (Strong Order) indicator blocks the subsequent
operation until the previous response data is written to memory and a
bresp is returned. Set the SO indicator instead of FENCE to maintain
strict order.

Fixes: cbdf5e3 ("libhns: Reimplement verbs of post_send and post_recv for hip08 RoCE")
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
@rleon
Copy link
Member

rleon commented Nov 12, 2024

You should fix CI errors before we will merge it.

__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c: In function ‘hns_roce_u_v2_post_send’:
/__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c:1339:25: error: ‘wqe’ may be used uninitialized [-Werror=maybe-uninitialized]
 1339 |                         hns_roce_write_dwqe(qp, wqe);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c:1275:36: note: ‘wqe’ was declared here
 1275 |         struct hns_roce_rc_sq_wqe *wqe;
      |                                    ^~~
cc1: all warnings being treated as errors
[170/604] /usr/bin/gcc -DVERBS_DEBUG -Dirdma_rdmav34_EXPORTS -I/__w/1/s/BUILD/rdma-core-55.0/redhat-linux-build/include -I/usr/include/libnl3 -I/usr/include/drm -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64   -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror -Wredundant-decls -g -std=gnu11 -fPIC -MD -MT providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o -MF providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o.d -o providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o -c /__w/1/s/BUILD/rdma-core-55.0/providers/irdma/uk.c
ninja: build stopped: subcommand failed.

For QPs which do not have an SQ, such as XRC TGT,the send_cq
pointer will not be initailized. Since the supported max_gs
will be 0 in this case, check it and return before referencing
the send_cq pointer.

Fixes: cbdf5e3 ("libhns: Reimplement verbs of post_send and post_recv for hip08 RoCE")
Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
@hginjgerx
Copy link
Author

You should fix CI errors before we will merge it.

__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c: In function ‘hns_roce_u_v2_post_send’:
/__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c:1339:25: error: ‘wqe’ may be used uninitialized [-Werror=maybe-uninitialized]
 1339 |                         hns_roce_write_dwqe(qp, wqe);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/__w/1/s/BUILD/rdma-core-55.0/providers/hns/hns_roce_u_hw_v2.c:1275:36: note: ‘wqe’ was declared here
 1275 |         struct hns_roce_rc_sq_wqe *wqe;
      |                                    ^~~
cc1: all warnings being treated as errors
[170/604] /usr/bin/gcc -DVERBS_DEBUG -Dirdma_rdmav34_EXPORTS -I/__w/1/s/BUILD/rdma-core-55.0/redhat-linux-build/include -I/usr/include/libnl3 -I/usr/include/drm -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64   -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror -Wredundant-decls -g -std=gnu11 -fPIC -MD -MT providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o -MF providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o.d -o providers/irdma/CMakeFiles/irdma-rdmav34.dir/uk.c.o -c /__w/1/s/BUILD/rdma-core-55.0/providers/irdma/uk.c
ninja: build stopped: subcommand failed.

Sorry I forgot to check CI results. Fixed.

@rleon rleon merged commit 48d0db2 into linux-rdma:master Nov 12, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants