Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2677b195 authored by Hugo Benichi's avatar Hugo Benichi
Browse files

Fix spurious DHCP parse error logging. 

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
parent e2be9f4a

core/java/android/net/metrics/DhcpClientEvent.java

+2 −2
Original line number Diff line number Diff line
@@ -54,7 +54,7 @@ public class DhcpClientEvent extends IpConnectivityEvent implements Parcelable { 
        }

    };



    public static void logEvent(int eventType, String ifName, String msg) {

        IpConnectivityEvent.logEvent(eventType, new DhcpClientEvent(ifName, msg));

    public static void logStateEvent(String ifName, String state) {

        logEvent(IpConnectivityEvent.IPCE_DHCP_STATE_CHANGE, new DhcpClientEvent(ifName, state));

    }

};

core/java/android/net/metrics/DhcpErrorEvent.java

+15 −5
Original line number Diff line number Diff line
@@ -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 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):

    // byte 0: error type

    // byte 1: error subtype
@@ -56,15 +58,18 @@ public class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable { 
    // byte 3: optional code

    public final int errorCode;



    private DhcpErrorEvent(int errorCode) {

    private DhcpErrorEvent(String ifName, int errorCode) {

        this.ifName = ifName;

        this.errorCode = errorCode;

    }



    private DhcpErrorEvent(Parcel in) {

        this.ifName = in.readString();

        this.errorCode = in.readInt();

    }



    public void writeToParcel(Parcel out, int flags) {

        out.writeString(ifName);

        out.writeInt(errorCode);

    }
@@ -79,12 +84,17 @@ public class DhcpErrorEvent extends IpConnectivityEvent implements Parcelable { 
        }

    };



    public static void logEvent(int errorCode) {

        IpConnectivityEvent.logEvent(IPCE_DHCP_PARSE_ERROR, new DhcpErrorEvent(errorCode));

    public static void logParseError(String ifName, int errorCode) {

        IpConnectivityEvent.logEvent(IPCE_DHCP_PARSE_ERROR, new DhcpErrorEvent(ifName, errorCode));

    }



    public static void logEvent(int errorCode, int option) {

        logEvent((0xFFFF0000 & errorCode) | (0xFF & option));

    public static void logReceiveError(String ifName) {

        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) {

services/net/java/android/net/dhcp/DhcpClient.java

+5 −8
Original line number Diff line number Diff line
@@ -30,8 +30,8 @@ import android.net.DhcpResults; 
import android.net.InterfaceConfiguration;

import android.net.LinkAddress;

import android.net.NetworkUtils;

import android.net.metrics.IpConnectivityEvent;

import android.net.metrics.DhcpClientEvent;

import android.net.metrics.DhcpErrorEvent;

import android.os.IBinder;

import android.os.INetworkManagementService;

import android.os.Message;
@@ -358,15 +358,13 @@ public class DhcpClient extends StateMachine { 
                    if (!mStopped) {

                        Log.e(TAG, "Read error", e);

                    }

                    DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_RECV_ERROR,

                            mIfaceName, e.getMessage());

                    DhcpErrorEvent.logReceiveError(mIfaceName);

                } catch (DhcpPacket.ParseException e) {

                    Log.e(TAG, "Can't parse packet: " + e.getMessage());

                    if (PACKET_DBG) {

                        Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length));

                    }

                    DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_PARSE_ERROR, mIfaceName,

                            e.getMessage());

                    DhcpErrorEvent.logParseError(mIfaceName, e.errorCode);

                }

            }

            if (DBG) Log.d(TAG, "Receive thread stopped");
@@ -463,9 +461,8 @@ public class DhcpClient extends StateMachine { 


    abstract class LoggingState extends State {

        public void enter() {

            String msg = "Entering state " + getName();

            if (STATE_DBG) Log.d(TAG, msg);

            DhcpClientEvent.logEvent(IpConnectivityEvent.IPCE_DHCP_STATE_CHANGE, mIfaceName, msg);

            if (STATE_DBG) Log.d(TAG, "Entering state " + getName());

            DhcpClientEvent.logStateEvent(mIfaceName, getName());

        }



        private String messageName(int what) {

services/net/java/android/net/dhcp/DhcpPacket.java

+32 −31
Original line number Diff line number Diff line
@@ -709,8 +709,10 @@ abstract class DhcpPacket { 
    }



    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));

            this.errorCode = errorCode;

        }

    }
@@ -766,9 +768,8 @@ abstract class DhcpPacket { 
        // check to see if we need to parse L2, IP, and UDP encaps

        if (pktType == ENCAP_L2) {

            if (packet.remaining() < MIN_PACKET_LENGTH_L2) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L2_TOO_SHORT);

                throw new ParseException("L2 packet too short, %d < %d",

                        packet.remaining(), MIN_PACKET_LENGTH_L2);

                throw new ParseException(DhcpErrorEvent.L2_TOO_SHORT,

                        "L2 packet too short, %d < %d", packet.remaining(), MIN_PACKET_LENGTH_L2);

            }



            byte[] l2dst = new byte[6];
