From 77fdf23d6f7831c09fc43d206b346e273ebe266a Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 31 Mar 2016 16:20:22 +0900 Subject: [PATCH] Honour the DHCP MTU option. Bug: 25590369 Bug: 27719279 Change-Id: I473b5ec9c13f0d74e1ddd55e9d4abcff374a83ef --- core/java/android/net/DhcpResults.java | 24 +++--- .../net/java/android/net/dhcp/DhcpPacket.java | 15 +++- .../net/java/android/net/ip/IpManager.java | 4 + .../src/android/net/dhcp/DhcpPacketTest.java | 77 +++++++++++++++++-- 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/core/java/android/net/DhcpResults.java b/core/java/android/net/DhcpResults.java index 97bd5d284c711..8c5f60395ca94 100644 --- a/core/java/android/net/DhcpResults.java +++ b/core/java/android/net/DhcpResults.java @@ -40,6 +40,9 @@ public class DhcpResults extends StaticIpConfiguration { public int leaseDuration; + /** Link MTU option. 0 means unset. */ + public int mtu; + public DhcpResults() { super(); } @@ -57,19 +60,7 @@ public class DhcpResults extends StaticIpConfiguration { serverAddress = source.serverAddress; vendorInfo = source.vendorInfo; leaseDuration = source.leaseDuration; - } - } - - /** - * Updates the DHCP fields that need to be retained from - * original DHCP request if the current renewal shows them - * being empty. - */ - public void updateFromDhcpRequest(DhcpResults orig) { - if (orig == null) return; - if (gateway == null) gateway = orig.gateway; - if (dnsServers.size() == 0) { - dnsServers.addAll(orig.dnsServers); + mtu = source.mtu; } } @@ -89,6 +80,7 @@ public class DhcpResults extends StaticIpConfiguration { super.clear(); vendorInfo = null; leaseDuration = 0; + mtu = 0; } @Override @@ -98,6 +90,7 @@ public class DhcpResults extends StaticIpConfiguration { str.append(" DHCP server ").append(serverAddress); str.append(" Vendor info ").append(vendorInfo); str.append(" lease ").append(leaseDuration).append(" seconds"); + if (mtu != 0) str.append(" MTU ").append(mtu); return str.toString(); } @@ -113,7 +106,8 @@ public class DhcpResults extends StaticIpConfiguration { return super.equals((StaticIpConfiguration) obj) && Objects.equals(serverAddress, target.serverAddress) && Objects.equals(vendorInfo, target.vendorInfo) && - leaseDuration == target.leaseDuration; + leaseDuration == target.leaseDuration && + mtu == target.mtu; } /** Implement the Parcelable interface */ @@ -134,6 +128,7 @@ public class DhcpResults extends StaticIpConfiguration { public void writeToParcel(Parcel dest, int flags) { super.writeToParcel(dest, flags); dest.writeInt(leaseDuration); + dest.writeInt(mtu); NetworkUtils.parcelInetAddress(dest, serverAddress, flags); dest.writeString(vendorInfo); } @@ -141,6 +136,7 @@ public class DhcpResults extends StaticIpConfiguration { private static void readFromParcel(DhcpResults dhcpResults, Parcel in) { StaticIpConfiguration.readFromParcel(dhcpResults, in); dhcpResults.leaseDuration = in.readInt(); + dhcpResults.mtu = in.readInt(); dhcpResults.serverAddress = (Inet4Address) NetworkUtils.unparcelInetAddress(in); dhcpResults.vendorInfo = in.readString(); } diff --git a/services/net/java/android/net/dhcp/DhcpPacket.java b/services/net/java/android/net/dhcp/DhcpPacket.java index e27f69efbe83b..a8814088d489e 100644 --- a/services/net/java/android/net/dhcp/DhcpPacket.java +++ b/services/net/java/android/net/dhcp/DhcpPacket.java @@ -57,6 +57,17 @@ abstract class DhcpPacket { public static final int HWADDR_LEN = 16; public static final int MAX_OPTION_LEN = 255; + + /** + * The minimum and maximum MTU that we are prepared to use. We set the minimum to the minimum + * IPv6 MTU because the IPv6 stack enters unusual codepaths when the link MTU drops below 1280, + * and does not recover if the MTU is brought above 1280 again. We set the maximum to 1500 + * because in general it is risky to assume that the hardware is able to send/receive packets + * larger than 1500 bytes even if the network supports it. + */ + private static final int MIN_MTU = 1280; + private static final int MAX_MTU = 1500; + /** * IP layer definitions. */ @@ -917,7 +928,7 @@ abstract class DhcpPacket { break; case DHCP_MTU: expectedLen = 2; - mtu = Short.valueOf(packet.getShort()); + mtu = packet.getShort(); break; case DHCP_DOMAIN_NAME: expectedLen = optionLen; @@ -1106,6 +1117,8 @@ abstract class DhcpPacket { results.serverAddress = mServerIdentifier; results.vendorInfo = mVendorInfo; results.leaseDuration = (mLeaseTime != null) ? mLeaseTime : INFINITE_LEASE; + results.mtu = (mMtu != null && MIN_MTU <= mMtu && mMtu <= MAX_MTU) ? mMtu : 0; + return results; } diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java index c25fae306c39b..d10834ad96956 100644 --- a/services/net/java/android/net/ip/IpManager.java +++ b/services/net/java/android/net/ip/IpManager.java @@ -605,6 +605,10 @@ public class IpManager extends StateMachine { } } newLp.setDomains(mDhcpResults.domains); + + if (mDhcpResults.mtu != 0) { + newLp.setMtu(mDhcpResults.mtu); + } } // [4] Add in TCP buffer sizes and HTTP Proxy config, if available. diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java index 876d95b7fa501..f8eaf7d0ee266 100644 --- a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java +++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java @@ -261,7 +261,7 @@ public class DhcpPacketTest extends TestCase { private void assertDhcpResults(String ipAddress, String gateway, String dnsServersString, String domains, String serverAddress, String vendorInfo, int leaseDuration, - boolean hasMeteredHint, DhcpResults dhcpResults) throws Exception { + boolean hasMeteredHint, int mtu, DhcpResults dhcpResults) throws Exception { assertEquals(new LinkAddress(ipAddress), dhcpResults.ipAddress); assertEquals(v4Address(gateway), dhcpResults.gateway); @@ -277,6 +277,7 @@ public class DhcpPacketTest extends TestCase { assertEquals(vendorInfo, dhcpResults.vendorInfo); assertEquals(leaseDuration, dhcpResults.leaseDuration); assertEquals(hasMeteredHint, dhcpResults.hasMeteredHint()); + assertEquals(mtu, dhcpResults.mtu); } @SmallTest @@ -310,7 +311,7 @@ public class DhcpPacketTest extends TestCase { assertTrue(offerPacket instanceof DhcpOfferPacket); // Implicitly checks it's non-null. DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("192.168.159.247/20", "192.168.159.254", "8.8.8.8,8.8.4.4", - null, "192.168.144.3", null, 7200, false, dhcpResults); + null, "192.168.144.3", null, 7200, false, 0, dhcpResults); } @SmallTest @@ -342,10 +343,70 @@ public class DhcpPacketTest extends TestCase { assertTrue(offerPacket instanceof DhcpOfferPacket); // Implicitly checks it's non-null. DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("192.168.43.247/24", "192.168.43.1", "192.168.43.1", - null, "192.168.43.1", "ANDROID_METERED", 3600, true, dhcpResults); + null, "192.168.43.1", "ANDROID_METERED", 3600, true, 0, dhcpResults); assertTrue(dhcpResults.hasMeteredHint()); } + private byte[] mtuBytes(int mtu) { + // 0x1a02: option 26, length 2. 0xff: no more options. + if (mtu > Short.MAX_VALUE - Short.MIN_VALUE) { + throw new IllegalArgumentException( + String.format("Invalid MTU %d, must be 16-bit unsigned", mtu)); + } + String hexString = String.format("1a02%04xff", mtu); + return HexEncoding.decode(hexString.toCharArray(), false); + } + + private void checkMtu(ByteBuffer packet, int expectedMtu, byte[] mtuBytes) throws Exception { + if (mtuBytes != null) { + packet.position(packet.capacity() - mtuBytes.length); + packet.put(mtuBytes); + packet.clear(); + } + DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3); + assertTrue(offerPacket instanceof DhcpOfferPacket); // Implicitly checks it's non-null. + DhcpResults dhcpResults = offerPacket.toDhcpResults(); + assertDhcpResults("192.168.159.247/20", "192.168.159.254", "8.8.8.8,8.8.4.4", + null, "192.168.144.3", null, 7200, false, expectedMtu, dhcpResults); + } + + @SmallTest + public void testMtu() throws Exception { + final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode(( + // IP header. + "451001480000000080118849c0a89003c0a89ff7" + + // UDP header. + "004300440134dcfa" + + // BOOTP header. + "02010600c997a63b0000000000000000c0a89ff70000000000000000" + + // MAC address. + "30766ff2a90c00000000000000000000" + + // Server name. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // File. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // Options + "638253633501023604c0a89003330400001c200104fffff0000304c0a89ffe06080808080808080404" + + "3a0400000e103b040000189cff00000000" + ).toCharArray(), false)); + + checkMtu(packet, 0, null); + checkMtu(packet, 0, mtuBytes(1501)); + checkMtu(packet, 1500, mtuBytes(1500)); + checkMtu(packet, 1499, mtuBytes(1499)); + checkMtu(packet, 1280, mtuBytes(1280)); + checkMtu(packet, 0, mtuBytes(1279)); + checkMtu(packet, 0, mtuBytes(576)); + checkMtu(packet, 0, mtuBytes(68)); + checkMtu(packet, 0, mtuBytes(Short.MIN_VALUE)); + checkMtu(packet, 0, mtuBytes(Short.MAX_VALUE + 3)); + checkMtu(packet, 0, mtuBytes(-1)); + } + @SmallTest public void testBadHwaddrLength() throws Exception { final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode(( @@ -453,7 +514,7 @@ public class DhcpPacketTest extends TestCase { assertTrue(offerPacket instanceof DhcpOfferPacket); DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("172.17.152.118/16", "172.17.1.1", "172.17.1.1", - null, "1.1.1.1", null, 43200, false, dhcpResults); + null, "1.1.1.1", null, 43200, false, 0, dhcpResults); } @SmallTest @@ -484,7 +545,7 @@ public class DhcpPacketTest extends TestCase { assertTrue(offerPacket instanceof DhcpOfferPacket); DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("10.63.93.4/20", "10.63.80.1", "192.0.2.1,192.0.2.2", - "domain123.co.uk", "192.0.2.254", null, 49094, false, dhcpResults); + "domain123.co.uk", "192.0.2.254", null, 49094, false, 0, dhcpResults); } @SmallTest @@ -518,7 +579,7 @@ public class DhcpPacketTest extends TestCase { assertEquals("BCF5AC000000", HexDump.toHexString(offerPacket.getClientMac())); DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("10.32.158.205/20", "10.32.144.1", "148.88.65.52,148.88.65.53", - "lancs.ac.uk", "10.32.255.128", null, 7200, false, dhcpResults); + "lancs.ac.uk", "10.32.255.128", null, 7200, false, 0, dhcpResults); } @SmallTest @@ -554,7 +615,7 @@ public class DhcpPacketTest extends TestCase { DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("10.15.122.242/16", "10.15.200.23", "209.129.128.3,209.129.148.3,209.129.128.6", - "wvm.edu", "10.1.105.252", null, 86400, false, dhcpResults); + "wvm.edu", "10.1.105.252", null, 86400, false, 0, dhcpResults); } @SmallTest @@ -621,7 +682,7 @@ public class DhcpPacketTest extends TestCase { assertEquals("FC3D93000000", HexDump.toHexString(offerPacket.getClientMac())); DhcpResults dhcpResults = offerPacket.toDhcpResults(); assertDhcpResults("192.168.189.49/24", "192.168.189.1", "8.8.8.8,8.8.4.4", - null, "192.171.189.2", null, 28800, false, dhcpResults); + null, "192.171.189.2", null, 28800, false, 0, dhcpResults); } @SmallTest