From 8409a1c297bf9dd2a1cf183bfe03f03f47659f67 Mon Sep 17 00:00:00 2001 From: markchien Date: Fri, 21 May 2021 15:11:35 +0800 Subject: [PATCH 1/2] Remove *Locked wording Since tethering run in single thread without any synchronized lock, remove "Locked" wording from the methods which the use lock before. Bug: 162920185 Test: atest TetheringCoverageTest Change-Id: Ia8c057800cab30cdcbacae4db2d706b6427e9861 Merged-In: Ia8c057800cab30cdcbacae4db2d706b6427e9861 --- .../networkstack/tethering/Tethering.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 759638083f..e4fbd71e56 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -495,11 +495,11 @@ public class Tethering { // See NetlinkHandler.cpp: notifyInterfaceChanged. if (VDBG) Log.d(TAG, "interfaceStatusChanged " + iface + ", " + up); if (up) { - maybeTrackNewInterfaceLocked(iface); + maybeTrackNewInterface(iface); } else { if (ifaceNameToType(iface) == TETHERING_BLUETOOTH || ifaceNameToType(iface) == TETHERING_WIGIG) { - stopTrackingInterfaceLocked(iface); + stopTrackingInterface(iface); } else { // Ignore usb0 down after enabling RNDIS. // We will handle disconnect in interfaceRemoved. @@ -535,12 +535,12 @@ public class Tethering { void interfaceAdded(String iface) { if (VDBG) Log.d(TAG, "interfaceAdded " + iface); - maybeTrackNewInterfaceLocked(iface); + maybeTrackNewInterface(iface); } void interfaceRemoved(String iface) { if (VDBG) Log.d(TAG, "interfaceRemoved " + iface); - stopTrackingInterfaceLocked(iface); + stopTrackingInterface(iface); } void startTethering(final TetheringRequestParcel request, final IIntResultListener listener) { @@ -694,14 +694,14 @@ public class Tethering { mEthernetCallback = new EthernetCallback(); mEthernetIfaceRequest = em.requestTetheredInterface(mExecutor, mEthernetCallback); } else { - stopEthernetTetheringLocked(); + stopEthernetTethering(); } return TETHER_ERROR_NO_ERROR; } - private void stopEthernetTetheringLocked() { + private void stopEthernetTethering() { if (mConfiguredEthernetIface != null) { - stopTrackingInterfaceLocked(mConfiguredEthernetIface); + stopTrackingInterface(mConfiguredEthernetIface); mConfiguredEthernetIface = null; } if (mEthernetCallback != null) { @@ -718,7 +718,7 @@ public class Tethering { // Ethernet callback arrived after Ethernet tethering stopped. Ignore. return; } - maybeTrackNewInterfaceLocked(iface, TETHERING_ETHERNET); + maybeTrackNewInterface(iface, TETHERING_ETHERNET); changeInterfaceState(iface, getRequestedState(TETHERING_ETHERNET)); mConfiguredEthernetIface = iface; } @@ -729,7 +729,7 @@ public class Tethering { // onAvailable called after stopping Ethernet tethering. return; } - stopEthernetTetheringLocked(); + stopEthernetTethering(); } } @@ -1030,7 +1030,7 @@ public class Tethering { // We can see this state on the way to both enabled and failure states. break; case WifiManager.WIFI_AP_STATE_ENABLED: - enableWifiIpServingLocked(ifname, ipmode); + enableWifiIpServing(ifname, ipmode); break; case WifiManager.WIFI_AP_STATE_DISABLING: // We can see this state on the way to disabled. @@ -1038,7 +1038,7 @@ public class Tethering { case WifiManager.WIFI_AP_STATE_DISABLED: case WifiManager.WIFI_AP_STATE_FAILED: default: - disableWifiIpServingLocked(ifname, curState); + disableWifiIpServing(ifname, curState); break; } } @@ -1062,7 +1062,7 @@ public class Tethering { // if no group is formed, bring it down if needed. if (p2pInfo == null || !p2pInfo.groupFormed) { - disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + disableWifiP2pIpServingIfNeeded(mWifiP2pTetherInterface); mWifiP2pTetherInterface = null; return; } @@ -1078,12 +1078,12 @@ public class Tethering { mLog.w("P2P tethered interface " + mWifiP2pTetherInterface + "is different from current interface " + group.getInterface() + ", re-tether it"); - disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + disableWifiP2pIpServingIfNeeded(mWifiP2pTetherInterface); } // Finally bring up serving on the new interface mWifiP2pTetherInterface = group.getInterface(); - enableWifiIpServingLocked(mWifiP2pTetherInterface, IFACE_IP_MODE_LOCAL_ONLY); + enableWifiIpServing(mWifiP2pTetherInterface, IFACE_IP_MODE_LOCAL_ONLY); } private void handleUserRestrictionAction() { @@ -1164,7 +1164,7 @@ public class Tethering { } } - private void disableWifiIpServingLockedCommon(int tetheringType, String ifname, int apState) { + private void disableWifiIpServingCommon(int tetheringType, String ifname, int apState) { mLog.log("Canceling WiFi tethering request -" + " type=" + tetheringType + " interface=" + ifname @@ -1191,23 +1191,23 @@ public class Tethering { : "specified interface: " + ifname)); } - private void disableWifiIpServingLocked(String ifname, int apState) { + private void disableWifiIpServing(String ifname, int apState) { // Regardless of whether we requested this transition, the AP has gone // down. Don't try to tether again unless we're requested to do so. // TODO: Remove this altogether, once Wi-Fi reliably gives us an // interface name with every broadcast. mWifiTetherRequested = false; - disableWifiIpServingLockedCommon(TETHERING_WIFI, ifname, apState); + disableWifiIpServingCommon(TETHERING_WIFI, ifname, apState); } - private void disableWifiP2pIpServingLockedIfNeeded(String ifname) { + private void disableWifiP2pIpServingIfNeeded(String ifname) { if (TextUtils.isEmpty(ifname)) return; - disableWifiIpServingLockedCommon(TETHERING_WIFI_P2P, ifname, /* fake */ 0); + disableWifiIpServingCommon(TETHERING_WIFI_P2P, ifname, /* fake */ 0); } - private void enableWifiIpServingLocked(String ifname, int wifiIpMode) { + private void enableWifiIpServing(String ifname, int wifiIpMode) { // Map wifiIpMode values to IpServer.Callback serving states, inferring // from mWifiTetherRequested as a final "best guess". final int ipServingMode; @@ -1224,7 +1224,7 @@ public class Tethering { } if (!TextUtils.isEmpty(ifname)) { - maybeTrackNewInterfaceLocked(ifname); + maybeTrackNewInterface(ifname); changeInterfaceState(ifname, ipServingMode); } else { mLog.e(String.format( @@ -2437,7 +2437,7 @@ public class Tethering { mTetherMainSM.sendMessage(which, state, 0, newLp); } - private void maybeTrackNewInterfaceLocked(final String iface) { + private void maybeTrackNewInterface(final String iface) { // If we don't care about this type of interface, ignore. final int interfaceType = ifaceNameToType(iface); if (interfaceType == TETHERING_INVALID) { @@ -2457,10 +2457,10 @@ public class Tethering { return; } - maybeTrackNewInterfaceLocked(iface, interfaceType); + maybeTrackNewInterface(iface, interfaceType); } - private void maybeTrackNewInterfaceLocked(final String iface, int interfaceType) { + private void maybeTrackNewInterface(final String iface, int interfaceType) { // If we have already started a TISM for this interface, skip. if (mTetherStates.containsKey(iface)) { mLog.log("active iface (" + iface + ") reported as added, ignoring"); @@ -2477,7 +2477,7 @@ public class Tethering { tetherState.ipServer.start(); } - private void stopTrackingInterfaceLocked(final String iface) { + private void stopTrackingInterface(final String iface) { final TetherState tetherState = mTetherStates.get(iface); if (tetherState == null) { mLog.log("attempting to remove unknown iface (" + iface + "), ignoring"); From 3b3d92ced0d21d5bcedab1c03b720dc92cc9eca3 Mon Sep 17 00:00:00 2001 From: markchien Date: Tue, 1 Jun 2021 10:58:04 +0800 Subject: [PATCH 2/2] Suppress IpServer message logs To avoid log flooding, suppress CMD_IPV6_TETHER_UPDATE and CMD_NEIGHBOR_EVENT message log because they appear frequentlg and don't have any content which means they are ueseless for debugging. Also remove logMessage from BaseServingState to fix duplicated message because all the message already log in TetheredState and LocalHotspotState which inherit BaseServingState. Bug: 162920185 Bug: 185649441 Test: atest TetheringCoverageTests atest MtsTetheringTestLatestSdk atest CtsTetheringTest Change-Id: Ib8fe719f3c1c4a65e6b5152df5e5adf33aba2079 Merged-In: Ib8fe719f3c1c4a65e6b5152df5e5adf33aba2079 --- Tethering/src/android/net/ip/IpServer.java | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index c45ce830e2..3428c1d640 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -1054,8 +1054,16 @@ public class IpServer extends StateMachine { mLastRaParams = newParams; } - private void logMessage(State state, int what) { - mLog.log(state.getName() + " got " + sMagicDecoderRing.get(what, Integer.toString(what))); + private void maybeLogMessage(State state, int what) { + switch (what) { + // Suppress some CMD_* to avoid log flooding. + case CMD_IPV6_TETHER_UPDATE: + case CMD_NEIGHBOR_EVENT: + break; + default: + mLog.log(state.getName() + " got " + + sMagicDecoderRing.get(what, Integer.toString(what))); + } } private void sendInterfaceState(int newInterfaceState) { @@ -1095,7 +1103,7 @@ public class IpServer extends StateMachine { @Override public boolean processMessage(Message message) { - logMessage(this, message.what); + maybeLogMessage(this, message.what); switch (message.what) { case CMD_TETHER_REQUESTED: mLastError = TetheringManager.TETHER_ERROR_NO_ERROR; @@ -1180,7 +1188,6 @@ public class IpServer extends StateMachine { @Override public boolean processMessage(Message message) { - logMessage(this, message.what); switch (message.what) { case CMD_TETHER_UNREQUESTED: transitionTo(mInitialState); @@ -1238,7 +1245,7 @@ public class IpServer extends StateMachine { public boolean processMessage(Message message) { if (super.processMessage(message)) return true; - logMessage(this, message.what); + maybeLogMessage(this, message.what); switch (message.what) { case CMD_TETHER_REQUESTED: mLog.e("CMD_TETHER_REQUESTED while in local-only hotspot mode."); @@ -1306,7 +1313,7 @@ public class IpServer extends StateMachine { public boolean processMessage(Message message) { if (super.processMessage(message)) return true; - logMessage(this, message.what); + maybeLogMessage(this, message.what); switch (message.what) { case CMD_TETHER_REQUESTED: mLog.e("CMD_TETHER_REQUESTED while already tethering."); @@ -1417,7 +1424,7 @@ public class IpServer extends StateMachine { class WaitingForRestartState extends State { @Override public boolean processMessage(Message message) { - logMessage(this, message.what); + maybeLogMessage(this, message.what); switch (message.what) { case CMD_TETHER_UNREQUESTED: transitionTo(mInitialState);