@@ -780,24 +781,22 @@ abstract class DhcpPacket { 
            short l2type = packet.getShort();



            if (l2type != OsConstants.ETH_P_IP) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L2_WRONG_ETH_TYPE);

                throw new ParseException("Unexpected L2 type 0x%04x, expected 0x%04x",

                        l2type, OsConstants.ETH_P_IP);

                throw new ParseException(DhcpErrorEvent.L2_WRONG_ETH_TYPE,

                        "Unexpected L2 type 0x%04x, expected 0x%04x", l2type, OsConstants.ETH_P_IP);

            }

        }



        if (pktType <= ENCAP_L3) {

            if (packet.remaining() < MIN_PACKET_LENGTH_L3) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_TOO_SHORT);

                throw new ParseException("L3 packet too short, %d < %d",

                        packet.remaining(), MIN_PACKET_LENGTH_L3);

                throw new ParseException(DhcpErrorEvent.L3_TOO_SHORT,

                        "L3 packet too short, %d < %d", packet.remaining(), MIN_PACKET_LENGTH_L3);

            }



            byte ipTypeAndLength = packet.get();

            int ipVersion = (ipTypeAndLength & 0xf0) >> 4;

            if (ipVersion != 4) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_NOT_IPV4);

                throw new ParseException("Invalid IP version %d", ipVersion);

                throw new ParseException(

                        DhcpErrorEvent.L3_NOT_IPV4, "Invalid IP version %d", ipVersion);

            }



            // System.out.println("ipType is " + ipType);
@@ -814,8 +813,8 @@ abstract class DhcpPacket { 
            ipDst = readIpAddress(packet);



            if (ipProto != IP_TYPE_UDP) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L4_NOT_UDP);

                throw new ParseException("Protocol not UDP: %d", ipProto);

                throw new ParseException(

                        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
@@ -841,15 +840,15 @@ abstract class DhcpPacket { 
                // 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

                // filter is set. http://b/26696823 .

                DhcpErrorEvent.logEvent(DhcpErrorEvent.L4_WRONG_PORT);

                throw new ParseException("Unexpected UDP ports %d->%d", udpSrcPort, udpDstPort);

                throw new ParseException(DhcpErrorEvent.L4_WRONG_PORT,

                        "Unexpected UDP ports %d->%d", udpSrcPort, udpDstPort);

            }

        }



        // 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) {

            DhcpErrorEvent.logEvent(DhcpErrorEvent.BOOTP_TOO_SHORT);

            throw new ParseException("Invalid type or BOOTP packet too short, %d < %d",

            throw new ParseException(DhcpErrorEvent.BOOTP_TOO_SHORT,

                        "Invalid type or BOOTP packet too short, %d < %d",

                        packet.remaining(), MIN_PACKET_LENGTH_BOOTP);

        }
@@ -873,8 +872,8 @@ abstract class DhcpPacket { 
            packet.get(ipv4addr);

            relayIp = (Inet4Address) Inet4Address.getByAddress(ipv4addr);

        } catch (UnknownHostException ex) {

            DhcpErrorEvent.logEvent(DhcpErrorEvent.L3_INVALID_IP);

            throw new ParseException("Invalid IPv4 address: %s", Arrays.toString(ipv4addr));

            throw new ParseException(DhcpErrorEvent.L3_INVALID_IP,

                    "Invalid IPv4 address: %s", Arrays.toString(ipv4addr));

        }



        // 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();



        if (dhcpMagicCookie != DHCP_MAGIC_COOKIE) {

            DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_BAD_MAGIC_COOKIE);

            throw new ParseException("Bad magic cookie 0x%08x, should be 0x%08x", dhcpMagicCookie,

                    DHCP_MAGIC_COOKIE);

            throw new ParseException(DhcpErrorEvent.DHCP_BAD_MAGIC_COOKIE,

                    "Bad magic cookie 0x%08x, should be 0x%08x",

                    dhcpMagicCookie, DHCP_MAGIC_COOKIE);

        }



        // parse options
@@ -1009,15 +1008,17 @@ abstract class DhcpPacket { 
                    }



                    if (expectedLen != optionLen) {

                        DhcpErrorEvent.logEvent(

                        final int errorCode = DhcpErrorEvent.errorCodeWithOption(

                                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);

                    }

                }

            } catch (BufferUnderflowException e) {

                DhcpErrorEvent.logEvent(DhcpErrorEvent.BUFFER_UNDERFLOW, optionType);

                throw new ParseException("BufferUnderflowException");

                final int errorCode = DhcpErrorEvent.errorCodeWithOption(

                        DhcpErrorEvent.BUFFER_UNDERFLOW, optionType);

                throw new ParseException(errorCode, "BufferUnderflowException");

            }

        }
@@ -1025,8 +1026,8 @@ abstract class DhcpPacket { 


        switch(dhcpType) {

            case (byte) 0xFF:

                DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_NO_MSG_TYPE);

                throw new ParseException("No DHCP message type option");

                throw new ParseException(DhcpErrorEvent.DHCP_NO_MSG_TYPE,

                        "No DHCP message type option");

            case DHCP_MESSAGE_TYPE_DISCOVER:

                newPacket = new DhcpDiscoverPacket(

                    transactionId, secs, clientMac, broadcast);
@@ -1059,8 +1060,8 @@ abstract class DhcpPacket { 
                    clientMac);

                break;

            default:

                DhcpErrorEvent.logEvent(DhcpErrorEvent.DHCP_UNKNOWN_MSG_TYPE);

                throw new ParseException("Unimplemented DHCP type %d", dhcpType);

                throw new ParseException(DhcpErrorEvent.DHCP_UNKNOWN_MSG_TYPE,

                        "Unimplemented DHCP type %d", dhcpType);

        }



        newPacket.mBroadcastAddress = bcAddr;