Add test for tetherOffloadClient{Add, Remove, Clear} am: dad664748a am: 32244e9034

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1744606

Change-Id: I15950033cd3d2c8f2830d1a089b8cd395c087e7f
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index 225bd58..f8a1094 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -2033,5 +2033,13 @@
         return mBpfConntrackEventConsumer;
     }
 
+    // Return tethering client information. This is used for testing only.
+    @NonNull
+    @VisibleForTesting
+    final HashMap<IpServer, HashMap<Inet4Address, ClientInfo>>
+            getTetherClientsForTesting() {
+        return mTetherClients;
+    }
+
     private static native String[] getBpfCounterNames();
 }
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
index 81b6beb..4967d27 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
@@ -224,8 +224,6 @@
             private int mSrcPort = PRIVATE_PORT;
             private int mDstPort = REMOTE_PORT;
 
-            Builder() {}
-
             public Builder setProto(int proto) {
                 if (proto != IPPROTO_TCP && proto != IPPROTO_UDP) {
                     fail("Not support protocol " + proto);
@@ -250,8 +248,6 @@
             private int mSrcPort = REMOTE_PORT;
             private int mDstPort = PUBLIC_PORT;
 
-            Builder() {}
-
             public Builder setProto(int proto) {
                 if (proto != IPPROTO_TCP && proto != IPPROTO_UDP) {
                     fail("Not support protocol " + proto);
@@ -279,8 +275,6 @@
             private int mDstPort = REMOTE_PORT;
             private long mLastUsed = 0;
 
-            Builder() {}
-
             public Tether4Value build() {
                 return new Tether4Value(mOif, mEthDstMac, mEthSrcMac, mEthProto, mPmtu,
                         mSrc46, mDst46, mSrcPort, mDstPort, mLastUsed);
@@ -301,8 +295,6 @@
             private int mDstPort = PRIVATE_PORT;
             private long mLastUsed = 0;
 
-            Builder() {}
-
             public Tether4Value build() {
                 return new Tether4Value(mOif, mEthDstMac, mEthSrcMac, mEthProto, mPmtu,
                         mSrc46, mDst46, mSrcPort, mDstPort, mLastUsed);
@@ -321,8 +313,6 @@
             private short mPublicPort = PUBLIC_PORT;
             private short mRemotePort = REMOTE_PORT;
 
-            Builder() {}
-
             public Builder setMsgType(short msgType) {
                 if (msgType != IPCTNL_MSG_CT_NEW && msgType != IPCTNL_MSG_CT_DELETE) {
                     fail("Not support message type " + msgType);
@@ -377,6 +367,7 @@
     // Late init since the object must be initialized by the BPF coordinator instance because
     // it has to access the non-static function of BPF coordinator.
     private BpfConntrackEventConsumer mConsumer;
+    private HashMap<IpServer, HashMap<Inet4Address, ClientInfo>> mTetherClients;
 
     private long mElapsedRealtimeNanos = 0;
     private final ArgumentCaptor<ArrayList> mStringArrayCaptor =
@@ -482,6 +473,8 @@
         final BpfCoordinator coordinator = new BpfCoordinator(mDeps);
 
         mConsumer = coordinator.getBpfConntrackEventConsumerForTesting();
+        mTetherClients = coordinator.getTetherClientsForTesting();
+
         final ArgumentCaptor<BpfCoordinator.BpfTetherStatsProvider>
                 tetherStatsProviderCaptor =
                 ArgumentCaptor.forClass(BpfCoordinator.BpfTetherStatsProvider.class);
@@ -1943,14 +1936,41 @@
         assertEquals(DOWNSTREAM4_RULE_VALUE_B, mBpfDownstream4Map.getValue(
                 DOWNSTREAM4_RULE_KEY_B));
 
+        // Clear client information for the first downstream only.
+        assertNull(mTetherClients.get(mIpServer));
+        assertNotNull(mTetherClients.get(mIpServer2));
+
         // [2] Remove the second downstream. Remove the rule set B which is on the second
         // downstream.
         coordinator.tetherOffloadClientClear(mIpServer2);
         assertNull(mBpfUpstream4Map.getValue(UPSTREAM4_RULE_KEY_B));
         assertNull(mBpfDownstream4Map.getValue(DOWNSTREAM4_RULE_KEY_B));
+
+        // Clear client information for the second downstream.
+        assertNull(mTetherClients.get(mIpServer2));
     }
 
-    // TODO: check the client information is not removed.
+    private void asseertClientInfoExist(@NonNull IpServer ipServer,
+            @NonNull ClientInfo clientInfo) {
+        HashMap<Inet4Address, ClientInfo> clients = mTetherClients.get(ipServer);
+        assertNotNull(clients);
+        assertEquals(clientInfo, clients.get(clientInfo.clientAddress));
+    }
+
+    // Although either ClientInfo for a given downstream (IpServer) is not found or a given
+    // client address is not found on a given downstream can be treated "ClientInfo not
+    // exist", we still want to know the real reason exactly. For example, we don't the
+    // exact reason in the following:
+    //   assertNull(clients == null ? clients : clients.get(clientInfo.clientAddress));
+    // This helper only verifies the case that the downstream still has at least one client.
+    // In other words, ClientInfo for a given IpServer has not been removed yet.
+    private void asseertClientInfoNotExist(@NonNull IpServer ipServer,
+            @NonNull ClientInfo clientInfo) {
+        HashMap<Inet4Address, ClientInfo> clients = mTetherClients.get(ipServer);
+        assertNotNull(clients);
+        assertNull(clients.get(clientInfo.clientAddress));
+    }
+
     @Test
     @IgnoreUpTo(Build.VERSION_CODES.R)
     public void testTetherOffloadRule4Clear_ChangeOrRemoveUpstream() throws Exception {
@@ -1982,5 +2002,64 @@
         // all rules.
         setUpstreamInformationTo(coordinator, INVALID_IFINDEX);
         checkRule4NotExistInUpstreamDownstreamMap();
+
+        // Client information should be not deleted.
+        asseertClientInfoExist(mIpServer, CLIENT_INFO_A);
+        asseertClientInfoExist(mIpServer2, CLIENT_INFO_B);
+    }
+
+    @Test
+    @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testTetherOffloadClientAddRemove() throws Exception {
+        final BpfCoordinator coordinator = makeBpfCoordinator();
+
+        // [1] Add client information A and B on on the same downstream.
+        final ClientInfo clientA = new ClientInfo(DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC,
+                PRIVATE_ADDR, MAC_A);
+        final ClientInfo clientB = new ClientInfo(DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC,
+                PRIVATE_ADDR2, MAC_B);
+        coordinator.tetherOffloadClientAdd(mIpServer, clientA);
+        coordinator.tetherOffloadClientAdd(mIpServer, clientB);
+        asseertClientInfoExist(mIpServer, clientA);
+        asseertClientInfoExist(mIpServer, clientB);
+
+        // Add the rules for client A and client B.
+        final Tether4Key upstream4KeyA = makeUpstream4Key(
+                DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, PRIVATE_ADDR, PRIVATE_PORT);
+        final Tether4Value upstream4ValueA = makeUpstream4Value(PUBLIC_PORT);
+        final Tether4Key downstream4KeyA = makeDownstream4Key(PUBLIC_PORT);
+        final Tether4Value downstream4ValueA = makeDownstream4Value(
+                DOWNSTREAM_IFINDEX, MAC_A, DOWNSTREAM_MAC, PRIVATE_ADDR, PRIVATE_PORT);
+        final Tether4Key upstream4KeyB = makeUpstream4Key(
+                DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC2, PRIVATE_ADDR2, PRIVATE_PORT2);
+        final Tether4Value upstream4ValueB = makeUpstream4Value(PUBLIC_PORT2);
+        final Tether4Key downstream4KeyB = makeDownstream4Key(PUBLIC_PORT2);
+        final Tether4Value downstream4ValueB = makeDownstream4Value(
+                DOWNSTREAM_IFINDEX, MAC_B, DOWNSTREAM_MAC2, PRIVATE_ADDR2, PRIVATE_PORT2);
+
+        mBpfUpstream4Map.insertEntry(upstream4KeyA, upstream4ValueA);
+        mBpfDownstream4Map.insertEntry(downstream4KeyA, downstream4ValueA);
+        mBpfUpstream4Map.insertEntry(upstream4KeyB, upstream4ValueB);
+        mBpfDownstream4Map.insertEntry(downstream4KeyB, downstream4ValueB);
+
+        // [2] Remove client information A. Only the rules on client A should be removed and
+        // the rules on client B should exist.
+        coordinator.tetherOffloadClientRemove(mIpServer, clientA);
+        asseertClientInfoNotExist(mIpServer, clientA);
+        asseertClientInfoExist(mIpServer, clientB);
+        assertNull(mBpfUpstream4Map.getValue(upstream4KeyA));
+        assertNull(mBpfDownstream4Map.getValue(downstream4KeyA));
+        assertEquals(upstream4ValueB, mBpfUpstream4Map.getValue(upstream4KeyB));
+        assertEquals(downstream4ValueB, mBpfDownstream4Map.getValue(downstream4KeyB));
+
+        // [3] Remove client information B. The rules on client B should be removed.
+        // Exactly, ClientInfo for a given IpServer is removed because the last client B
+        // has been removed from the downstream. Can't use the helper #asseertClientInfoExist
+        // to check because the container ClientInfo for a given downstream has been removed.
+        // See #asseertClientInfoExist.
+        coordinator.tetherOffloadClientRemove(mIpServer, clientB);
+        assertNull(mTetherClients.get(mIpServer));
+        assertNull(mBpfUpstream4Map.getValue(upstream4KeyB));
+        assertNull(mBpfDownstream4Map.getValue(downstream4KeyB));
     }
 }