De-flakify sock_diag_test. SockDestroyUdpTest.testClosesUdpSockets was failing a low percentage of the time because of two bugs: 1. When creating a UDP socket pair, we were connecting one socket to the other, but the other socket to iself. This made it possible for sockets on ::ffff:127.0.0.1 and ::1 to be bound to the same port, and: 2. The code that attempts to get a diag_msg for a given socket did not consider the fact that socket dumps, unlike operations that operate on a specific socket with a cookie, do not use the IP addresses in idiag_src and idiag_dst. This made it so that attempting to close a socket on [::1:1234 might have instead closed a different socket on [::ffff:127.0.0.1]:1234. Fix #1 by not doing the wrong thing, and fix #2 by using the inode number to ensure that FindSockInfoFromFd returns the requested socket. Also remove FindSockInfoFromReq, since it was only used once and was potentially misleading - while it only returns one socket, it might not return the one the caller expects. This allows us to tighten the code and ensure that operations that are supposed to return one socket only ever return one socket. Test: all_tests.sh passes on android-3.18. Bug: 31119353 Change-Id: I5d65e5a30c37490db516b5c6e730f89b5fea1b27
diff --git a/net/test/net_test.py b/net/test/net_test.py index f58e2b9..0048ae6 100755 --- a/net/test/net_test.py +++ b/net/test/net_test.py
@@ -176,14 +176,14 @@ addr = listensock.getsockname() if socktype == SOCK_STREAM: listensock.listen(1) - clientsock.connect(addr) + clientsock.connect(listensock.getsockname()) if socktype == SOCK_STREAM: acceptedsock, _ = listensock.accept() DisableFinWait(clientsock) DisableFinWait(acceptedsock) listensock.close() else: - listensock.connect(addr) + listensock.connect(clientsock.getsockname()) acceptedsock = listensock return clientsock, acceptedsock
diff --git a/net/test/sock_diag.py b/net/test/sock_diag.py index 4efd0c5..13ed0a8 100755 --- a/net/test/sock_diag.py +++ b/net/test/sock_diag.py
@@ -19,6 +19,7 @@ # pylint: disable=g-bad-todo import errno +import os from socket import * # pylint: disable=wildcard-import import struct @@ -374,15 +375,21 @@ sock_id = InetDiagSockId((sport, dport, src, dst, iface, "\x00" * 8)) return InetDiagReqV2((family, protocol, 0, 0xffffffff, sock_id)) - def FindSockInfoFromReq(self, req): - for diag_msg, attrs in self.Dump(req, ""): - return diag_msg, attrs - raise ValueError("Dump of %s returned no sockets" % req) - def FindSockInfoFromFd(self, s): """Gets a diag_msg and attrs from the kernel for the specified socket.""" req = self.DiagReqFromSocket(s) - return self.FindSockInfoFromReq(req) + # The kernel doesn't use idiag_src and idiag_dst when dumping sockets, it + # only uses them when targeting a specific socket with a cookie. Check the + # the inode number to ensure we don't mistakenly match another socket on + # the same port but with a different IP address. + inode = os.fstat(s.fileno()).st_ino + results = self.Dump(req, "") + if len(results) == 0: + raise ValueError("Dump of %s returned no sockets" % req) + for diag_msg, attrs in results: + if diag_msg.inode == inode: + return diag_msg, attrs + raise ValueError("Dump of %s did not contain inode %d" % (req, inode)) def FindSockDiagFromFd(self, s): """Gets an InetDiagMsg from the kernel for the specified socket."""
diff --git a/net/test/sock_diag_test.py b/net/test/sock_diag_test.py index 1d57c34..c7ac0d4 100755 --- a/net/test/sock_diag_test.py +++ b/net/test/sock_diag_test.py
@@ -504,8 +504,10 @@ # to work on 3.10. if net_test.LINUX_VERSION >= (3, 18): diag_req.states = 1 << tcp_test.TCP_FIN_WAIT2 - diag_msg, attrs = self.sock_diag.FindSockInfoFromReq(diag_req) - self.assertEquals(tcp_test.TCP_FIN_WAIT2, diag_msg.state) + infos = self.sock_diag.Dump(diag_req, "") + self.assertTrue(any(diag_msg.state == tcp_test.TCP_FIN_WAIT2 + for diag_msg, attrs in infos), + "Expected to find FIN_WAIT2 socket in %s" % infos) def FindChildSockets(self, s): """Finds the SYN_RECV child sockets of a given listening socket."""