From 812ffea0292e6d5f5bed93f121068b71eb4ea27d Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 30 May 2019 17:11:14 +0900 Subject: [PATCH 1/2] Factorize custom asserts. Also a few utilities that were in the way, and some opportunistic cleanups. Test: FrameworksNetTest NetworkStackTest Change-Id: I385070e2044fd967cb18f1ffea9a86a4627b742e --- core/java/android/net/IpSecConfig.java | 41 ++++++++++++----------- core/java/android/net/IpSecTransform.java | 14 ++++---- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/core/java/android/net/IpSecConfig.java b/core/java/android/net/IpSecConfig.java index 3552655983..43c8ff2335 100644 --- a/core/java/android/net/IpSecConfig.java +++ b/core/java/android/net/IpSecConfig.java @@ -15,6 +15,7 @@ */ package android.net; +import android.annotation.Nullable; import android.os.Parcel; import android.os.Parcelable; @@ -333,25 +334,25 @@ public final class IpSecConfig implements Parcelable { } }; - @VisibleForTesting - /** Equals method used for testing */ - public static boolean equals(IpSecConfig lhs, IpSecConfig rhs) { - if (lhs == null || rhs == null) return (lhs == rhs); - return (lhs.mMode == rhs.mMode - && lhs.mSourceAddress.equals(rhs.mSourceAddress) - && lhs.mDestinationAddress.equals(rhs.mDestinationAddress) - && ((lhs.mNetwork != null && lhs.mNetwork.equals(rhs.mNetwork)) - || (lhs.mNetwork == rhs.mNetwork)) - && lhs.mEncapType == rhs.mEncapType - && lhs.mEncapSocketResourceId == rhs.mEncapSocketResourceId - && lhs.mEncapRemotePort == rhs.mEncapRemotePort - && lhs.mNattKeepaliveInterval == rhs.mNattKeepaliveInterval - && lhs.mSpiResourceId == rhs.mSpiResourceId - && IpSecAlgorithm.equals(lhs.mEncryption, rhs.mEncryption) - && IpSecAlgorithm.equals(lhs.mAuthenticatedEncryption, rhs.mAuthenticatedEncryption) - && IpSecAlgorithm.equals(lhs.mAuthentication, rhs.mAuthentication) - && lhs.mMarkValue == rhs.mMarkValue - && lhs.mMarkMask == rhs.mMarkMask - && lhs.mXfrmInterfaceId == rhs.mXfrmInterfaceId); + @Override + public boolean equals(@Nullable Object other) { + if (!(other instanceof IpSecConfig)) return false; + final IpSecConfig rhs = (IpSecConfig) other; + return (mMode == rhs.mMode + && mSourceAddress.equals(rhs.mSourceAddress) + && mDestinationAddress.equals(rhs.mDestinationAddress) + && ((mNetwork != null && mNetwork.equals(rhs.mNetwork)) + || (mNetwork == rhs.mNetwork)) + && mEncapType == rhs.mEncapType + && mEncapSocketResourceId == rhs.mEncapSocketResourceId + && mEncapRemotePort == rhs.mEncapRemotePort + && mNattKeepaliveInterval == rhs.mNattKeepaliveInterval + && mSpiResourceId == rhs.mSpiResourceId + && IpSecAlgorithm.equals(mEncryption, rhs.mEncryption) + && IpSecAlgorithm.equals(mAuthenticatedEncryption, rhs.mAuthenticatedEncryption) + && IpSecAlgorithm.equals(mAuthentication, rhs.mAuthentication) + && mMarkValue == rhs.mMarkValue + && mMarkMask == rhs.mMarkMask + && mXfrmInterfaceId == rhs.mXfrmInterfaceId); } } diff --git a/core/java/android/net/IpSecTransform.java b/core/java/android/net/IpSecTransform.java index a12df28eac..93ae4f1188 100644 --- a/core/java/android/net/IpSecTransform.java +++ b/core/java/android/net/IpSecTransform.java @@ -148,15 +148,13 @@ public final class IpSecTransform implements AutoCloseable { } /** - * Equals method used for testing - * - * @hide + * Standard equals. */ - @VisibleForTesting - public static boolean equals(IpSecTransform lhs, IpSecTransform rhs) { - if (lhs == null || rhs == null) return (lhs == rhs); - return IpSecConfig.equals(lhs.getConfig(), rhs.getConfig()) - && lhs.mResourceId == rhs.mResourceId; + public boolean equals(Object other) { + if (this == other) return true; + if (!(other instanceof IpSecTransform)) return false; + final IpSecTransform rhs = (IpSecTransform) other; + return getConfig().equals(rhs.getConfig()) && mResourceId == rhs.mResourceId; } /** From d749767f4431ee14b2e004833259d359d9790c58 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Fri, 14 Jun 2019 11:54:49 -0700 Subject: [PATCH 2/2] Inline readNetworkStatsDetailInternal, make mUseBpfStats final This change inlines the logic from readNetworkStatsDetailInternal, and reduces reundant checks in mUseBpfStats Bug: 113122541 Test: atest FrameworksNetTests run, passing Change-Id: If2ef8d8f038f32c8cf974aa02cfc1dc7e44dbad3 --- .../server/net/NetworkStatsFactory.java | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java index 2d3c66d68f..7687718b06 100644 --- a/services/core/java/com/android/server/net/NetworkStatsFactory.java +++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java @@ -68,7 +68,7 @@ public class NetworkStatsFactory { /** Path to {@code /proc/net/xt_qtaguid/stats}. */ private final File mStatsXtUid; - private boolean mUseBpfStats; + private final boolean mUseBpfStats; private INetd mNetdService; @@ -302,6 +302,17 @@ public class NetworkStatsFactory { return readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL); } + @GuardedBy("sPersistentDataLock") + private void requestSwapActiveStatsMapLocked() throws RemoteException { + // Ask netd to do a active map stats swap. When the binder call successfully returns, + // the system server should be able to safely read and clean the inactive map + // without race problem. + if (mNetdService == null) { + mNetdService = NetdService.getInstance(); + } + mNetdService.trafficSwapActiveStatsMap(); + } + /** * Reads the detailed UID stats based on the provided parameters * @@ -312,24 +323,6 @@ public class NetworkStatsFactory { * @return the NetworkStats instance containing network statistics at the present time. */ public NetworkStats readNetworkStatsDetail( - int limitUid, @Nullable String[] limitIfaces, int limitTag) throws IOException { - return readNetworkStatsDetailInternal(limitUid, limitIfaces, limitTag); - } - - @GuardedBy("sPersistentDataLock") - private void requestSwapActiveStatsMapLocked() throws RemoteException { - // Ask netd to do a active map stats swap. When the binder call successfully returns, - // the system server should be able to safely read and clean the inactive map - // without race problem. - if (mUseBpfStats) { - if (mNetdService == null) { - mNetdService = NetdService.getInstance(); - } - mNetdService.trafficSwapActiveStatsMap(); - } - } - - private NetworkStats readNetworkStatsDetailInternal( int limitUid, String[] limitIfaces, int limitTag) throws IOException { // In order to prevent deadlocks, anything protected by this lock MUST NOT call out to other // code that will acquire other locks within the system server. See b/134244752.