diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 225bd5830e..f8a10941b2 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -2033,5 +2033,13 @@ public class BpfCoordinator { return mBpfConntrackEventConsumer; } + // Return tethering client information. This is used for testing only. + @NonNull + @VisibleForTesting + final HashMap> + 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 81b6beb2f7..4967d27683 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 @@ public class BpfCoordinatorTest { 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 @@ public class BpfCoordinatorTest { 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 @@ public class BpfCoordinatorTest { 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 @@ public class BpfCoordinatorTest { 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 @@ public class BpfCoordinatorTest { 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 @@ public class BpfCoordinatorTest { // 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> mTetherClients; private long mElapsedRealtimeNanos = 0; private final ArgumentCaptor mStringArrayCaptor = @@ -482,6 +473,8 @@ public class BpfCoordinatorTest { final BpfCoordinator coordinator = new BpfCoordinator(mDeps); mConsumer = coordinator.getBpfConntrackEventConsumerForTesting(); + mTetherClients = coordinator.getTetherClientsForTesting(); + final ArgumentCaptor tetherStatsProviderCaptor = ArgumentCaptor.forClass(BpfCoordinator.BpfTetherStatsProvider.class); @@ -1943,14 +1936,41 @@ public class BpfCoordinatorTest { 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)); + } + + private void asseertClientInfoExist(@NonNull IpServer ipServer, + @NonNull ClientInfo clientInfo) { + HashMap 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 clients = mTetherClients.get(ipServer); + assertNotNull(clients); + assertNull(clients.get(clientInfo.clientAddress)); } - // TODO: check the client information is not removed. @Test @IgnoreUpTo(Build.VERSION_CODES.R) public void testTetherOffloadRule4Clear_ChangeOrRemoveUpstream() throws Exception { @@ -1982,5 +2002,64 @@ public class BpfCoordinatorTest { // 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)); } }