diff options
author | Kevin Krakauer <krakauer@google.com> | 2019-02-28 12:52:06 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-02-28 12:53:02 -0800 |
commit | 420a89acd32f8deb0ff62f407c05109a2c378884 (patch) | |
tree | 7e5e0d7d5fef12ca735a3350c971a84b54045940 | |
parent | 121db29a93c651b8b62e8701bb0f16c231b08257 (diff) |
Fix flaky raw socket test.
The specific issue was:
- Test creates a raw ICMP socket
- Test sends an ICMP echo request (aka ping request) to itself via loopback
- Now two events race:
- The raw socket recieves the ICMP echo request
- Netstack receives the request and generates a reply (aka ping reply),
which it sends back over loopback, where it is eventually received by the
raw socket
- The test was written to expect packets in a specific order, but they can
come in any order.
PiperOrigin-RevId: 236179066
Change-Id: I02c07c919d3d28093add3d18dd9196fbbc870813
-rw-r--r-- | test/syscalls/linux/raw_socket_ipv4.cc | 105 |
1 files changed, 62 insertions, 43 deletions
diff --git a/test/syscalls/linux/raw_socket_ipv4.cc b/test/syscalls/linux/raw_socket_ipv4.cc index c6749321c..53174e736 100644 --- a/test/syscalls/linux/raw_socket_ipv4.cc +++ b/test/syscalls/linux/raw_socket_ipv4.cc @@ -45,15 +45,15 @@ class RawSocketTest : public ::testing::Test { // The loopback address. struct sockaddr_in addr_; - void sendEmptyICMP(struct icmphdr *icmp); + void SendEmptyICMP(struct icmphdr *icmp); - void sendEmptyICMPTo(int sock, struct sockaddr_in *addr, + void SendEmptyICMPTo(int sock, struct sockaddr_in *addr, struct icmphdr *icmp); - void receiveICMP(char *recv_buf, size_t recv_buf_len, size_t expected_size, + void ReceiveICMP(char *recv_buf, size_t recv_buf_len, size_t expected_size, struct sockaddr_in *src); - void receiveICMPFrom(char *recv_buf, size_t recv_buf_len, + void ReceiveICMPFrom(char *recv_buf, size_t recv_buf_len, size_t expected_size, struct sockaddr_in *src, int sock); }; @@ -97,34 +97,52 @@ TEST_F(RawSocketTest, SendAndReceive) { struct icmphdr icmp; icmp.type = ICMP_ECHO; icmp.code = 0; - icmp.checksum = *(unsigned short *)&icmp.checksum; - icmp.un.echo.sequence = *(unsigned short *)&icmp.un.echo.sequence; - icmp.un.echo.id = *(unsigned short *)&icmp.un.echo.id; - ASSERT_NO_FATAL_FAILURE(sendEmptyICMP(&icmp)); + icmp.checksum = 2011; + icmp.un.echo.sequence = 2012; + icmp.un.echo.id = 2014; + ASSERT_NO_FATAL_FAILURE(SendEmptyICMP(&icmp)); - // Receive the packet and make sure it's identical. + // We're going to receive both the echo request and reply, but the order is + // indeterminate. char recv_buf[512]; struct sockaddr_in src; - ASSERT_NO_FATAL_FAILURE(receiveICMP(recv_buf, ABSL_ARRAYSIZE(recv_buf), - sizeof(struct icmphdr), &src)); - EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); - EXPECT_EQ(memcmp(recv_buf + sizeof(struct iphdr), &icmp, sizeof(icmp)), 0); - - // We should also receive the automatically generated echo reply. - ASSERT_NO_FATAL_FAILURE(receiveICMP(recv_buf, ABSL_ARRAYSIZE(recv_buf), - sizeof(struct icmphdr), &src)); - EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); - struct icmphdr *reply_icmp = - (struct icmphdr *)(recv_buf + sizeof(struct iphdr)); - // Most fields should be the same. - EXPECT_EQ(reply_icmp->code, icmp.code); - EXPECT_EQ(reply_icmp->un.echo.sequence, icmp.un.echo.sequence); - EXPECT_EQ(reply_icmp->un.echo.id, icmp.un.echo.id); - // A couple are different. - EXPECT_EQ(reply_icmp->type, ICMP_ECHOREPLY); - // The checksum is computed in such a way that it is guaranteed to have - // changed. - EXPECT_NE(reply_icmp->checksum, icmp.checksum); + bool received_request = false; + bool received_reply = false; + + for (int i = 0; i < 2; i++) { + // Receive the packet. + ASSERT_NO_FATAL_FAILURE(ReceiveICMP(recv_buf, ABSL_ARRAYSIZE(recv_buf), + sizeof(struct icmphdr), &src)); + EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); + struct icmphdr *recvd_icmp = + reinterpret_cast<struct icmphdr *>(recv_buf + sizeof(struct iphdr)); + switch (recvd_icmp->type) { + case ICMP_ECHO: + EXPECT_FALSE(received_request); + received_request = true; + // The packet should be identical to what we sent. + EXPECT_EQ(memcmp(recv_buf + sizeof(struct iphdr), &icmp, sizeof(icmp)), + 0); + break; + + case ICMP_ECHOREPLY: + EXPECT_FALSE(received_reply); + received_reply = true; + // Most fields should be the same. + EXPECT_EQ(recvd_icmp->code, icmp.code); + EXPECT_EQ(recvd_icmp->un.echo.sequence, icmp.un.echo.sequence); + EXPECT_EQ(recvd_icmp->un.echo.id, icmp.un.echo.id); + // A couple are different. + EXPECT_EQ(recvd_icmp->type, ICMP_ECHOREPLY); + // The checksum is computed in such a way that it is guaranteed to have + // changed. + EXPECT_NE(recvd_icmp->checksum, icmp.checksum); + break; + } + } + + ASSERT_TRUE(received_request); + ASSERT_TRUE(received_reply); } // We should be able to create multiple raw sockets for the same protocol and @@ -141,21 +159,21 @@ TEST_F(RawSocketTest, MultipleSocketReceive) { struct icmphdr icmp; icmp.type = ICMP_ECHO; icmp.code = 0; - icmp.checksum = *(unsigned short *)&icmp.checksum; - icmp.un.echo.sequence = *(unsigned short *)&icmp.un.echo.sequence; - icmp.un.echo.id = *(unsigned short *)&icmp.un.echo.id; - ASSERT_NO_FATAL_FAILURE(sendEmptyICMP(&icmp)); + icmp.checksum = 2014; + icmp.un.echo.sequence = 2016; + icmp.un.echo.id = 2018; + ASSERT_NO_FATAL_FAILURE(SendEmptyICMP(&icmp)); // Receive it on socket 1. char recv_buf1[512]; struct sockaddr_in src; - ASSERT_NO_FATAL_FAILURE(receiveICMP(recv_buf1, ABSL_ARRAYSIZE(recv_buf1), + ASSERT_NO_FATAL_FAILURE(ReceiveICMP(recv_buf1, ABSL_ARRAYSIZE(recv_buf1), sizeof(struct icmphdr), &src)); EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); // Receive it on socket 2. char recv_buf2[512]; - ASSERT_NO_FATAL_FAILURE(receiveICMPFrom(recv_buf2, ABSL_ARRAYSIZE(recv_buf2), + ASSERT_NO_FATAL_FAILURE(ReceiveICMPFrom(recv_buf2, ABSL_ARRAYSIZE(recv_buf2), sizeof(struct icmphdr), &src, s2.get())); EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); @@ -177,7 +195,8 @@ TEST_F(RawSocketTest, RawAndPingSockets) { struct icmphdr icmp; icmp.type = ICMP_ECHO; icmp.code = 0; - icmp.un.echo.sequence = *(unsigned short *)&icmp.un.echo.sequence; + icmp.un.echo.sequence = + *static_cast<unsigned short *>(&icmp.un.echo.sequence); ASSERT_THAT(RetryEINTR(sendto)(ping_sock.get(), &icmp, sizeof(icmp), 0, (struct sockaddr *)&addr_, sizeof(addr_)), SyscallSucceedsWithValue(sizeof(icmp))); @@ -185,7 +204,7 @@ TEST_F(RawSocketTest, RawAndPingSockets) { // Receive the packet via raw socket. char recv_buf[512]; struct sockaddr_in src; - ASSERT_NO_FATAL_FAILURE(receiveICMP(recv_buf, ABSL_ARRAYSIZE(recv_buf), + ASSERT_NO_FATAL_FAILURE(ReceiveICMP(recv_buf, ABSL_ARRAYSIZE(recv_buf), sizeof(struct icmphdr), &src)); EXPECT_EQ(memcmp(&src, &addr_, sizeof(sockaddr_in)), 0); @@ -201,11 +220,11 @@ TEST_F(RawSocketTest, RawAndPingSockets) { 0); } -void RawSocketTest::sendEmptyICMP(struct icmphdr *icmp) { - ASSERT_NO_FATAL_FAILURE(sendEmptyICMPTo(s_, &addr_, icmp)); +void RawSocketTest::SendEmptyICMP(struct icmphdr *icmp) { + ASSERT_NO_FATAL_FAILURE(SendEmptyICMPTo(s_, &addr_, icmp)); } -void RawSocketTest::sendEmptyICMPTo(int sock, struct sockaddr_in *addr, +void RawSocketTest::SendEmptyICMPTo(int sock, struct sockaddr_in *addr, struct icmphdr *icmp) { struct iovec iov = {.iov_base = icmp, .iov_len = sizeof(*icmp)}; struct msghdr msg { @@ -215,13 +234,13 @@ void RawSocketTest::sendEmptyICMPTo(int sock, struct sockaddr_in *addr, ASSERT_THAT(sendmsg(sock, &msg, 0), SyscallSucceedsWithValue(sizeof(*icmp))); } -void RawSocketTest::receiveICMP(char *recv_buf, size_t recv_buf_len, +void RawSocketTest::ReceiveICMP(char *recv_buf, size_t recv_buf_len, size_t expected_size, struct sockaddr_in *src) { ASSERT_NO_FATAL_FAILURE( - receiveICMPFrom(recv_buf, recv_buf_len, expected_size, src, s_)); + ReceiveICMPFrom(recv_buf, recv_buf_len, expected_size, src, s_)); } -void RawSocketTest::receiveICMPFrom(char *recv_buf, size_t recv_buf_len, +void RawSocketTest::ReceiveICMPFrom(char *recv_buf, size_t recv_buf_len, size_t expected_size, struct sockaddr_in *src, int sock) { struct iovec iov = {.iov_base = recv_buf, .iov_len = recv_buf_len}; |