randomkang commented on code in PR #3304:
URL: https://github.com/apache/brpc/pull/3304#discussion_r4478069875
##########
test/brpc_rdma_unittest.cpp:
##########
@@ -2202,6 +2204,257 @@ TEST_F(RdmaTest,
server_alloc_resource_fail_fallback_tcp) {
StopServer();
}
+// Build a valid v3 hello that also carries an MTU value.
+rdma::RdmaHello MakeValidV3HelloWithMtu(uint32_t mtu) {
+ rdma::RdmaHello msg = MakeValidV3Hello();
+ msg.set_mtu(mtu);
+ return msg;
+}
+
+// A client hello carrying MTU must not break the server handshake:
+// the server still parses the hello and advances to S_ACK_WAIT.
+TEST_F(RdmaTest, v3_server_accepts_client_hello_with_mtu) {
+ StartServer();
+
+ sockaddr_in addr;
+ bzero((char*)&addr, sizeof(addr));
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(PORT);
+ butil::fd_guard sockfd(socket(AF_INET, SOCK_STREAM, 0));
+ ASSERT_TRUE(sockfd >= 0);
+ ASSERT_EQ(0, connect(sockfd, (sockaddr*)&addr, sizeof(sockaddr)));
+ Socket* s = WaitForServerSocket();
+ ASSERT_TRUE(s != NULL);
+
+ rdma::RdmaHello msg = MakeValidV3HelloWithMtu(IBV_MTU_4096);
+ std::string packet = MakeV3Packet(msg);
+ ASSERT_EQ((ssize_t)packet.size(),
+ write(sockfd, packet.data(), packet.size()));
+
+ ASSERT_RDMA_STATE(rdma::RdmaEndpoint::S_ACK_WAIT, RdmaTransportOf(s));
+
+ rdma::RdmaHello reply;
+ ReadServerV3Reply(sockfd, &reply);
+
+ // ACK flags=0 -> clean FALLBACK_TCP so the test ends without hardware.
+ uint32_t flags = butil::HostToNet32(0);
+ ASSERT_EQ((ssize_t)sizeof(flags), write(sockfd, &flags, sizeof(flags)));
+ ASSERT_RDMA_STATE(rdma::RdmaEndpoint::FALLBACK_TCP, RdmaTransportOf(s));
+
+ sockfd.reset(-1);
+ ASSERT_TRUE(WaitForServerSocketGone());
+ StopServer();
+}
+
+// The server reply must carry a negotiated MTU when the client advertised one.
+// The server computes min(local, client) at the start of BringUpQp (before the
+// g_skip_rdma_init early-return), so _outgoing_mtu is set even in UT mode;
+// FillLocalRdmaHello then includes it in the reply.
+TEST_F(RdmaTest, v3_server_reply_has_negotiated_mtu) {
+ StartServer();
+
+ sockaddr_in addr;
+ bzero((char*)&addr, sizeof(addr));
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(PORT);
+ butil::fd_guard sockfd(socket(AF_INET, SOCK_STREAM, 0));
+ ASSERT_TRUE(sockfd >= 0);
+ ASSERT_EQ(0, connect(sockfd, (sockaddr*)&addr, sizeof(sockaddr)));
+ Socket* s = WaitForServerSocket();
+ ASSERT_TRUE(s != NULL);
+
+ // Advertise a non-default MTU so we can verify negotiation.
+ rdma::RdmaHello msg = MakeValidV3HelloWithMtu(IBV_MTU_4096);
Review Comment:
ok
##########
src/brpc/rdma/rdma_endpoint.cpp:
##########
@@ -1448,7 +1458,20 @@ int RdmaEndpoint::BringUpQp(const ParsedHello& remote,
bool is_server) {
}
attr.qp_state = IBV_QPS_RTR;
- attr.path_mtu = IBV_MTU_1024; // TODO: support more mtu in future
+ // MTU negotiation: use the peer-advertised MTU if available, otherwise
+ // fall back to the legacy default (IBV_MTU_1024).
+ // Server side: _outgoing_mtu was already computed at function entry;
+ // reuse it for the QP path_mtu attribute.
+ // Client side: remote.path_mtu is already the server's negotiated
+ // min(local, client) and we use it as-is.
+ uint32_t negotiated_mtu =
+ std::min(static_cast<uint32_t>(IBV_MTU_1024), GetRdmaActiveMtu());
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
