Merge "Update error handling in BpfNetMaps"

service-t/src/com/android/server/net/NetworkStatsFactory.java

@@ -164,16 +164,17 @@ public class NetworkStatsFactory {
     }
 
     public NetworkStatsFactory(@NonNull Context ctx) {
-        this(ctx, new File("/proc/"), true);
+        this(ctx, new File("/proc/"), true, new BpfNetMaps());
     }
 
     @VisibleForTesting
-    public NetworkStatsFactory(@NonNull Context ctx, File procRoot, boolean useBpfStats) {
+    public NetworkStatsFactory(@NonNull Context ctx, File procRoot, boolean useBpfStats,
+            BpfNetMaps bpfNetMaps) {
         mStatsXtIfaceAll = new File(procRoot, "net/xt_qtaguid/iface_stat_all");
         mStatsXtIfaceFmt = new File(procRoot, "net/xt_qtaguid/iface_stat_fmt");
         mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats");
         mUseBpfStats = useBpfStats;
-        mBpfNetMaps = new BpfNetMaps();
+        mBpfNetMaps = bpfNetMaps;
         synchronized (mPersistentDataLock) {
             mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1);
             mTunAnd464xlatAdjustedStats = new NetworkStats(SystemClock.elapsedRealtime(), -1);

service/src/com/android/server/BpfNetMaps.java

@@ -49,10 +49,16 @@ import java.io.IOException;
  * {@hide}
  */
 public class BpfNetMaps {
+    private static final boolean PRE_T = !SdkLevel.isAtLeastT();
+    static {
+        if (!PRE_T) {
+            System.loadLibrary("service-connectivity");
+        }
+    }
+
     private static final String TAG = "BpfNetMaps";
     private final INetd mNetd;
     // Use legacy netd for releases before T.
-    private static final boolean PRE_T = !SdkLevel.isAtLeastT();
     private static boolean sInitialized = false;
 
     // Lock for sConfigurationMap entry for UID_RULES_CONFIGURATION_KEY.
@@ -98,11 +104,26 @@ public class BpfNetMaps {
     }
 
     /**
-     * Only tests or BpfNetMaps#ensureInitialized can call this function.
+     * Set configurationMap for test.
      */
     @VisibleForTesting
-    public static void initialize(final Dependencies deps) {
+    public static void setConfigurationMapForTest(BpfMap<U32, U32> configurationMap) {
-        sConfigurationMap = deps.getConfigurationMap();
+        sConfigurationMap = configurationMap;
+    }
+
+    private static BpfMap<U32, U32> getConfigurationMap() {
+        try {
+            return new BpfMap<>(
+                    CONFIGURATION_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, U32.class);
+        } catch (ErrnoException e) {
+            throw new IllegalStateException("Cannot open netd configuration map", e);
+        }
+    }
+
+    private static void setBpfMaps() {
+        if (sConfigurationMap == null) {
+            sConfigurationMap = getConfigurationMap();
+        }
     }
 
     /**
@@ -111,33 +132,11 @@ public class BpfNetMaps {
      */
     private static synchronized void ensureInitialized() {
         if (sInitialized) return;
-        if (!PRE_T) {
+        setBpfMaps();
-            System.loadLibrary("service-connectivity");
+        native_init();
-            native_init();
-            initialize(new Dependencies());
-        }
         sInitialized = true;
     }
 
-    /**
-     * Dependencies of BpfNetMaps, for injection in tests.
-     */
-    @VisibleForTesting
-    public static class Dependencies {
-        /**
-         *  Get configuration BPF map.
-         */
-        public BpfMap<U32, U32> getConfigurationMap() {
-            try {
-                return new BpfMap<>(
-                        CONFIGURATION_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, U32.class);
-            } catch (ErrnoException e) {
-                Log.e(TAG, "Cannot open netd configuration map: " + e);
-                return null;
-            }
-        }
-    }
-
     /** Constructor used after T that doesn't need to use netd anymore. */
     public BpfNetMaps() {
         this(null);
@@ -146,7 +145,9 @@ public class BpfNetMaps {
     }
 
     public BpfNetMaps(final INetd netd) {
-        ensureInitialized();
+        if (!PRE_T) {
+            ensureInitialized();
+        }
         mNetd = netd;
     }
 

tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt

@@ -47,6 +47,7 @@ import android.util.Log
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
 import com.android.connectivity.resources.R
+import com.android.server.BpfNetMaps
 import com.android.server.ConnectivityService
 import com.android.server.NetworkAgentWrapper
 import com.android.server.TestNetIdManager
@@ -208,6 +209,7 @@ class ConnectivityServiceIntegrationTest {
         doReturn(mock(ProxyTracker::class.java)).`when`(deps).makeProxyTracker(any(), any())
         doReturn(mock(MockableSystemProperties::class.java)).`when`(deps).systemProperties
         doReturn(TestNetIdManager()).`when`(deps).makeNetIdManager()
+        doReturn(mock(BpfNetMaps::class.java)).`when`(deps).getBpfNetMaps(any())
         doAnswer { inv ->
             object : MultinetworkPolicyTracker(inv.getArgument(0), inv.getArgument(1),
                     inv.getArgument(2)) {

tests/unit/java/com/android/server/BpfNetMapsTest.java

@@ -85,24 +85,13 @@ public final class BpfNetMapsTest {
     private BpfNetMaps mBpfNetMaps;
 
     @Mock INetd mNetd;
-    private static final TestBpfMap<U32, U32> sConfigurationMap =
+    private final BpfMap<U32, U32> mConfigurationMap = new TestBpfMap<>(U32.class, U32.class);
-            new TestBpfMap<>(U32.class, U32.class);
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
+        BpfNetMaps.setConfigurationMapForTest(mConfigurationMap);
         mBpfNetMaps = new BpfNetMaps(mNetd);
-        BpfNetMaps.initialize(makeDependencies());
-        sConfigurationMap.clear();
-    }
-
-    private static BpfNetMaps.Dependencies makeDependencies() {
-        return new BpfNetMaps.Dependencies() {
-            @Override
-            public BpfMap<U32, U32> getConfigurationMap() {
-                return sConfigurationMap;
-            }
-        };
     }
 
     @Test
@@ -121,7 +110,7 @@ public final class BpfNetMapsTest {
         for (final int chain: enableChains) {
             match |= mBpfNetMaps.getMatchByFirewallChain(chain);
         }
-        sConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(match));
+        mConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(match));
 
         for (final int chain: FIREWALL_CHAINS) {
             final String testCase = "EnabledChains: " + enableChains + " CheckedChain: " + chain;
@@ -187,17 +176,17 @@ public final class BpfNetMapsTest {
             expectedMatch |= mBpfNetMaps.getMatchByFirewallChain(chain);
         }
 
-        assertEquals(0, sConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
+        assertEquals(0, mConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
 
         for (final int chain: testChains) {
             mBpfNetMaps.setChildChain(chain, true /* enable */);
         }
-        assertEquals(expectedMatch, sConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
+        assertEquals(expectedMatch, mConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
 
         for (final int chain: testChains) {
             mBpfNetMaps.setChildChain(chain, false /* enable */);
         }
-        assertEquals(0, sConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
+        assertEquals(0, mConfigurationMap.getValue(UID_RULES_CONFIGURATION_KEY).val);
     }
 
     private void doTestSetChildChain(final int testChain) throws Exception {
@@ -207,7 +196,7 @@ public final class BpfNetMapsTest {
     @Test
     @IgnoreUpTo(Build.VERSION_CODES.S_V2)
     public void testSetChildChain() throws Exception {
-        sConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(0));
+        mConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(0));
         doTestSetChildChain(FIREWALL_CHAIN_DOZABLE);
         doTestSetChildChain(FIREWALL_CHAIN_STANDBY);
         doTestSetChildChain(FIREWALL_CHAIN_POWERSAVE);
@@ -221,7 +210,7 @@ public final class BpfNetMapsTest {
     @Test
     @IgnoreUpTo(Build.VERSION_CODES.S_V2)
     public void testSetChildChainMultipleChain() throws Exception {
-        sConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(0));
+        mConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(0));
         doTestSetChildChain(List.of(
                 FIREWALL_CHAIN_DOZABLE,
                 FIREWALL_CHAIN_STANDBY));

tests/unit/java/com/android/server/net/NetworkStatsFactoryTest.java

@@ -44,6 +44,7 @@ import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
 
 import com.android.frameworks.tests.net.R;
+import com.android.server.BpfNetMaps;
 import com.android.testutils.DevSdkIgnoreRule;
 import com.android.testutils.DevSdkIgnoreRunner;
 
@@ -74,6 +75,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
     private File mTestProc;
     private NetworkStatsFactory mFactory;
     @Mock private Context mContext;
+    @Mock private BpfNetMaps mBpfNetMaps;
 
     @Before
     public void setUp() throws Exception {
@@ -84,7 +86,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
         // applications. So in order to have a test support native library, the native code
         // related to networkStatsFactory is compiled to a minimal native library and loaded here.
         System.loadLibrary("networkstatsfactorytestjni");
-        mFactory = new NetworkStatsFactory(mContext, mTestProc, false);
+        mFactory = new NetworkStatsFactory(mContext, mTestProc, false, mBpfNetMaps);
         mFactory.updateUnderlyingNetworkInfos(new UnderlyingNetworkInfo[0]);
     }