LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[SP07] Remove reference of NetworkStatsService in IpServer

Browse Source 
Currently NetworkStatsService is notified when downstream is
updated. However, it seems unnecessary given that tether stats
is persist since boot, and there is no any upstream change when
downstream is changed.

Test: atest NetworkStatsServiceTest IpServerTest
Bug: 130855321
Change-Id: Ie300bfeb0a04678fcfcf300843b6f859af9df91d
This commit is contained in:
junyulai
2019-12-03 14:34:13 +08:00
committed by markchien
parent 15d8bb03fe
commit 5864a3f0aa
3 changed files with 23 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
Tethering/src/android/net/ip/IpServer.java
View File

@@ -26,7 +26,6 @@ import static android.net.util.TetheringMessageBase.BASE_IPSERVER;
import android.net.INetd; import android.net.INetd;
import android.net.INetworkStackStatusCallback; import android.net.INetworkStackStatusCallback;
import android.net.INetworkStatsService;
import android.net.IpPrefix; import android.net.IpPrefix;
import android.net.LinkAddress; import android.net.LinkAddress;
import android.net.LinkProperties; import android.net.LinkProperties;
@@ -176,7 +175,6 @@ public class IpServer extends StateMachine {
private final SharedLog mLog; private final SharedLog mLog;
private final INetd mNetd; private final INetd mNetd;
private final INetworkStatsService mStatsService;
private final Callback mCallback; private final Callback mCallback;
private final InterfaceController mInterfaceCtrl; private final InterfaceController mInterfaceCtrl;
@@ -208,12 +206,10 @@ public class IpServer extends StateMachine {
public IpServer( public IpServer(
String ifaceName, Looper looper, int interfaceType, SharedLog log, String ifaceName, Looper looper, int interfaceType, SharedLog log,
INetd netd, INetworkStatsService statsService, Callback callback, INetd netd, Callback callback, boolean usingLegacyDhcp, Dependencies deps) {
boolean usingLegacyDhcp, Dependencies deps) {
super(ifaceName, looper); super(ifaceName, looper);
mLog = log.forSubComponent(ifaceName); mLog = log.forSubComponent(ifaceName);
mNetd = netd; mNetd = netd;
mStatsService = statsService;
mCallback = callback; mCallback = callback;
mInterfaceCtrl = new InterfaceController(ifaceName, mNetd, mLog); mInterfaceCtrl = new InterfaceController(ifaceName, mNetd, mLog);
mIfaceName = ifaceName; mIfaceName = ifaceName;
@@ -881,12 +877,6 @@ public class IpServer extends StateMachine {
// Sometimes interfaces are gone before we get // Sometimes interfaces are gone before we get
// to remove their rules, which generates errors. // to remove their rules, which generates errors.
// Just do the best we can. // Just do the best we can.
try {
// About to tear down NAT; gather remaining statistics.
mStatsService.forceUpdate();
} catch (Exception e) {
mLog.e("Exception in forceUpdate: " + e.toString());
}
try { try {
mNetd.ipfwdRemoveInterfaceForward(mIfaceName, upstreamIface); mNetd.ipfwdRemoveInterfaceForward(mIfaceName, upstreamIface);
} catch (RemoteException | ServiceSpecificException e) { } catch (RemoteException | ServiceSpecificException e) {

2
Tethering/src/com/android/server/connectivity/tethering/Tethering.java
View File

@@ -2056,7 +2056,7 @@ public class Tethering {
mLog.log("adding TetheringInterfaceStateMachine for: " + iface); mLog.log("adding TetheringInterfaceStateMachine for: " + iface);
final TetherState tetherState = new TetherState( final TetherState tetherState = new TetherState(
new IpServer(iface, mLooper, interfaceType, mLog, mNetd, mStatsService, new IpServer(iface, mLooper, interfaceType, mLog, mNetd,
makeControlCallback(), mConfig.enableLegacyDhcpServer, makeControlCallback(), mConfig.enableLegacyDhcpServer,
mDeps.getIpServerDependencies())); mDeps.getIpServerDependencies()));
mTetherStates.put(iface, tetherState); mTetherStates.put(iface, tetherState);

50
Tethering/tests/unit/src/android/net/ip/IpServerTest.java
View File

@@ -52,7 +52,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import android.net.INetd; import android.net.INetd;
import android.net.INetworkStatsService;
import android.net.InterfaceConfigurationParcel; import android.net.InterfaceConfigurationParcel;
import android.net.IpPrefix; import android.net.IpPrefix;
import android.net.LinkAddress; import android.net.LinkAddress;
@@ -99,7 +98,6 @@ public class IpServerTest {
private static final int MAKE_DHCPSERVER_TIMEOUT_MS = 1000; private static final int MAKE_DHCPSERVER_TIMEOUT_MS = 1000;
@Mock private INetd mNetd; @Mock private INetd mNetd;
@Mock private INetworkStatsService mStatsService;
@Mock private IpServer.Callback mCallback; @Mock private IpServer.Callback mCallback;
@Mock private SharedLog mSharedLog; @Mock private SharedLog mSharedLog;
@Mock private IDhcpServer mDhcpServer; @Mock private IDhcpServer mDhcpServer;
@@ -139,13 +137,13 @@ public class IpServerTest {
mInterfaceConfiguration.prefixLength = BLUETOOTH_DHCP_PREFIX_LENGTH; mInterfaceConfiguration.prefixLength = BLUETOOTH_DHCP_PREFIX_LENGTH;
} }
mIpServer = new IpServer( mIpServer = new IpServer(
IFACE_NAME, mLooper.getLooper(), interfaceType, mSharedLog, mNetd, mStatsService, IFACE_NAME, mLooper.getLooper(), interfaceType, mSharedLog, mNetd,
mCallback, usingLegacyDhcp, mDependencies); mCallback, usingLegacyDhcp, mDependencies);
mIpServer.start(); mIpServer.start();
// Starting the state machine always puts us in a consistent state and notifies // Starting the state machine always puts us in a consistent state and notifies
// the rest of the world that we've changed from an unknown to available state. // the rest of the world that we've changed from an unknown to available state.
mLooper.dispatchAll(); mLooper.dispatchAll();
reset(mNetd, mStatsService, mCallback); reset(mNetd, mCallback);
when(mRaDaemon.start()).thenReturn(true); when(mRaDaemon.start()).thenReturn(true);
} }
@@ -162,7 +160,7 @@ public class IpServerTest {
if (upstreamIface != null) { if (upstreamIface != null) {
dispatchTetherConnectionChanged(upstreamIface); dispatchTetherConnectionChanged(upstreamIface);
} }
reset(mNetd, mStatsService, mCallback); reset(mNetd, mCallback);
} }
@Before public void setUp() throws Exception { @Before public void setUp() throws Exception {
@@ -173,13 +171,13 @@ public class IpServerTest {
@Test @Test
public void startsOutAvailable() { public void startsOutAvailable() {
mIpServer = new IpServer(IFACE_NAME, mLooper.getLooper(), TETHERING_BLUETOOTH, mSharedLog, mIpServer = new IpServer(IFACE_NAME, mLooper.getLooper(), TETHERING_BLUETOOTH, mSharedLog,
mNetd, mStatsService, mCallback, false /* usingLegacyDhcp */, mDependencies); mNetd, mCallback, false /* usingLegacyDhcp */, mDependencies);
mIpServer.start(); mIpServer.start();
mLooper.dispatchAll(); mLooper.dispatchAll();
verify(mCallback).updateInterfaceState( verify(mCallback).updateInterfaceState(
mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR); mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
verify(mCallback).updateLinkProperties(eq(mIpServer), any(LinkProperties.class)); verify(mCallback).updateLinkProperties(eq(mIpServer), any(LinkProperties.class));
verifyNoMoreInteractions(mCallback, mNetd, mStatsService); verifyNoMoreInteractions(mCallback, mNetd);
} }
@Test @Test
@@ -198,7 +196,7 @@ public class IpServerTest {
// None of these commands should trigger us to request action from // None of these commands should trigger us to request action from
// the rest of the system. // the rest of the system.
dispatchCommand(command); dispatchCommand(command);
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
} }
@@ -210,7 +208,7 @@ public class IpServerTest {
verify(mCallback).updateInterfaceState( verify(mCallback).updateInterfaceState(
mIpServer, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR); mIpServer, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
verify(mCallback).updateLinkProperties(eq(mIpServer), any(LinkProperties.class)); verify(mCallback).updateLinkProperties(eq(mIpServer), any(LinkProperties.class));
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -228,7 +226,7 @@ public class IpServerTest {
mIpServer, STATE_TETHERED, TETHER_ERROR_NO_ERROR); mIpServer, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
inOrder.verify(mCallback).updateLinkProperties( inOrder.verify(mCallback).updateLinkProperties(
eq(mIpServer), any(LinkProperties.class)); eq(mIpServer), any(LinkProperties.class));
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -236,7 +234,7 @@ public class IpServerTest {
initTetheredStateMachine(TETHERING_BLUETOOTH, null); initTetheredStateMachine(TETHERING_BLUETOOTH, null);
dispatchCommand(IpServer.CMD_TETHER_UNREQUESTED); dispatchCommand(IpServer.CMD_TETHER_UNREQUESTED);
InOrder inOrder = inOrder(mNetd, mStatsService, mCallback); InOrder inOrder = inOrder(mNetd, mCallback);
inOrder.verify(mNetd).tetherApplyDnsInterfaces(); inOrder.verify(mNetd).tetherApplyDnsInterfaces();
inOrder.verify(mNetd).tetherInterfaceRemove(IFACE_NAME); inOrder.verify(mNetd).tetherInterfaceRemove(IFACE_NAME);
inOrder.verify(mNetd).networkRemoveInterface(INetd.LOCAL_NET_ID, IFACE_NAME); inOrder.verify(mNetd).networkRemoveInterface(INetd.LOCAL_NET_ID, IFACE_NAME);
@@ -245,7 +243,7 @@ public class IpServerTest {
mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR); mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
inOrder.verify(mCallback).updateLinkProperties( inOrder.verify(mCallback).updateLinkProperties(
eq(mIpServer), any(LinkProperties.class)); eq(mIpServer), any(LinkProperties.class));
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -265,7 +263,7 @@ public class IpServerTest {
inOrder.verify(mCallback).updateLinkProperties( inOrder.verify(mCallback).updateLinkProperties(
eq(mIpServer), mLinkPropertiesCaptor.capture()); eq(mIpServer), mLinkPropertiesCaptor.capture());
assertIPv4AddressAndDirectlyConnectedRoute(mLinkPropertiesCaptor.getValue()); assertIPv4AddressAndDirectlyConnectedRoute(mLinkPropertiesCaptor.getValue());
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -285,7 +283,7 @@ public class IpServerTest {
inOrder.verify(mCallback).updateLinkProperties( inOrder.verify(mCallback).updateLinkProperties(
eq(mIpServer), mLinkPropertiesCaptor.capture()); eq(mIpServer), mLinkPropertiesCaptor.capture());
assertIPv4AddressAndDirectlyConnectedRoute(mLinkPropertiesCaptor.getValue()); assertIPv4AddressAndDirectlyConnectedRoute(mLinkPropertiesCaptor.getValue());
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -298,7 +296,7 @@ public class IpServerTest {
InOrder inOrder = inOrder(mNetd); InOrder inOrder = inOrder(mNetd);
inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE);
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -306,13 +304,12 @@ public class IpServerTest {
initTetheredStateMachine(TETHERING_BLUETOOTH, UPSTREAM_IFACE); initTetheredStateMachine(TETHERING_BLUETOOTH, UPSTREAM_IFACE);
dispatchTetherConnectionChanged(UPSTREAM_IFACE2); dispatchTetherConnectionChanged(UPSTREAM_IFACE2);
InOrder inOrder = inOrder(mNetd, mStatsService); InOrder inOrder = inOrder(mNetd);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2);
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -322,12 +319,10 @@ public class IpServerTest {
doThrow(RemoteException.class).when(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); doThrow(RemoteException.class).when(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2);
dispatchTetherConnectionChanged(UPSTREAM_IFACE2); dispatchTetherConnectionChanged(UPSTREAM_IFACE2);
InOrder inOrder = inOrder(mNetd, mStatsService); InOrder inOrder = inOrder(mNetd);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE2);
} }
@@ -340,13 +335,11 @@ public class IpServerTest {
IFACE_NAME, UPSTREAM_IFACE2); IFACE_NAME, UPSTREAM_IFACE2);
dispatchTetherConnectionChanged(UPSTREAM_IFACE2); dispatchTetherConnectionChanged(UPSTREAM_IFACE2);
InOrder inOrder = inOrder(mNetd, mStatsService); InOrder inOrder = inOrder(mNetd);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE2);
} }
@@ -356,8 +349,7 @@ public class IpServerTest {
initTetheredStateMachine(TETHERING_BLUETOOTH, UPSTREAM_IFACE); initTetheredStateMachine(TETHERING_BLUETOOTH, UPSTREAM_IFACE);
dispatchCommand(IpServer.CMD_TETHER_UNREQUESTED); dispatchCommand(IpServer.CMD_TETHER_UNREQUESTED);
InOrder inOrder = inOrder(mNetd, mStatsService, mCallback); InOrder inOrder = inOrder(mNetd, mCallback);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNetd).tetherApplyDnsInterfaces(); inOrder.verify(mNetd).tetherApplyDnsInterfaces();
@@ -368,7 +360,7 @@ public class IpServerTest {
mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR); mIpServer, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
inOrder.verify(mCallback).updateLinkProperties( inOrder.verify(mCallback).updateLinkProperties(
eq(mIpServer), any(LinkProperties.class)); eq(mIpServer), any(LinkProperties.class));
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
@Test @Test
@@ -435,11 +427,11 @@ public class IpServerTest {
public void ignoresDuplicateUpstreamNotifications() throws Exception { public void ignoresDuplicateUpstreamNotifications() throws Exception {
initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE); initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE);
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
for (int i = 0; i < 5; i++) { for (int i = 0; i < 5; i++) {
dispatchTetherConnectionChanged(UPSTREAM_IFACE); dispatchTetherConnectionChanged(UPSTREAM_IFACE);
verifyNoMoreInteractions(mNetd, mStatsService, mCallback); verifyNoMoreInteractions(mNetd, mCallback);
} }
} }