From d42fca678e0f030018e32a876ca4cf46cd519a57 Mon Sep 17 00:00:00 2001 From: markchien Date: Tue, 19 Mar 2019 21:25:33 +0800 Subject: [PATCH] Replace TcpSocketInfo with similar structure Replace TcpSocketInfo with TcpKeepalivePacketDataParcelable because their structures are very similar. bug: 128882321 Test: -build, flash, boot -FrameworksNetTests Change-Id: Iafb4031a64ba4775a495c156e2c997d890c6b261 --- .../net/TcpKeepalivePacketDataParcelable.aidl | 2 + .../connectivity/TcpKeepaliveController.java | 41 ++++++------ .../android/net/TcpKeepalivePacketData.java | 62 +++++++------------ .../net/TcpKeepalivePacketDataTest.java | 48 ++++++++------ 4 files changed, 72 insertions(+), 81 deletions(-) diff --git a/core/java/android/net/TcpKeepalivePacketDataParcelable.aidl b/core/java/android/net/TcpKeepalivePacketDataParcelable.aidl index 7329c63b09bec..d66b6ae3ab54e 100644 --- a/core/java/android/net/TcpKeepalivePacketDataParcelable.aidl +++ b/core/java/android/net/TcpKeepalivePacketDataParcelable.aidl @@ -23,4 +23,6 @@ parcelable TcpKeepalivePacketDataParcelable { int dstPort; int seq; int ack; + int rcvWnd; + int rcvWndScale; } diff --git a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java b/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java index e78d25a1601b0..f4d9006a70682 100644 --- a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java +++ b/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java @@ -31,7 +31,7 @@ import android.net.NetworkUtils; import android.net.SocketKeepalive.InvalidPacketException; import android.net.SocketKeepalive.InvalidSocketException; import android.net.TcpKeepalivePacketData; -import android.net.TcpKeepalivePacketData.TcpSocketInfo; +import android.net.TcpKeepalivePacketDataParcelable; import android.net.TcpRepairWindow; import android.os.Handler; import android.os.MessageQueue; @@ -46,7 +46,6 @@ import com.android.internal.annotations.GuardedBy; import com.android.server.connectivity.KeepaliveTracker.KeepaliveInfo; import java.io.FileDescriptor; -import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.SocketException; @@ -109,8 +108,8 @@ public class TcpKeepaliveController { public static TcpKeepalivePacketData getTcpKeepalivePacket(@NonNull FileDescriptor fd) throws InvalidPacketException, InvalidSocketException { try { - final TcpSocketInfo tsi = switchToRepairMode(fd); - return TcpKeepalivePacketData.tcpKeepalivePacket(tsi); + final TcpKeepalivePacketDataParcelable tcpDetails = switchToRepairMode(fd); + return TcpKeepalivePacketData.tcpKeepalivePacket(tcpDetails); } catch (InvalidPacketException | InvalidSocketException e) { switchOutOfRepairMode(fd); throw e; @@ -120,20 +119,15 @@ public class TcpKeepaliveController { * Switch the tcp socket to repair mode and query detail tcp information. * * @param fd the fd of socket on which to use keepalive offload. - * @return a {@link TcpKeepalivePacketData#TcpSocketInfo} object for current + * @return a {@link TcpKeepalivePacketData#TcpKeepalivePacketDataParcelable} object for current * tcp/ip information. */ - private static TcpSocketInfo switchToRepairMode(FileDescriptor fd) + private static TcpKeepalivePacketDataParcelable switchToRepairMode(FileDescriptor fd) throws InvalidSocketException { if (DBG) Log.i(TAG, "switchToRepairMode to start tcp keepalive : " + fd); + final TcpKeepalivePacketDataParcelable tcpDetails = new TcpKeepalivePacketDataParcelable(); final SocketAddress srcSockAddr; final SocketAddress dstSockAddr; - final InetAddress srcAddress; - final InetAddress dstAddress; - final int srcPort; - final int dstPort; - int seq; - final int ack; final TcpRepairWindow trw; // Query source address and port. @@ -144,8 +138,8 @@ public class TcpKeepaliveController { throw new InvalidSocketException(ERROR_INVALID_SOCKET, e); } if (srcSockAddr instanceof InetSocketAddress) { - srcAddress = getAddress((InetSocketAddress) srcSockAddr); - srcPort = getPort((InetSocketAddress) srcSockAddr); + tcpDetails.srcAddress = getAddress((InetSocketAddress) srcSockAddr); + tcpDetails.srcPort = getPort((InetSocketAddress) srcSockAddr); } else { Log.e(TAG, "Invalid or mismatched SocketAddress"); throw new InvalidSocketException(ERROR_INVALID_SOCKET); @@ -158,8 +152,8 @@ public class TcpKeepaliveController { throw new InvalidSocketException(ERROR_INVALID_SOCKET, e); } if (dstSockAddr instanceof InetSocketAddress) { - dstAddress = getAddress((InetSocketAddress) dstSockAddr); - dstPort = getPort((InetSocketAddress) dstSockAddr); + tcpDetails.dstAddress = getAddress((InetSocketAddress) dstSockAddr); + tcpDetails.dstPort = getPort((InetSocketAddress) dstSockAddr); } else { Log.e(TAG, "Invalid or mismatched peer SocketAddress"); throw new InvalidSocketException(ERROR_INVALID_SOCKET); @@ -178,10 +172,10 @@ public class TcpKeepaliveController { } // Query write sequence number from SEND_QUEUE. Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR_QUEUE, TCP_SEND_QUEUE); - seq = Os.getsockoptInt(fd, IPPROTO_TCP, TCP_QUEUE_SEQ); + tcpDetails.seq = Os.getsockoptInt(fd, IPPROTO_TCP, TCP_QUEUE_SEQ); // Query read sequence number from RECV_QUEUE. Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR_QUEUE, TCP_RECV_QUEUE); - ack = Os.getsockoptInt(fd, IPPROTO_TCP, TCP_QUEUE_SEQ); + tcpDetails.ack = Os.getsockoptInt(fd, IPPROTO_TCP, TCP_QUEUE_SEQ); // Switch to NO_QUEUE to prevent illegal socket read/write in repair mode. Os.setsockoptInt(fd, IPPROTO_TCP, TCP_REPAIR_QUEUE, TCP_NO_QUEUE); // Finally, check if socket is still idle. TODO : this check needs to move to @@ -197,6 +191,8 @@ public class TcpKeepaliveController { // Query tcp window size. trw = NetworkUtils.getTcpRepairWindow(fd); + tcpDetails.rcvWnd = trw.rcvWnd; + tcpDetails.rcvWndScale = trw.rcvWndScale; } catch (ErrnoException e) { Log.e(TAG, "Exception reading TCP state from socket", e); if (e.errno == ENOPROTOOPT) { @@ -212,10 +208,9 @@ public class TcpKeepaliveController { // Keepalive sequence number is last sequence number - 1. If it couldn't be retrieved, // then it must be set to -1, so decrement in all cases. - seq = seq - 1; + tcpDetails.seq = tcpDetails.seq - 1; - return new TcpSocketInfo(srcAddress, srcPort, dstAddress, dstPort, seq, ack, trw.rcvWnd, - trw.rcvWndScale); + return tcpDetails; } /** @@ -287,8 +282,8 @@ public class TcpKeepaliveController { switchOutOfRepairMode(fd); } - private static InetAddress getAddress(InetSocketAddress inetAddr) { - return inetAddr.getAddress(); + private static byte [] getAddress(InetSocketAddress inetAddr) { + return inetAddr.getAddress().getAddress(); } private static int getPort(InetSocketAddress inetAddr) { diff --git a/services/net/java/android/net/TcpKeepalivePacketData.java b/services/net/java/android/net/TcpKeepalivePacketData.java index 398a6b31cbce3..d79ad1fe41a9f 100644 --- a/services/net/java/android/net/TcpKeepalivePacketData.java +++ b/services/net/java/android/net/TcpKeepalivePacketData.java @@ -25,8 +25,8 @@ import android.os.Parcel; import android.os.Parcelable; import android.system.OsConstants; -import java.net.Inet4Address; import java.net.InetAddress; +import java.net.UnknownHostException; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.Objects; @@ -56,10 +56,10 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce // This should only be constructed via static factory methods, such as // tcpKeepalivePacket. - private TcpKeepalivePacketData(TcpSocketInfo tcpDetails, byte[] data) - throws InvalidPacketException { - super(tcpDetails.srcAddress, tcpDetails.srcPort, tcpDetails.dstAddress, - tcpDetails.dstPort, data); + private TcpKeepalivePacketData(final TcpKeepalivePacketDataParcelable tcpDetails, + final byte[] data) throws InvalidPacketException, UnknownHostException { + super(InetAddress.getByAddress(tcpDetails.srcAddress), tcpDetails.srcPort, + InetAddress.getByAddress(tcpDetails.dstAddress), tcpDetails.dstPort, data); tcpSeq = tcpDetails.seq; tcpAck = tcpDetails.ack; // In the packet, the window is shifted right by the window scale. @@ -71,17 +71,22 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce * Factory method to create tcp keepalive packet structure. */ public static TcpKeepalivePacketData tcpKeepalivePacket( - TcpSocketInfo tcpDetails) throws InvalidPacketException { + TcpKeepalivePacketDataParcelable tcpDetails) throws InvalidPacketException { final byte[] packet; - if ((tcpDetails.srcAddress instanceof Inet4Address) - && (tcpDetails.dstAddress instanceof Inet4Address)) { - packet = buildV4Packet(tcpDetails); - } else { - // TODO: support ipv6 + try { + if ((tcpDetails.srcAddress != null) && (tcpDetails.dstAddress != null) + && (tcpDetails.srcAddress.length == 4 /* V4 IP length */) + && (tcpDetails.dstAddress.length == 4 /* V4 IP length */)) { + packet = buildV4Packet(tcpDetails); + } else { + // TODO: support ipv6 + throw new InvalidPacketException(ERROR_INVALID_IP_ADDRESS); + } + return new TcpKeepalivePacketData(tcpDetails, packet); + } catch (UnknownHostException e) { throw new InvalidPacketException(ERROR_INVALID_IP_ADDRESS); } - return new TcpKeepalivePacketData(tcpDetails, packet); } /** @@ -89,7 +94,7 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce */ // TODO : if this code is ever moved to the network stack, factorize constants with the ones // over there. - private static byte[] buildV4Packet(TcpSocketInfo tcpDetails) { + private static byte[] buildV4Packet(TcpKeepalivePacketDataParcelable tcpDetails) { final int length = IPV4_HEADER_LENGTH + TCP_HEADER_LENGTH; ByteBuffer buf = ByteBuffer.allocate(length); buf.order(ByteOrder.BIG_ENDIAN); @@ -102,8 +107,8 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce buf.put((byte) OsConstants.IPPROTO_TCP); final int ipChecksumOffset = buf.position(); buf.putShort((short) 0); // IP checksum - buf.put(tcpDetails.srcAddress.getAddress()); - buf.put(tcpDetails.dstAddress.getAddress()); + buf.put(tcpDetails.srcAddress); + buf.put(tcpDetails.dstAddress); buf.putShort((short) tcpDetails.srcPort); buf.putShort((short) tcpDetails.dstPort); buf.putInt(tcpDetails.seq); // Sequence Number @@ -122,31 +127,6 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce // TODO: add buildV6Packet. - /** Represents tcp/ip information. */ - // TODO: Replace TcpSocketInfo with TcpKeepalivePacketDataParcelable. - public static class TcpSocketInfo { - public final InetAddress srcAddress; - public final InetAddress dstAddress; - public final int srcPort; - public final int dstPort; - public final int seq; - public final int ack; - public final int rcvWnd; - public final int rcvWndScale; - - public TcpSocketInfo(InetAddress sAddr, int sPort, InetAddress dAddr, - int dPort, int writeSeq, int readSeq, int rWnd, int rWndScale) { - srcAddress = sAddr; - dstAddress = dAddr; - srcPort = sPort; - dstPort = dPort; - seq = writeSeq; - ack = readSeq; - rcvWnd = rWnd; - rcvWndScale = rWndScale; - } - } - @Override public boolean equals(@Nullable final Object o) { if (!(o instanceof TcpKeepalivePacketData)) return false; @@ -218,6 +198,8 @@ public class TcpKeepalivePacketData extends KeepalivePacketData implements Parce parcel.dstPort = dstPort; parcel.seq = tcpSeq; parcel.ack = tcpAck; + parcel.rcvWnd = tcpWnd; + parcel.rcvWndScale = tcpWndScale; return parcel; } diff --git a/tests/net/java/android/net/TcpKeepalivePacketDataTest.java b/tests/net/java/android/net/TcpKeepalivePacketDataTest.java index 1f2dd275bb7bb..372ffcd057b4b 100644 --- a/tests/net/java/android/net/TcpKeepalivePacketDataTest.java +++ b/tests/net/java/android/net/TcpKeepalivePacketDataTest.java @@ -21,12 +21,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import android.net.SocketKeepalive.InvalidPacketException; -import android.net.TcpKeepalivePacketData.TcpSocketInfo; import com.android.internal.util.TestUtils; -import libcore.net.InetAddressUtils; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -37,14 +34,14 @@ import java.nio.ByteBuffer; @RunWith(JUnit4.class) public final class TcpKeepalivePacketDataTest { + private static final byte[] IPV4_KEEPALIVE_SRC_ADDR = {10, 0, 0, 1}; + private static final byte[] IPV4_KEEPALIVE_DST_ADDR = {10, 0, 0, 5}; @Before public void setUp() {} @Test - public void testV4TcpKeepalivePacket() { - final InetAddress srcAddr = InetAddressUtils.parseNumericAddress("192.168.0.1"); - final InetAddress dstAddr = InetAddressUtils.parseNumericAddress("192.168.0.10"); + public void testV4TcpKeepalivePacket() throws Exception { final int srcPort = 1234; final int dstPort = 4321; final int seq = 0x11111111; @@ -52,20 +49,28 @@ public final class TcpKeepalivePacketDataTest { final int wnd = 8000; final int wndScale = 2; TcpKeepalivePacketData resultData = null; - TcpSocketInfo testInfo = new TcpSocketInfo( - srcAddr, srcPort, dstAddr, dstPort, seq, ack, wnd, wndScale); + final TcpKeepalivePacketDataParcelable testInfo = new TcpKeepalivePacketDataParcelable(); + testInfo.srcAddress = IPV4_KEEPALIVE_SRC_ADDR; + testInfo.srcPort = srcPort; + testInfo.dstAddress = IPV4_KEEPALIVE_DST_ADDR; + testInfo.dstPort = dstPort; + testInfo.seq = seq; + testInfo.ack = ack; + testInfo.rcvWnd = wnd; + testInfo.rcvWndScale = wndScale; try { resultData = TcpKeepalivePacketData.tcpKeepalivePacket(testInfo); } catch (InvalidPacketException e) { fail("InvalidPacketException: " + e); } - assertEquals(testInfo.srcAddress, resultData.srcAddress); - assertEquals(testInfo.dstAddress, resultData.dstAddress); + assertEquals(InetAddress.getByAddress(testInfo.srcAddress), resultData.srcAddress); + assertEquals(InetAddress.getByAddress(testInfo.dstAddress), resultData.dstAddress); assertEquals(testInfo.srcPort, resultData.srcPort); assertEquals(testInfo.dstPort, resultData.dstPort); assertEquals(testInfo.seq, resultData.tcpSeq); assertEquals(testInfo.ack, resultData.tcpAck); + assertEquals(testInfo.rcvWnd, resultData.tcpWnd); assertEquals(testInfo.rcvWndScale, resultData.tcpWndScale); TestUtils.assertParcelingIsLossless(resultData, TcpKeepalivePacketData.CREATOR); @@ -78,11 +83,11 @@ public final class TcpKeepalivePacketDataTest { byte[] ip = new byte[4]; buf = ByteBuffer.wrap(packet, 12, 4); buf.get(ip); - assertArrayEquals(ip, srcAddr.getAddress()); + assertArrayEquals(ip, IPV4_KEEPALIVE_SRC_ADDR); // Destination IP address. buf = ByteBuffer.wrap(packet, 16, 4); buf.get(ip); - assertArrayEquals(ip, dstAddr.getAddress()); + assertArrayEquals(ip, IPV4_KEEPALIVE_DST_ADDR); buf = ByteBuffer.wrap(packet, 20, 12); // Source port. @@ -102,25 +107,32 @@ public final class TcpKeepalivePacketDataTest { @Test public void testParcel() throws Exception { - final InetAddress srcAddr = InetAddresses.parseNumericAddress("192.168.0.1"); - final InetAddress dstAddr = InetAddresses.parseNumericAddress("192.168.0.10"); final int srcPort = 1234; final int dstPort = 4321; final int sequence = 0x11111111; final int ack = 0x22222222; final int wnd = 48_000; final int wndScale = 2; + final TcpKeepalivePacketDataParcelable testInfo = new TcpKeepalivePacketDataParcelable(); + testInfo.srcAddress = IPV4_KEEPALIVE_SRC_ADDR; + testInfo.srcPort = srcPort; + testInfo.dstAddress = IPV4_KEEPALIVE_DST_ADDR; + testInfo.dstPort = dstPort; + testInfo.seq = sequence; + testInfo.ack = ack; + testInfo.rcvWnd = wnd; + testInfo.rcvWndScale = wndScale; TcpKeepalivePacketData testData = null; TcpKeepalivePacketDataParcelable resultData = null; - TcpSocketInfo testInfo = new TcpSocketInfo( - srcAddr, srcPort, dstAddr, dstPort, sequence, ack, wnd, wndScale); testData = TcpKeepalivePacketData.tcpKeepalivePacket(testInfo); resultData = testData.toStableParcelable(); - assertArrayEquals(resultData.srcAddress, srcAddr.getAddress()); - assertArrayEquals(resultData.dstAddress, dstAddr.getAddress()); + assertArrayEquals(resultData.srcAddress, IPV4_KEEPALIVE_SRC_ADDR); + assertArrayEquals(resultData.dstAddress, IPV4_KEEPALIVE_DST_ADDR); assertEquals(resultData.srcPort, srcPort); assertEquals(resultData.dstPort, dstPort); assertEquals(resultData.seq, sequence); assertEquals(resultData.ack, ack); + assertEquals(resultData.rcvWnd, wnd); + assertEquals(resultData.rcvWndScale, wndScale); } }