Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix spurious DHCP parse error logging.

Browse Source 
This patch moves the event logging of DHCP response packet parse errors
to DHCPClient in a single place. It also logs receive IO errors as
DHCPErrors instead of DHCPClientEvents.

BUG=28197345

Change-Id: I7ad666cff4d8b97915880477347fbb3f588fdb2a
This commit is contained in:
Hugo Benichi
2016-04-18 09:04:04 +09:00
parent e2be9f4a67
commit 2677b1957b
4 changed files with 54 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
core/java/android/net/metrics/DhcpClientEvent.java
View File

@@ -54,7 +54,7 @@ public class DhcpClientEvent extends IpConnectivityEvent implements Parcelable {
} }
}; };
public static void logEvent(int eventType, String ifName, String msg) { public static void logStateEvent(String ifName, String state) {
IpConnectivityEvent.logEvent(eventType, new DhcpClientEvent(ifName, msg)); logEvent(IpConnectivityEvent.IPCE_DHCP_STATE_CHANGE, new DhcpClientEvent(ifName, state));
} }
}; };

20
core/java/android/net/metrics/DhcpErrorEvent.java
View File

@@ -48,7 +48,9 @@ public class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable {
public static final int DHCP_UNKNOWN_MSG_TYPE = makeErrorCode(DHCP_ERROR, 5); public static final int DHCP_UNKNOWN_MSG_TYPE = makeErrorCode(DHCP_ERROR, 5);
public static final int BUFFER_UNDERFLOW = makeErrorCode(MISC_ERROR, 1); public static final int BUFFER_UNDERFLOW = makeErrorCode(MISC_ERROR, 1);
public static final int RECEIVE_ERROR = makeErrorCode(MISC_ERROR, 2);
public final String ifName;
// error code byte format (MSB to LSB): // error code byte format (MSB to LSB):
// byte 0: error type // byte 0: error type
// byte 1: error subtype // byte 1: error subtype
@@ -56,15 +58,18 @@ public class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable {
// byte 3: optional code // byte 3: optional code
public final int errorCode; public final int errorCode;
private DhcpErrorEvent(int errorCode) { private DhcpErrorEvent(String ifName, int errorCode) {
this.ifName = ifName;
this.errorCode = errorCode; this.errorCode = errorCode;
} }
private DhcpErrorEvent(Parcel in) { private DhcpErrorEvent(Parcel in) {
this.ifName = in.readString();
this.errorCode = in.readInt(); this.errorCode = in.readInt();
} }
public void writeToParcel(Parcel out, int flags) { public void writeToParcel(Parcel out, int flags) {
out.writeString(ifName);
out.writeInt(errorCode); out.writeInt(errorCode);
} }
@@ -79,12 +84,17 @@ public class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable {
} }
}; };
public static void logEvent(int errorCode) { public static void logParseError(String ifName, int errorCode) {
IpConnectivityEvent.logEvent(IPCE_DHCP_PARSE_ERROR, new DhcpErrorEvent(errorCode)); IpConnectivityEvent.logEvent(IPCE_DHCP_PARSE_ERROR, new DhcpErrorEvent(ifName, errorCode));
} }
public static void logEvent(int errorCode, int option) { public static void logReceiveError(String ifName) {
logEvent((0xFFFF0000 & errorCode) | (0xFF & option)); IpConnectivityEvent.logEvent(IPCE_DHCP_RECV_ERROR,
new DhcpErrorEvent(ifName, RECEIVE_ERROR));
}
public static int errorCodeWithOption(int errorCode, int option) {
return (0xFFFF0000 & errorCode) | (0xFF & option);
} }
private static int makeErrorCode(int type, int subtype) { private static int makeErrorCode(int type, int subtype) {

13
services/net/java/android/net/dhcp/DhcpClient.java
View File

@@ -30,8 +30,8 @@ import android.net.DhcpResults;
import android.net.InterfaceConfiguration; import android.net.InterfaceConfiguration;
import android.net.LinkAddress; import android.net.LinkAddress;
import android.net.NetworkUtils; import android.net.NetworkUtils;
import android.net.metrics.IpConnectivityEvent;
import android.net.metrics.DhcpClientEvent; import android.net.metrics.DhcpClientEvent;
import android.net.metrics.DhcpErrorEvent;
import android.os.IBinder; import android.os.IBinder;
import android.os.INetworkManagementService; import android.os.INetworkManagementService;
import android.os.Message; import android.os.Message;
@@ -358,15 +358,13 @@ public class DhcpClient extends StateMachine {
if (!mStopped) { if (!mStopped) {
Log.e(TAG, "Read error", e); Log.e(TAG, "Read error", e);
} }
DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_RECV_ERROR, DhcpErrorEvent.logReceiveError(mIfaceName);
mIfaceName, e.getMessage());
} catch (DhcpPacket.ParseException e) { } catch (DhcpPacket.ParseException e) {
Log.e(TAG, "Can't parse packet: " + e.getMessage()); Log.e(TAG, "Can't parse packet: " + e.getMessage());
if (PACKET_DBG) { if (PACKET_DBG) {
Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length)); Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length));
} }
DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_PARSE_ERROR, mIfaceName, DhcpErrorEvent.logParseError(mIfaceName, e.errorCode);
e.getMessage());
} }
} }
if (DBG) Log.d(TAG, "Receive thread stopped"); if (DBG) Log.d(TAG, "Receive thread stopped");
@@ -463,9 +461,8 @@ public class DhcpClient extends StateMachine {
abstract class LoggingState extends State { abstract class LoggingState extends State {
public void enter() { public void enter() {
String msg = "Entering state " + getName(); if (STATE_DBG) Log.d(TAG, "Entering state " + getName());
if (STATE_DBG) Log.d(TAG, msg); DhcpClientEvent.logStateEvent(mIfaceName, getName());
DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_STATE_CHANGE, mIfaceName, msg);
} }
private String messageName(int what) { private String messageName(int what) {

63
services/net/java/android/net/dhcp/DhcpPacket.java
View File

@@ -709,8 +709,10 @@ abstract class DhcpPacket {
} }
public static class ParseException extends Exception { public static class ParseException extends Exception {
public ParseException(String msg, Object... args) { public final int errorCode;
public ParseException(int errorCode, String msg, Object... args) {
super(String.format(msg, args)); super(String.format(msg, args));
this.errorCode = errorCode;
} }
} }
@@ -766,9 +768,8 @@ abstract class DhcpPacket {
// check to see if we need to parse L2, IP, and UDP encaps // check to see if we need to parse L2, IP, and UDP encaps
if (pktType == ENCAP_L2) { if (pktType == ENCAP_L2) {
if (packet.remaining() < MIN_PACKET_LENGTH_L2) { if (packet.remaining() < MIN_PACKET_LENGTH_L2) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L2_TOO_SHORT); throw new ParseException(DhcpErrorEvent.L2_TOO_SHORT,
throw new ParseException("L2 packet too short, %d < %d", "L2 packet too short, %d < %d", packet.remaining(), MIN_PACKET_LENGTH_L2);
packet.remaining(), MIN_PACKET_LENGTH_L2);
} }
byte[] l2dst = new byte[6]; byte[] l2dst = new byte[6];
@@ -780,24 +781,22 @@ abstract class DhcpPacket {
short l2type = packet.getShort(); short l2type = packet.getShort();
if (l2type != OsConstants.ETH_P_IP) { if (l2type != OsConstants.ETH_P_IP) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L2_WRONG_ETH_TYPE); throw new ParseException(DhcpErrorEvent.L2_WRONG_ETH_TYPE,
throw new ParseException("Unexpected L2 type 0x%04x, expected 0x%04x", "Unexpected L2 type 0x%04x, expected 0x%04x", l2type, OsConstants.ETH_P_IP);
l2type, OsConstants.ETH_P_IP);
} }
} }
if (pktType <= ENCAP_L3) { if (pktType <= ENCAP_L3) {
if (packet.remaining() < MIN_PACKET_LENGTH_L3) { if (packet.remaining() < MIN_PACKET_LENGTH_L3) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_TOO_SHORT); throw new ParseException(DhcpErrorEvent.L3_TOO_SHORT,
throw new ParseException("L3 packet too short, %d < %d", "L3 packet too short, %d < %d", packet.remaining(), MIN_PACKET_LENGTH_L3);
packet.remaining(), MIN_PACKET_LENGTH_L3);
} }
byte ipTypeAndLength = packet.get(); byte ipTypeAndLength = packet.get();
int ipVersion = (ipTypeAndLength & 0xf0) >> 4; int ipVersion = (ipTypeAndLength & 0xf0) >> 4;
if (ipVersion != 4) { if (ipVersion != 4) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_NOT_IPV4); throw new ParseException(
throw new ParseException("Invalid IP version %d", ipVersion); DhcpErrorEvent.L3_NOT_IPV4, "Invalid IP version %d", ipVersion);
} }
// System.out.println("ipType is " + ipType); // System.out.println("ipType is " + ipType);
@@ -814,8 +813,8 @@ abstract class DhcpPacket {
ipDst = readIpAddress(packet); ipDst = readIpAddress(packet);
if (ipProto != IP_TYPE_UDP) { if (ipProto != IP_TYPE_UDP) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L4_NOT_UDP); throw new ParseException(
throw new ParseException("Protocol not UDP: %d", ipProto); DhcpErrorEvent.L4_NOT_UDP, "Protocol not UDP: %d", ipProto);
} }
// Skip options. This cannot cause us to read beyond the end of the buffer because the // Skip options. This cannot cause us to read beyond the end of the buffer because the
@@ -841,15 +840,15 @@ abstract class DhcpPacket {
// socket to drop packets that don't have the right source ports. However, it's // socket to drop packets that don't have the right source ports. However, it's
// possible that a packet arrives between when the socket is bound and when the // possible that a packet arrives between when the socket is bound and when the
// filter is set. http://b/26696823 . // filter is set. http://b/26696823 .
DhcpErrorEvent.logEvent(DhcpErrorEvent.L4_WRONG_PORT); throw new ParseException(DhcpErrorEvent.L4_WRONG_PORT,
throw new ParseException("Unexpected UDP ports %d->%d", udpSrcPort, udpDstPort); "Unexpected UDP ports %d->%d", udpSrcPort, udpDstPort);
} }
} }
// We need to check the length even for ENCAP_L3 because the IPv4 header is variable-length. // We need to check the length even for ENCAP_L3 because the IPv4 header is variable-length.
if (pktType > ENCAP_BOOTP || packet.remaining() < MIN_PACKET_LENGTH_BOOTP) { if (pktType > ENCAP_BOOTP || packet.remaining() < MIN_PACKET_LENGTH_BOOTP) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.BOOTP_TOO_SHORT); throw new ParseException(DhcpErrorEvent.BOOTP_TOO_SHORT,
throw new ParseException("Invalid type or BOOTP packet too short, %d < %d", "Invalid type or BOOTP packet too short, %d < %d",
packet.remaining(), MIN_PACKET_LENGTH_BOOTP); packet.remaining(), MIN_PACKET_LENGTH_BOOTP);
} }
@@ -873,8 +872,8 @@ abstract class DhcpPacket {
packet.get(ipv4addr); packet.get(ipv4addr);
relayIp = (Inet4Address) Inet4Address.getByAddress(ipv4addr); relayIp = (Inet4Address) Inet4Address.getByAddress(ipv4addr);
} catch (UnknownHostException ex) { } catch (UnknownHostException ex) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_INVALID_IP); throw new ParseException(DhcpErrorEvent.L3_INVALID_IP,
throw new ParseException("Invalid IPv4 address: %s", Arrays.toString(ipv4addr)); "Invalid IPv4 address: %s", Arrays.toString(ipv4addr));
} }
// Some DHCP servers have been known to announce invalid client hardware address values such // Some DHCP servers have been known to announce invalid client hardware address values such
@@ -898,9 +897,9 @@ abstract class DhcpPacket {
int dhcpMagicCookie = packet.getInt(); int dhcpMagicCookie = packet.getInt();
if (dhcpMagicCookie != DHCP_MAGIC_COOKIE) { if (dhcpMagicCookie != DHCP_MAGIC_COOKIE) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_BAD_MAGIC_COOKIE); throw new ParseException(DhcpErrorEvent.DHCP_BAD_MAGIC_COOKIE,
throw new ParseException("Bad magic cookie 0x%08x, should be 0x%08x", dhcpMagicCookie, "Bad magic cookie 0x%08x, should be 0x%08x",
DHCP_MAGIC_COOKIE); dhcpMagicCookie, DHCP_MAGIC_COOKIE);
} }
// parse options // parse options
@@ -1009,15 +1008,17 @@ abstract class DhcpPacket {
} }
if (expectedLen != optionLen) { if (expectedLen != optionLen) {
DhcpErrorEvent.logEvent( final int errorCode = DhcpErrorEvent.errorCodeWithOption(
DhcpErrorEvent.DHCP_INVALID_OPTION_LENGTH, optionType); DhcpErrorEvent.DHCP_INVALID_OPTION_LENGTH, optionType);
throw new ParseException("Invalid length %d for option %d, expected %d", throw new ParseException(errorCode,
"Invalid length %d for option %d, expected %d",
optionLen, optionType, expectedLen); optionLen, optionType, expectedLen);
} }
} }
} catch (BufferUnderflowException e) { } catch (BufferUnderflowException e) {
DhcpErrorEvent.logEvent(DhcpErrorEvent.BUFFER_UNDERFLOW, optionType); final int errorCode = DhcpErrorEvent.errorCodeWithOption(
throw new ParseException("BufferUnderflowException"); DhcpErrorEvent.BUFFER_UNDERFLOW, optionType);
throw new ParseException(errorCode, "BufferUnderflowException");
} }
} }
@@ -1025,8 +1026,8 @@ abstract class DhcpPacket {
switch(dhcpType) { switch(dhcpType) {
case (byte) 0xFF: case (byte) 0xFF:
DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_NO_MSG_TYPE); throw new ParseException(DhcpErrorEvent.DHCP_NO_MSG_TYPE,
throw new ParseException("No DHCP message type option"); "No DHCP message type option");
case DHCP_MESSAGE_TYPE_DISCOVER: case DHCP_MESSAGE_TYPE_DISCOVER:
newPacket = new DhcpDiscoverPacket( newPacket = new DhcpDiscoverPacket(
transactionId, secs, clientMac, broadcast); transactionId, secs, clientMac, broadcast);
@@ -1059,8 +1060,8 @@ abstract class DhcpPacket {
clientMac); clientMac);
break; break;
default: default:
DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_UNKNOWN_MSG_TYPE); throw new ParseException(DhcpErrorEvent.DHCP_UNKNOWN_MSG_TYPE,
throw new ParseException("Unimplemented DHCP type %d", dhcpType); "Unimplemented DHCP type %d", dhcpType);
} }
newPacket.mBroadcastAddress = bcAddr; newPacket.mBroadcastAddress = bcAddr;