From 151384dda4d9a10e495897f5275584ae380fef14 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 3 Dec 2019 11:37:09 +0800 Subject: [PATCH] p2p: revise tethering handler for shared group interface support When leaving a group, all information are erased and no group interface is passed to tethering service. For separate group interface, tethering could be stopped on p2p group interface removed. For shared group interface, i.e. management interface and group interface share one interface, ex. p2p0, tethering has no chance to be stopped since management interface won't be removed after leaving a group. Bug: 141382930 Test: atest FrameworksNetTests Test: atest FrameworksWifiTests Test: atest TetheringTests Change-Id: Ib611018b67c76ff79c7e6658136721090feb145b --- .../src/android/net/ip/IpServer.java | 3 +- .../connectivity/tethering/Tethering.java | 49 ++++++++++++------- .../unit/src/android/net/ip/IpServerTest.java | 12 ++++- .../connectivity/tethering/TetheringTest.java | 12 +++-- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/packages/Tethering/src/android/net/ip/IpServer.java b/packages/Tethering/src/android/net/ip/IpServer.java index 4306cf0bd5f30..0491ad7c3413e 100644 --- a/packages/Tethering/src/android/net/ip/IpServer.java +++ b/packages/Tethering/src/android/net/ip/IpServer.java @@ -441,7 +441,8 @@ public class IpServer extends StateMachine { } final Boolean setIfaceUp; - if (mInterfaceType == TetheringManager.TETHERING_WIFI) { + if (mInterfaceType == TetheringManager.TETHERING_WIFI + || mInterfaceType == TetheringManager.TETHERING_WIFI_P2P) { // The WiFi stack has ownership of the interface up/down state. // It is unclear whether the Bluetooth or USB stacks will manage their own // state. diff --git a/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java b/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java index 26875b1d51ae2..5bf41ce25b2b9 100644 --- a/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java +++ b/packages/Tethering/src/com/android/server/connectivity/tethering/Tethering.java @@ -207,6 +207,7 @@ public class Tethering { private Network mTetherUpstream; private TetherStatesParcel mTetherStatesParcel; private boolean mDataSaverEnabled = false; + private String mWifiP2pTetherInterface = null; public Tethering(TetheringDependencies deps) { mLog.mark("Tethering.constructed"); @@ -852,6 +853,11 @@ public class Tethering { } } + private boolean isGroupOwner(WifiP2pGroup group) { + return group != null && group.isGroupOwner() + && !TextUtils.isEmpty(group.getInterface()); + } + private void handleWifiP2pAction(Intent intent) { if (mConfig.isWifiP2pLegacyTetheringMode()) return; @@ -864,24 +870,31 @@ public class Tethering { Log.d(TAG, "WifiP2pAction: P2pInfo: " + p2pInfo + " Group: " + group); } - if (p2pInfo == null) return; - // When a p2p group is disconnected, p2pInfo would be cleared. - // group is still valid for detecting whether this device is group owner. - if (group == null || !group.isGroupOwner() - || TextUtils.isEmpty(group.getInterface())) return; - synchronized (Tethering.this.mPublicSync) { - // Enter below only if this device is Group Owner with a valid interface. - if (p2pInfo.groupFormed) { - TetherState tetherState = mTetherStates.get(group.getInterface()); - if (tetherState == null - || (tetherState.lastState != IpServer.STATE_TETHERED - && tetherState.lastState != IpServer.STATE_LOCAL_ONLY)) { - enableWifiIpServingLocked(group.getInterface(), IFACE_IP_MODE_LOCAL_ONLY); - } - } else { - disableWifiP2pIpServingLocked(group.getInterface()); + // if no group is formed, bring it down if needed. + if (p2pInfo == null || !p2pInfo.groupFormed) { + disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + mWifiP2pTetherInterface = null; + return; } + + // If there is a group but the device is not the owner, bail out. + if (!isGroupOwner(group)) return; + + // If already serving from the correct interface, nothing to do. + if (group.getInterface().equals(mWifiP2pTetherInterface)) return; + + // If already serving from another interface, turn it down first. + if (!TextUtils.isEmpty(mWifiP2pTetherInterface)) { + mLog.w("P2P tethered interface " + mWifiP2pTetherInterface + + "is different from current interface " + + group.getInterface() + ", re-tether it"); + disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + } + + // Finally bring up serving on the new interface + mWifiP2pTetherInterface = group.getInterface(); + enableWifiIpServingLocked(mWifiP2pTetherInterface, IFACE_IP_MODE_LOCAL_ONLY); } } @@ -979,7 +992,9 @@ public class Tethering { disableWifiIpServingLockedCommon(TETHERING_WIFI, ifname, apState); } - private void disableWifiP2pIpServingLocked(String ifname) { + private void disableWifiP2pIpServingLockedIfNeeded(String ifname) { + if (TextUtils.isEmpty(ifname)) return; + disableWifiIpServingLockedCommon(TETHERING_WIFI_P2P, ifname, /* dummy */ 0); } diff --git a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 1f50b6bf7fb32..f29ad780b92c2 100644 --- a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -273,7 +273,7 @@ public class IpServerTest { dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY); InOrder inOrder = inOrder(mCallback, mNetd); inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg -> - IFACE_NAME.equals(cfg.ifName) && assertContainsFlag(cfg.flags, IF_STATE_UP))); + IFACE_NAME.equals(cfg.ifName) && assertNotContainsFlag(cfg.flags, IF_STATE_UP))); inOrder.verify(mNetd).tetherInterfaceAdd(IFACE_NAME); inOrder.verify(mNetd).networkAddInterface(INetd.LOCAL_NET_ID, IFACE_NAME); inOrder.verify(mNetd, times(2)).networkAddRoute(eq(INetd.LOCAL_NET_ID), eq(IFACE_NAME), @@ -547,4 +547,14 @@ public class IpServerTest { fail("Missing flag: " + match); return false; } + + private boolean assertNotContainsFlag(String[] flags, String match) { + for (String flag : flags) { + if (flag.equals(match)) { + fail("Unexpected flag: " + match); + return false; + } + } + return true; + } } diff --git a/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java b/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java index 857e4deaf9194..affd69154b70c 100644 --- a/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java +++ b/packages/Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java @@ -493,13 +493,15 @@ public class TetheringTest { private void sendWifiP2pConnectionChanged( boolean isGroupFormed, boolean isGroupOwner, String ifname) { + WifiP2pGroup group = null; WifiP2pInfo p2pInfo = new WifiP2pInfo(); p2pInfo.groupFormed = isGroupFormed; - p2pInfo.isGroupOwner = isGroupOwner; - - WifiP2pGroup group = mock(WifiP2pGroup.class); - when(group.isGroupOwner()).thenReturn(isGroupOwner); - when(group.getInterface()).thenReturn(ifname); + if (isGroupFormed) { + p2pInfo.isGroupOwner = isGroupOwner; + group = mock(WifiP2pGroup.class); + when(group.isGroupOwner()).thenReturn(isGroupOwner); + when(group.getInterface()).thenReturn(ifname); + } final Intent intent = mock(Intent.class); when(intent.getAction()).thenReturn(WifiP2pManager.WIFI_P2P_CONNECTION_CHANGED_ACTION);