chenBright commented on code in PR #3304:
URL: https://github.com/apache/brpc/pull/3304#discussion_r4438372533


##########
test/brpc_rdma_unittest.cpp:
##########
@@ -2202,6 +2202,275 @@ 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)));
+    usleep(100000);

Review Comment:
   Could we replace the fixed `usleep(100000)` calls with condition-based waits?
   
   Connection acceptance and handshake processing happen asynchronously, so 
they may take longer than 100 ms on a heavily loaded CI worker. In that case, 
these tests can fail intermittently even when the implementation is correct.
   
   The existing `WaitForServerSocket()`, `ASSERT_RDMA_STATE`, and 
`WaitForServerSocketGone()` helpers provide bounded waits for these transitions.



-- 
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]

Reply via email to