From 9307ca2ba13dd74ee1e4ad51ea4cf57741e23f0e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Sat, 12 Jan 2019 01:54:23 +0900 Subject: Delete the clatd functions from NetworkManagementService. Make Nat464Xlat talk to netd directly instead of through NetworkManagementService. The methods in NetworkmanagementService don't really provide any value: since the only thing they do is call into netd, we might as well have the callers talk to netd directly, In order to do this, pass INetworkManagementService and INetd to the NetworkAgentInfo constructor, and update callers appropriately. Bug: 65674744 Test: builds, boots Test: atest FrameworksNetTests Change-Id: Iac4cfe709c6279e4d9682b6754963e533707bd12 --- .../java/android/os/INetworkManagementService.aidl | 10 ----- .../com/android/server/ConnectivityService.java | 2 +- .../android/server/NetworkManagementService.java | 45 ++-------------------- .../android/server/connectivity/Nat464Xlat.java | 13 ++++--- .../server/connectivity/NetworkAgentInfo.java | 14 +++++-- .../android/server/ConnectivityServiceTest.java | 4 +- .../server/connectivity/LingerMonitorTest.java | 6 ++- .../server/connectivity/Nat464XlatTest.java | 30 +++++++-------- 8 files changed, 44 insertions(+), 80 deletions(-) diff --git a/core/java/android/os/INetworkManagementService.aidl b/core/java/android/os/INetworkManagementService.aidl index fdd74882eb39..8ced7225b0d4 100644 --- a/core/java/android/os/INetworkManagementService.aidl +++ b/core/java/android/os/INetworkManagementService.aidl @@ -355,16 +355,6 @@ interface INetworkManagementService */ void removeVpnUidRanges(int netId, in UidRange[] ranges); - /** - * Start the clatd (464xlat) service on the given interface. - */ - void startClatd(String interfaceName); - - /** - * Stop the clatd (464xlat) service on the given interface. - */ - void stopClatd(String interfaceName); - /** * Start listening for mobile activity state changes. */ diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d0666b98c0e0..d6f3e2ba4835 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4879,7 +4879,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, nc, currentScore, - mContext, mTrackerHandler, new NetworkMisc(networkMisc), this); + mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mNMS); // Make sure the network capabilities reflect what the agent info says. nai.networkCapabilities = mixInCapabilities(nai, nc); final String extraInfo = networkInfo.getExtraInfo(); diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 00b13207de2c..600a6ae0babd 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -17,13 +17,11 @@ package com.android.server; import static android.Manifest.permission.CONNECTIVITY_INTERNAL; -import static android.Manifest.permission.DUMP; import static android.Manifest.permission.NETWORK_SETTINGS; import static android.Manifest.permission.NETWORK_STACK; import static android.Manifest.permission.SHUTDOWN; import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_DOZABLE; import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NAME_DOZABLE; -import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NAME_NONE; import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NAME_POWERSAVE; import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NAME_STANDBY; import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NONE; @@ -40,6 +38,7 @@ import static android.net.NetworkStats.TAG_ALL; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; import static android.net.TrafficStats.UID_TETHERING; + import static com.android.server.NetworkManagementService.NetdResponseCode.ClatdStatusResult; import static com.android.server.NetworkManagementService.NetdResponseCode.InterfaceGetCfgResult; import static com.android.server.NetworkManagementService.NetdResponseCode.InterfaceListResult; @@ -53,11 +52,9 @@ import static com.android.server.NetworkManagementSocketTagger.PROP_QTAGUID_ENAB import android.annotation.NonNull; import android.app.ActivityManager; -import android.content.ContentResolver; import android.content.Context; import android.net.ConnectivityManager; import android.net.INetd; -import android.net.TetherStatsParcel; import android.net.INetworkManagementEventObserver; import android.net.ITetheringStatsProvider; import android.net.InterfaceConfiguration; @@ -69,18 +66,15 @@ import android.net.NetworkPolicyManager; import android.net.NetworkStats; import android.net.NetworkUtils; import android.net.RouteInfo; +import android.net.TetherStatsParcel; import android.net.UidRange; -import android.net.UidRangeParcel; import android.net.util.NetdService; -import android.net.wifi.WifiConfiguration; -import android.net.wifi.WifiConfiguration.KeyMgmt; import android.os.BatteryStats; import android.os.Binder; import android.os.Handler; import android.os.IBinder; import android.os.INetworkActivityListener; import android.os.INetworkManagementService; -import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; import android.os.RemoteCallbackList; @@ -91,12 +85,7 @@ import android.os.StrictMode; import android.os.SystemClock; import android.os.SystemProperties; import android.os.Trace; -import android.provider.Settings; import android.telephony.DataConnectionRealTimeInfo; -import android.telephony.PhoneStateListener; -import android.telephony.SubscriptionManager; -import android.telephony.TelephonyManager; -import android.text.TextUtils; import android.util.Log; import android.util.Slog; import android.util.SparseBooleanArray; @@ -109,13 +98,11 @@ import com.android.internal.net.NetworkStatsFactory; import com.android.internal.util.DumpUtils; import com.android.internal.util.HexDump; import com.android.internal.util.Preconditions; -import com.android.server.NativeDaemonConnector.Command; -import com.android.server.NativeDaemonConnector.SensitiveArg; + import com.google.android.collect.Maps; import java.io.BufferedReader; import java.io.DataInputStream; -import java.io.File; import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.IOException; @@ -123,15 +110,11 @@ import java.io.InputStreamReader; import java.io.PrintWriter; import java.net.InetAddress; import java.net.InterfaceAddress; -import java.net.NetworkInterface; -import java.net.SocketException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; -import java.util.StringTokenizer; import java.util.concurrent.CountDownLatch; /** @@ -2146,28 +2129,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub } } - @Override - public void startClatd(String interfaceName) throws IllegalStateException { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - - try { - mNetdService.clatdStart(interfaceName); - } catch (RemoteException | ServiceSpecificException e) { - throw new IllegalStateException(e); - } - } - - @Override - public void stopClatd(String interfaceName) throws IllegalStateException { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - - try { - mNetdService.clatdStop(interfaceName); - } catch (RemoteException | ServiceSpecificException e) { - throw new IllegalStateException(e); - } - } - @Override public void registerNetworkActivityListener(INetworkActivityListener listener) { mNetworkActivityListeners.register(listener); diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index 6596d27d021e..9d9b1cfdf6e2 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -16,8 +16,9 @@ package com.android.server.connectivity; -import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; +import android.net.INetd; +import android.net.InterfaceConfiguration; import android.net.LinkAddress; import android.net.LinkProperties; import android.net.NetworkInfo; @@ -59,6 +60,7 @@ public class Nat464Xlat extends BaseNetworkObserver { NetworkInfo.State.SUSPENDED, }; + private final INetd mNetd; private final INetworkManagementService mNMService; // The network we're running on, and its type. @@ -76,7 +78,8 @@ public class Nat464Xlat extends BaseNetworkObserver { private String mIface; private State mState = State.IDLE; - public Nat464Xlat(INetworkManagementService nmService, NetworkAgentInfo nai) { + public Nat464Xlat(NetworkAgentInfo nai, INetd netd, INetworkManagementService nmService) { + mNetd = netd; mNMService = nmService; mNetwork = nai; } @@ -140,7 +143,7 @@ public class Nat464Xlat extends BaseNetworkObserver { return; } try { - mNMService.startClatd(baseIface); + mNetd.clatdStart(baseIface); } catch(RemoteException|IllegalStateException e) { Slog.e(TAG, "Error starting clatd on " + baseIface, e); } @@ -162,7 +165,7 @@ public class Nat464Xlat extends BaseNetworkObserver { */ private void enterStoppingState() { try { - mNMService.stopClatd(mBaseIface); + mNetd.clatdStop(mBaseIface); } catch(RemoteException|IllegalStateException e) { Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } @@ -204,7 +207,7 @@ public class Nat464Xlat extends BaseNetworkObserver { Slog.e(TAG, "startClat: Can't start clat on null interface"); return; } - // TODO: should we only do this if mNMService.startClatd() succeeds? + // TODO: should we only do this if mNetd.clatdStart() succeeds? Slog.i(TAG, "Starting clatd on " + baseIface); enterStartingState(baseIface); } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 54c89aa04111..9ea73fbb1882 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -17,6 +17,7 @@ package com.android.server.connectivity; import android.content.Context; +import android.net.INetd; import android.net.INetworkMonitor; import android.net.LinkProperties; import android.net.Network; @@ -239,12 +240,15 @@ public class NetworkAgentInfo implements Comparable { private static final String TAG = ConnectivityService.class.getSimpleName(); private static final boolean VDBG = false; private final ConnectivityService mConnService; + private final INetd mNetd; + private final INetworkManagementService mNMS; private final Context mContext; private final Handler mHandler; public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, - NetworkMisc misc, ConnectivityService connService) { + NetworkMisc misc, ConnectivityService connService, INetd netd, + INetworkManagementService nms) { this.messenger = messenger; asyncChannel = ac; network = net; @@ -253,6 +257,8 @@ public class NetworkAgentInfo implements Comparable { networkCapabilities = nc; currentScore = score; mConnService = connService; + mNetd = netd; + mNMS = nms; mContext = context; mHandler = handler; networkMisc = misc; @@ -587,18 +593,18 @@ public class NetworkAgentInfo implements Comparable { public void updateClat(INetworkManagementService netd) { if (Nat464Xlat.requiresClat(this)) { - maybeStartClat(netd); + maybeStartClat(); } else { maybeStopClat(); } } /** Ensure clat has started for this network. */ - public void maybeStartClat(INetworkManagementService netd) { + public void maybeStartClat() { if (clatd != null && clatd.isStarted()) { return; } - clatd = new Nat464Xlat(netd, this); + clatd = new Nat464Xlat(this, mNetd, mNMS); clatd.start(); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index bf3964416e11..2a92a7dabd98 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4683,7 +4683,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.sendLinkProperties(cellLp); mCellNetworkAgent.connect(true); networkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); - verify(mNetworkManagementService, times(1)).startClatd(MOBILE_IFNAME); + verify(mMockNetd, times(1)).clatdStart(MOBILE_IFNAME); Nat464Xlat clat = mService.getNat464Xlat(mCellNetworkAgent); // Clat iface up, expect stack link updated. @@ -4710,7 +4710,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, mCellNetworkAgent); - verify(mNetworkManagementService, times(1)).stopClatd(MOBILE_IFNAME); + verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); // Clat iface removed, expect linkproperties revert to original one clat.interfaceRemoved(CLAT_PREFIX + MOBILE_IFNAME); diff --git a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java index 4c52d818269d..9578ded1a089 100644 --- a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java @@ -32,11 +32,13 @@ import android.app.PendingIntent; import android.content.Context; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.net.INetd; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkMisc; import android.net.NetworkStack; +import android.os.INetworkManagementService; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.text.format.DateUtils; @@ -66,6 +68,8 @@ public class LingerMonitorTest { LingerMonitor mMonitor; @Mock ConnectivityService mConnService; + @Mock INetd mNetd; + @Mock INetworkManagementService mNMS; @Mock Context mCtx; @Mock NetworkMisc mMisc; @Mock NetworkNotificationManager mNotifier; @@ -352,7 +356,7 @@ public class LingerMonitorTest { caps.addCapability(0); caps.addTransportType(transport); NetworkAgentInfo nai = new NetworkAgentInfo(null, null, new Network(netId), info, null, - caps, 50, mCtx, null, mMisc, mConnService); + caps, 50, mCtx, null, mMisc, mConnService, mNetd, mNMS); nai.everValidated = true; return nai; } diff --git a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java index bf42412d68d8..07b1d057c882 100644 --- a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java +++ b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java @@ -17,9 +17,7 @@ package com.android.server.connectivity; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -27,6 +25,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.net.ConnectivityManager; +import android.net.INetd; import android.net.InterfaceConfiguration; import android.net.LinkAddress; import android.net.LinkProperties; @@ -57,6 +56,7 @@ public class Nat464XlatTest { @Mock ConnectivityService mConnectivity; @Mock NetworkMisc mMisc; + @Mock INetd mNetd; @Mock INetworkManagementService mNms; @Mock InterfaceConfiguration mConfig; @Mock NetworkAgentInfo mNai; @@ -65,7 +65,7 @@ public class Nat464XlatTest { Handler mHandler; Nat464Xlat makeNat464Xlat() { - return new Nat464Xlat(mNms, mNai); + return new Nat464Xlat(mNai, mNetd, mNms); } @Before @@ -129,7 +129,7 @@ public class Nat464XlatTest { nat.start(); verify(mNms).registerObserver(eq(nat)); - verify(mNms).startClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStart(eq(BASE_IFACE)); // Stacked interface up notification arrives. nat.interfaceLinkStateChanged(STACKED_IFACE, true); @@ -144,7 +144,7 @@ public class Nat464XlatTest { // ConnectivityService stops clat (Network disconnects, IPv4 addr appears, ...). nat.stop(); - verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStop(eq(BASE_IFACE)); // Stacked interface removed notification arrives. nat.interfaceRemoved(STACKED_IFACE); @@ -156,7 +156,7 @@ public class Nat464XlatTest { assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); assertIdle(nat); - verifyNoMoreInteractions(mNms, mConnectivity); + verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } @Test @@ -168,7 +168,7 @@ public class Nat464XlatTest { nat.start(); verify(mNms).registerObserver(eq(nat)); - verify(mNms).startClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStart(eq(BASE_IFACE)); // Stacked interface up notification arrives. nat.interfaceLinkStateChanged(STACKED_IFACE, true); @@ -185,7 +185,7 @@ public class Nat464XlatTest { mLooper.dispatchNext(); verify(mNms).unregisterObserver(eq(nat)); - verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStop(eq(BASE_IFACE)); verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture()); assertTrue(c.getValue().getStackedLinks().isEmpty()); assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); @@ -194,7 +194,7 @@ public class Nat464XlatTest { // ConnectivityService stops clat: no-op. nat.stop(); - verifyNoMoreInteractions(mNms, mConnectivity); + verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } @Test @@ -205,13 +205,13 @@ public class Nat464XlatTest { nat.start(); verify(mNms).registerObserver(eq(nat)); - verify(mNms).startClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStart(eq(BASE_IFACE)); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) nat.stop(); verify(mNms).unregisterObserver(eq(nat)); - verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStop(eq(BASE_IFACE)); assertIdle(nat); // In-flight interface up notification arrives: no-op @@ -225,7 +225,7 @@ public class Nat464XlatTest { assertIdle(nat); - verifyNoMoreInteractions(mNms, mConnectivity); + verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } @Test @@ -236,16 +236,16 @@ public class Nat464XlatTest { nat.start(); verify(mNms).registerObserver(eq(nat)); - verify(mNms).startClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStart(eq(BASE_IFACE)); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) nat.stop(); verify(mNms).unregisterObserver(eq(nat)); - verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNetd).clatdStop(eq(BASE_IFACE)); assertIdle(nat); - verifyNoMoreInteractions(mNms, mConnectivity); + verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } static void assertIdle(Nat464Xlat nat) { -- cgit v1.2.3-59-g8ed1b From dac3918fa17be34881507795d17e9e43fbb09c04 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 15 Jan 2019 11:25:56 +0900 Subject: Test parceling empty LinkProperties. Test: atest FrameworksNetTests Change-Id: Iec53a9fdce1ba1db86751ea5f14a11d28be543eb --- tests/net/java/android/net/LinkPropertiesTest.java | 27 ++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/net/java/android/net/LinkPropertiesTest.java b/tests/net/java/android/net/LinkPropertiesTest.java index 932fee0c1956..299fbefc78e4 100644 --- a/tests/net/java/android/net/LinkPropertiesTest.java +++ b/tests/net/java/android/net/LinkPropertiesTest.java @@ -849,6 +849,18 @@ public class LinkPropertiesTest { assertEquals(new ArraySet<>(expectRemoved), (new ArraySet<>(result.removed))); } + private void assertParcelingIsLossless(LinkProperties source) { + Parcel p = Parcel.obtain(); + source.writeToParcel(p, /* flags */ 0); + p.setDataPosition(0); + final byte[] marshalled = p.marshall(); + p = Parcel.obtain(); + p.unmarshall(marshalled, 0, marshalled.length); + p.setDataPosition(0); + LinkProperties dest = LinkProperties.CREATOR.createFromParcel(p); + assertEquals(source, dest); + } + @Test public void testLinkPropertiesParcelable() throws Exception { LinkProperties source = new LinkProperties(); @@ -870,15 +882,12 @@ public class LinkPropertiesTest { source.setNat64Prefix(new IpPrefix("2001:db8:1:2:64:64::/96")); - Parcel p = Parcel.obtain(); - source.writeToParcel(p, /* flags */ 0); - p.setDataPosition(0); - final byte[] marshalled = p.marshall(); - p = Parcel.obtain(); - p.unmarshall(marshalled, 0, marshalled.length); - p.setDataPosition(0); - LinkProperties dest = LinkProperties.CREATOR.createFromParcel(p); + assertParcelingIsLossless(source); + } - assertEquals(source, dest); + @Test + public void testParcelUninitialized() throws Exception { + LinkProperties empty = new LinkProperties(); + assertParcelingIsLossless(empty); } } -- cgit v1.2.3-59-g8ed1b