Skip to content

Commit

Permalink
fix huge rdma_recv_block_type no effect issue. (#2326)
Browse files Browse the repository at this point in the history 
It has no effect for `huge` type of recv_block_size
which is as large as 2^21, because the `block_size`
variable is defined as type `uint16_t`, its maximum
value is 2^16. So, change the type of `block_size`
to `uint32_t` with relevant updates.

Signed-off-by: Lijin Xiong <lijin.xiong@zstack.io>
Co-authored-by: Lijin Xiong <lijin.xiong@zstack.io>
  • Loading branch information
@legionxiong
legionxiong and Lijin Xiong authored Oct 25, 2023
1 parent e7d75c6 commit d142274
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 29 deletions.
19 changes: 11 additions & 8 deletions src/brpc/rdma/rdma_endpoint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,20 @@ static const size_t RESERVED_WR_NUM = 3;
// message length (2B)
// hello version (2B)
// impl version (2B): 0 means should use tcp
// block size (2B)
// block size (4B)
// sq size (2B)
// rq size (2B)
// GID (16B)
// QP number (4B)
static const char* MAGIC_STR = "RDMA";
static const size_t MAGIC_STR_LEN = 4;
static const size_t HELLO_MSG_LEN_MIN = 38;
static const size_t HELLO_MSG_LEN_MIN = 40;
static const size_t HELLO_MSG_LEN_MAX = 4096;
static const size_t ACK_MSG_LEN = 4;
static uint16_t g_rdma_hello_msg_len = 38; // In Byte
static uint16_t g_rdma_hello_version = 1;
static uint16_t g_rdma_hello_msg_len = 40; // In Byte
static uint16_t g_rdma_hello_version = 2;
static uint16_t g_rdma_impl_version = 1;
static uint16_t g_rdma_recv_block_size = 0;
static uint32_t g_rdma_recv_block_size = 0;

static const uint32_t MAX_INLINE_DATA = 64;
static const uint8_t MAX_HOP_LIMIT = 16;
Expand All @@ -105,7 +105,7 @@ struct HelloMessage {
uint16_t msg_len;
uint16_t hello_ver;
uint16_t impl_ver;
uint16_t block_size;
uint32_t block_size;
uint16_t sq_size;
uint16_t rq_size;
uint16_t lid;
Expand All @@ -118,7 +118,9 @@ void HelloMessage::Serialize(void* data) const {
*(current_pos++) = butil::HostToNet16(msg_len);
*(current_pos++) = butil::HostToNet16(hello_ver);
*(current_pos++) = butil::HostToNet16(impl_ver);
*(current_pos++) = butil::HostToNet16(block_size);
uint32_t* block_size_pos = (uint32_t*)current_pos;
*block_size_pos = butil::HostToNet32(block_size);
current_pos += 2; // move forward 4 Bytes
*(current_pos++) = butil::HostToNet16(sq_size);
*(current_pos++) = butil::HostToNet16(rq_size);
*(current_pos++) = butil::HostToNet16(lid);
Expand All @@ -132,7 +134,8 @@ void HelloMessage::Deserialize(void* data) {
msg_len = butil::NetToHost16(*current_pos++);
hello_ver = butil::NetToHost16(*current_pos++);
impl_ver = butil::NetToHost16(*current_pos++);
block_size = butil::NetToHost16(*current_pos++);
block_size = butil::NetToHost32(*(uint32_t*)current_pos);
current_pos += 2; // move forward 4 Bytes
sq_size = butil::NetToHost16(*current_pos++);
rq_size = butil::NetToHost16(*current_pos++);
lid = butil::NetToHost16(*current_pos++);
Expand Down
2 changes: 1 addition & 1 deletion src/brpc/rdma/rdma_endpoint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ friend class brpc::Socket;
// Data address of _rbuf
std::vector<void*> _rbuf_data;
// Remote block size for receiving
uint16_t _remote_recv_block_size;
uint32_t _remote_recv_block_size;

// The number of new recv WRs acked to the remote side
uint16_t _accumulated_ack;
Expand Down
41 changes: 21 additions & 20 deletions test/brpc_rdma_unittest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "echo.pb.h"

static const int PORT = 8713;
static const size_t RDMA_HELLO_MSG_LEN = 40;

using namespace brpc;

Expand Down Expand Up @@ -203,7 +204,7 @@ TEST_F(RdmaTest, client_hello_msg_invalid_magic_str) {
Socket* s = GetSocketFromServer(0);
ASSERT_EQ(rdma::RdmaEndpoint::UNINIT, s->_rdma_ep->_state);

uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
memcpy(data, "PRPC", 4); // send as normal baidu_std protocol
memset(data + 4, 0, 32);
ASSERT_EQ(38, write(sockfd, data, 38));
Expand Down Expand Up @@ -277,7 +278,7 @@ TEST_F(RdmaTest, client_hello_msg_invalid_len) {
addr.sin_family = AF_INET;
addr.sin_port = htons(PORT);
Socket* s = NULL;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];

butil::fd_guard sockfd1(socket(AF_INET, SOCK_STREAM, 0));
ASSERT_TRUE(sockfd1 >= 0);
Expand Down Expand Up @@ -316,7 +317,7 @@ TEST_F(RdmaTest, client_hello_msg_invalid_version) {
addr.sin_family = AF_INET;
addr.sin_port = htons(PORT);
Socket* s = NULL;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
uint16_t len = butil::HostToNet16(38);
uint16_t ver = butil::HostToNet16(1);

Expand Down Expand Up @@ -371,7 +372,7 @@ TEST_F(RdmaTest, client_hello_msg_invalid_sq_rq_block_size) {
addr.sin_port = htons(PORT);
Socket* s = NULL;
rdma::HelloMessage msg;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
msg.msg_len = 38;
msg.hello_ver = 1;
msg.impl_ver = 1;
Expand Down Expand Up @@ -447,7 +448,7 @@ TEST_F(RdmaTest, client_close_after_qp_build) {
addr.sin_port = htons(PORT);
Socket* s = NULL;
rdma::HelloMessage msg;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
msg.msg_len = 38;
msg.hello_ver = 1;
msg.impl_ver = 1;
Expand Down Expand Up @@ -484,7 +485,7 @@ TEST_F(RdmaTest, client_close_during_ack_send) {
addr.sin_port = htons(PORT);
Socket* s = NULL;
rdma::HelloMessage msg;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
msg.msg_len = 38;
msg.hello_ver = 1;
msg.impl_ver = 1;
Expand Down Expand Up @@ -525,7 +526,7 @@ TEST_F(RdmaTest, client_close_after_ack_send) {
addr.sin_port = htons(PORT);
Socket* s = NULL;
rdma::HelloMessage msg;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
msg.msg_len = 38;
msg.hello_ver = 1;
msg.impl_ver = 1;
Expand Down Expand Up @@ -583,7 +584,7 @@ TEST_F(RdmaTest, client_send_data_on_tcp_after_ack_send) {
addr.sin_port = htons(PORT);
Socket* s = NULL;
rdma::HelloMessage msg;
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
msg.msg_len = 38;
msg.hello_ver = 1;
msg.impl_ver = 1;
Expand Down Expand Up @@ -692,7 +693,7 @@ TEST_F(RdmaTest, server_close_before_hello_send) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
close(acc_fd);
usleep(100000);
Expand Down Expand Up @@ -728,7 +729,7 @@ TEST_F(RdmaTest, server_miss_during_magic_str) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
ASSERT_EQ(2, write(acc_fd, "RD", 2));
usleep(100000);
Expand Down Expand Up @@ -763,7 +764,7 @@ TEST_F(RdmaTest, server_close_during_magic_str) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
ASSERT_EQ(2, write(acc_fd, "RD", 2));
close(acc_fd);
Expand Down Expand Up @@ -800,7 +801,7 @@ TEST_F(RdmaTest, server_hello_invalid_magic_str) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
ASSERT_EQ(4, write(acc_fd, "ABCD", 4));
usleep(100000);
Expand Down Expand Up @@ -836,7 +837,7 @@ TEST_F(RdmaTest, server_miss_during_hello_msg) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
ASSERT_EQ(4, write(acc_fd, "RDMA", 4));
ASSERT_EQ(2, write(acc_fd, "00", 2));
Expand Down Expand Up @@ -871,7 +872,7 @@ TEST_F(RdmaTest, server_close_during_hello_msg) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
ASSERT_EQ(4, write(acc_fd, "RDMA", 4));
ASSERT_EQ(2, write(acc_fd, "00", 2));
Expand Down Expand Up @@ -909,7 +910,7 @@ TEST_F(RdmaTest, server_hello_invalid_msg_len) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
memcpy(data, "RDMA", 4);
uint16_t len = butil::HostToNet16(35);
Expand Down Expand Up @@ -949,7 +950,7 @@ TEST_F(RdmaTest, server_hello_invalid_version) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));
memcpy(data, "RDMA", 4);
uint16_t len = butil::HostToNet16(38);
Expand Down Expand Up @@ -992,7 +993,7 @@ TEST_F(RdmaTest, server_hello_invalid_sq_rq_size) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));

rdma::HelloMessage msg;
Expand Down Expand Up @@ -1044,7 +1045,7 @@ TEST_F(RdmaTest, server_miss_after_ack) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));

rdma::HelloMessage msg;
Expand Down Expand Up @@ -1096,7 +1097,7 @@ TEST_F(RdmaTest, server_close_after_ack) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));

rdma::HelloMessage msg;
Expand Down Expand Up @@ -1149,7 +1150,7 @@ TEST_F(RdmaTest, server_send_data_on_tcp_after_ack) {

butil::fd_guard acc_fd(accept(sockfd, NULL, NULL));
ASSERT_TRUE(acc_fd >= 0);
uint8_t data[38];
uint8_t data[RDMA_HELLO_MSG_LEN];
ASSERT_EQ(38, read(acc_fd, data, 38));

rdma::HelloMessage msg;
Expand Down

0 comments on commit d142274

Please sign in to comment.