QBONE ICMP reachable: eliminate last usage of `SOCK_RAW`. A prior CL converted the send direction to [`SOCK_DGRAM`](https://lore.kernel.org/all/20110510180957.GA3262@albatros/) but the receive direction was also using raw sockets, with socket options set using `struct icmp6_filter`. Now that we are using `SOCK_DGRAM`, we can simply read the ICMP echo responses from the same socket, greatly simplifying all of this. PiperOrigin-RevId: 848258830
diff --git a/quiche/quic/qbone/bonnet/icmp_reachable.cc b/quiche/quic/qbone/bonnet/icmp_reachable.cc index 83239dc..7bee28c 100644 --- a/quiche/quic/qbone/bonnet/icmp_reachable.cc +++ b/quiche/quic/qbone/bonnet/icmp_reachable.cc
@@ -42,8 +42,7 @@ alarm_(alarm_factory_->CreateAlarm(new AlarmCallback(this))), kernel_(kernel), stats_(stats), - send_fd_(0), - recv_fd_(0) { + sock_fd_(0) { src_.sin6_family = AF_INET6; dst_.sin6_family = AF_INET6; // Ensure the destination has its scope set to the QBONE TUN/TAP device. @@ -55,63 +54,40 @@ } IcmpReachable::~IcmpReachable() { - if (send_fd_ > 0) { - kernel_->close(send_fd_); - } - if (recv_fd_ > 0) { - bool success = event_loop_->UnregisterSocket(recv_fd_); - QUICHE_DCHECK(success); - - kernel_->close(recv_fd_); + if (sock_fd_ > 0) { + if (polling_registered_) { + bool success = event_loop_->UnregisterSocket(sock_fd_); + QUICHE_DCHECK(success); + } + kernel_->close(sock_fd_); } } bool IcmpReachable::Init() { - send_fd_ = + sock_fd_ = kernel_->socket(PF_INET6, SOCK_DGRAM | SOCK_NONBLOCK, IPPROTO_ICMPV6); - if (send_fd_ < 0) { - QUIC_PLOG(ERROR) << "Unable to open send socket."; + if (sock_fd_ < 0) { + QUIC_PLOG(ERROR) << "Unable to open ICMP socket."; return false; } - if (kernel_->bind(send_fd_, reinterpret_cast<struct sockaddr*>(&src_), + if (kernel_->bind(sock_fd_, reinterpret_cast<struct sockaddr*>(&src_), sizeof(sockaddr_in6)) < 0) { - QUIC_PLOG(ERROR) << "Unable to bind send socket."; + QUIC_PLOG(ERROR) << "Unable to bind ICMP socket."; return false; } - recv_fd_ = - kernel_->socket(PF_INET6, SOCK_RAW | SOCK_NONBLOCK, IPPROTO_ICMPV6); - if (recv_fd_ < 0) { - QUIC_PLOG(ERROR) << "Unable to open recv socket."; + if (!event_loop_->RegisterSocket(sock_fd_, kEventMask, &cb_)) { + QUIC_LOG(ERROR) << "Unable to register ICMP socket"; return false; } - - if (kernel_->bind(recv_fd_, reinterpret_cast<struct sockaddr*>(&src_), - sizeof(sockaddr_in6)) < 0) { - QUIC_PLOG(ERROR) << "Unable to bind recv socket."; - return false; - } - - icmp6_filter filter; - ICMP6_FILTER_SETBLOCKALL(&filter); - ICMP6_FILTER_SETPASS(ICMP6_ECHO_REPLY, &filter); - if (kernel_->setsockopt(recv_fd_, SOL_ICMPV6, ICMP6_FILTER, &filter, - sizeof(filter)) < 0) { - QUIC_LOG(ERROR) << "Unable to set ICMP6 filter."; - return false; - } - - if (!event_loop_->RegisterSocket(recv_fd_, kEventMask, &cb_)) { - QUIC_LOG(ERROR) << "Unable to register recv ICMP socket"; - return false; - } + polling_registered_ = true; alarm_->Set(clock_->Now()); - // Obtain the local port assigned to send_fd_. + // Obtain the local port assigned to sock_fd_. struct sockaddr_in6 sa = {}; socklen_t addrlen = sizeof(sa); - if (kernel_->getsockname(send_fd_, reinterpret_cast<struct sockaddr*>(&sa), + if (kernel_->getsockname(sock_fd_, reinterpret_cast<struct sockaddr*>(&sa), &addrlen) == -1) { QUIC_PLOG(ERROR) << "Unable to getsockname:"; } @@ -190,7 +166,7 @@ reinterpret_cast<const char*>(&icmp_header_), sizeof(icmp_header_))); - ssize_t size = kernel_->sendto(send_fd_, &icmp_header_, sizeof(icmp6_hdr), 0, + ssize_t size = kernel_->sendto(sock_fd_, &icmp_header_, sizeof(icmp6_hdr), 0, reinterpret_cast<struct sockaddr*>(&dst_), sizeof(sockaddr_in6));
diff --git a/quiche/quic/qbone/bonnet/icmp_reachable.h b/quiche/quic/qbone/bonnet/icmp_reachable.h index 8efaade..d88632a 100644 --- a/quiche/quic/qbone/bonnet/icmp_reachable.h +++ b/quiche/quic/qbone/bonnet/icmp_reachable.h
@@ -134,8 +134,8 @@ StatsInterface* stats_; - int send_fd_; - int recv_fd_; + int sock_fd_; + bool polling_registered_ = false; absl::Mutex header_lock_; icmp6_hdr icmp_header_ ABSL_GUARDED_BY(header_lock_){};
diff --git a/quiche/quic/qbone/bonnet/icmp_reachable_test.cc b/quiche/quic/qbone/bonnet/icmp_reachable_test.cc index 81dbbce..492d578 100644 --- a/quiche/quic/qbone/bonnet/icmp_reachable_test.cc +++ b/quiche/quic/qbone/bonnet/icmp_reachable_test.cc
@@ -31,7 +31,6 @@ constexpr char kSourceAddress[] = "fe80:1:2:3:4::1"; constexpr char kDestinationAddress[] = "fe80:4:3:2:1::1"; -constexpr int kFakeWriteFd = 0; constexpr int kSendPort = 12345; icmp6_hdr ParseIcmpHeader(const void* buf, size_t len) { @@ -94,26 +93,22 @@ int pipe_fds[2]; QUICHE_CHECK(pipe(pipe_fds) >= 0) << "pipe() failed"; - read_fd_ = pipe_fds[0]; - read_src_fd_ = pipe_fds[1]; + simulated_sock_fd_ = pipe_fds[0]; + simulated_sock_recv_fd_ = pipe_fds[1]; } void SetFdExpectations() { InSequence seq; EXPECT_CALL(kernel_, if_nametoindex(_)); - EXPECT_CALL(kernel_, socket(_, _, _)).WillOnce(Return(kFakeWriteFd)); - EXPECT_CALL(kernel_, bind(kFakeWriteFd, _, _)).WillOnce(Return(0)); + EXPECT_CALL(kernel_, socket(_, _, _)).WillOnce(Return(simulated_sock_fd_)); + EXPECT_CALL(kernel_, bind(simulated_sock_fd_, _, _)).WillOnce(Return(0)); - EXPECT_CALL(kernel_, socket(_, _, _)).WillOnce(Return(read_fd_)); - EXPECT_CALL(kernel_, bind(read_fd_, _, _)).WillOnce(Return(0)); - - EXPECT_CALL(kernel_, setsockopt(read_fd_, SOL_ICMPV6, ICMP6_FILTER, _, _)); EXPECT_CALL(kernel_, getsockname(_, _, _)) .WillOnce(DoAll(SetArgPointee<1>( *reinterpret_cast<struct sockaddr*>(&send_socket_)), Return(0))); - EXPECT_CALL(kernel_, close(read_fd_)).WillOnce([](int fd) { + EXPECT_CALL(kernel_, close(simulated_sock_fd_)).WillOnce([](int fd) { return close(fd); }); } @@ -124,8 +119,8 @@ struct sockaddr_in6 send_socket_ = {.sin6_port = kSendPort}; - int read_fd_; - int read_src_fd_; + int simulated_sock_fd_; + int simulated_sock_recv_fd_; StrictMock<MockKernel> kernel_; std::unique_ptr<QuicEventLoop> event_loop_; @@ -140,7 +135,7 @@ ASSERT_TRUE(reachable.Init()); - EXPECT_CALL(kernel_, sendto(kFakeWriteFd, _, _, _, _, _)) + EXPECT_CALL(kernel_, sendto(simulated_sock_fd_, _, _, _, _, _)) .WillOnce([](int sockfd, const void* buf, size_t len, int flags, const struct sockaddr* dest_addr, socklen_t addrlen) { auto icmp_header = ParseIcmpHeader(buf, len); @@ -162,7 +157,7 @@ ASSERT_TRUE(reachable.Init()); - EXPECT_CALL(kernel_, sendto(kFakeWriteFd, _, _, _, _, _)) + EXPECT_CALL(kernel_, sendto(simulated_sock_fd_, _, _, _, _, _)) .Times(2) .WillRepeatedly([](int sockfd, const void* buf, size_t len, int flags, const struct sockaddr* dest_addr, @@ -186,7 +181,7 @@ ASSERT_TRUE(reachable.Init()); icmp6_hdr last_request_hdr{}; - EXPECT_CALL(kernel_, sendto(kFakeWriteFd, _, _, _, _, _)) + EXPECT_CALL(kernel_, sendto(simulated_sock_fd_, _, _, _, _, _)) .Times(2) .WillRepeatedly([&last_request_hdr]( int sockfd, const void* buf, size_t len, int flags, @@ -199,7 +194,7 @@ std::string packed_source = source_.ToPackedString(); memcpy(&source_addr.sin6_addr, packed_source.data(), packed_source.size()); - EXPECT_CALL(kernel_, recvfrom(read_fd_, _, _, _, _, _)) + EXPECT_CALL(kernel_, recvfrom(simulated_sock_fd_, _, _, _, _, _)) .WillOnce([&source_addr](int sockfd, void* buf, size_t len, int flags, struct sockaddr* src_addr, socklen_t* addrlen) { *reinterpret_cast<sockaddr_in6*>(src_addr) = source_addr; @@ -212,7 +207,7 @@ icmp6_hdr response = last_request_hdr; response.icmp6_type = ICMP6_ECHO_REPLY; - write(read_src_fd_, reinterpret_cast<const void*>(&response), + write(simulated_sock_recv_fd_, reinterpret_cast<const void*>(&response), sizeof(icmp6_hdr)); event_loop_->RunEventLoopOnce(QuicTime::Delta::FromSeconds(1)); @@ -230,7 +225,7 @@ ASSERT_TRUE(reachable.Init()); - EXPECT_CALL(kernel_, sendto(kFakeWriteFd, _, _, _, _, _)) + EXPECT_CALL(kernel_, sendto(simulated_sock_fd_, _, _, _, _, _)) .WillOnce([](int sockfd, const void* buf, size_t len, int flags, const struct sockaddr* dest_addr, socklen_t addrlen) { errno = EAGAIN; @@ -249,12 +244,12 @@ ASSERT_TRUE(reachable.Init()); - EXPECT_CALL(kernel_, sendto(kFakeWriteFd, _, _, _, _, _)) + EXPECT_CALL(kernel_, sendto(simulated_sock_fd_, _, _, _, _, _)) .WillOnce([](int sockfd, const void* buf, size_t len, int flags, const struct sockaddr* dest_addr, socklen_t addrlen) { return len; }); - EXPECT_CALL(kernel_, recvfrom(read_fd_, _, _, _, _, _)) + EXPECT_CALL(kernel_, recvfrom(simulated_sock_fd_, _, _, _, _, _)) .WillOnce([](int sockfd, void* buf, size_t len, int flags, struct sockaddr* src_addr, socklen_t* addrlen) { errno = EIO; @@ -263,7 +258,7 @@ icmp6_hdr response{}; - write(read_src_fd_, reinterpret_cast<const void*>(&response), + write(simulated_sock_recv_fd_, reinterpret_cast<const void*>(&response), sizeof(icmp6_hdr)); event_loop_->RunEventLoopOnce(QuicTime::Delta::FromSeconds(1));