From f5390a9d8fb3e0831bff0974a2ca86228b240cc2 Mon Sep 17 00:00:00 2001 From: KH Shi Date: Mon, 29 May 2023 21:47:30 +0800 Subject: [PATCH] Add tests for OffloadHardwareInterface NativeHandles shouldn't be closed in OffloadHardwareInterface after passing them to initOffload() of AIDL service, because their owner will be changed to ParcelFileDescriptor by libbinder. Add test cases to verify that NativeHandles are closed correctly. NativeHandles should be closed after initOffload if: - Init with HIDL HAL interface - Has null NativeHandle, any non-null NativeHandles should be closed Bug: 277834236 Test: atest OffloadHardwareInterfaceTest Change-Id: Icbe74da73d3bdc582d1816b314fdfce9661465fc --- .../OffloadHardwareInterfaceTest.java | 99 ++++++++++++++----- 1 file changed, 74 insertions(+), 25 deletions(-) diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadHardwareInterfaceTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadHardwareInterfaceTest.java index b1f875b615..4413d268e2 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadHardwareInterfaceTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadHardwareInterfaceTest.java @@ -20,6 +20,8 @@ import static android.system.OsConstants.AF_INET; import static android.system.OsConstants.AF_UNIX; import static android.system.OsConstants.SOCK_STREAM; +import static com.android.networkstack.tethering.OffloadHardwareInterface.NF_NETLINK_CONNTRACK_DESTROY; +import static com.android.networkstack.tethering.OffloadHardwareInterface.NF_NETLINK_CONNTRACK_NEW; import static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_AIDL; import static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_HIDL_1_0; import static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_HIDL_1_1; @@ -34,6 +36,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -57,7 +60,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.FileDescriptor; @@ -75,8 +77,9 @@ public final class OffloadHardwareInterfaceTest { private OffloadHardwareInterface mOffloadHw; private OffloadHalCallback mOffloadHalCallback; - @Mock private IOffloadHal mIOffload; - @Mock private NativeHandle mNativeHandle; + private IOffloadHal mIOffload; + private NativeHandle mNativeHandle1; + private NativeHandle mNativeHandle2; // Random values to test Netlink message. private static final short TEST_TYPE = 184; @@ -97,7 +100,9 @@ public final class OffloadHardwareInterfaceTest { @Override public NativeHandle createConntrackSocket(final int groups) { - return mNativeHandle; + return groups == (NF_NETLINK_CONNTRACK_NEW | NF_NETLINK_CONNTRACK_DESTROY) + ? mNativeHandle1 + : mNativeHandle2; } } @@ -105,45 +110,89 @@ public final class OffloadHardwareInterfaceTest { public void setUp() { MockitoAnnotations.initMocks(this); mOffloadHalCallback = new OffloadHalCallback(); - when(mIOffload.initOffload(any(NativeHandle.class), any(NativeHandle.class), - any(OffloadHalCallback.class))).thenReturn(true); + mIOffload = mock(IOffloadHal.class); } - private void startOffloadHardwareInterface(int offloadHalVersion) + private void startOffloadHardwareInterface(int offloadHalVersion, boolean isHalInitSuccess) throws Exception { + startOffloadHardwareInterface(offloadHalVersion, isHalInitSuccess, mock(NativeHandle.class), + mock(NativeHandle.class)); + } + + private void startOffloadHardwareInterface(int offloadHalVersion, boolean isHalInitSuccess, + NativeHandle handle1, NativeHandle handle2) throws Exception { final SharedLog log = new SharedLog("test"); final Handler handler = new Handler(mTestLooper.getLooper()); - final int num = offloadHalVersion != OFFLOAD_HAL_VERSION_NONE ? 1 : 0; + final boolean hasNullHandle = handle1 == null || handle2 == null; + // If offloadHalVersion is OFFLOAD_HAL_VERSION_NONE or it has null NativeHandle arguments, + // mIOffload.initOffload() shouldn't be called. + final int initNum = (offloadHalVersion != OFFLOAD_HAL_VERSION_NONE && !hasNullHandle) + ? 1 + : 0; + // If it is HIDL or has null NativeHandle argument, NativeHandles should be closed. + final int handleCloseNum = (hasNullHandle + || offloadHalVersion == OFFLOAD_HAL_VERSION_HIDL_1_0 + || offloadHalVersion == OFFLOAD_HAL_VERSION_HIDL_1_1) ? 1 : 0; + mNativeHandle1 = handle1; + mNativeHandle2 = handle2; + when(mIOffload.initOffload(any(NativeHandle.class), any(NativeHandle.class), + any(OffloadHalCallback.class))).thenReturn(isHalInitSuccess); mOffloadHw = new OffloadHardwareInterface(handler, log, new MyDependencies(handler, log, offloadHalVersion)); - assertEquals(offloadHalVersion, mOffloadHw.initOffload(mOffloadHalCallback)); - verify(mIOffload, times(num)).initOffload(any(NativeHandle.class), any(NativeHandle.class), - eq(mOffloadHalCallback)); + assertEquals(isHalInitSuccess && !hasNullHandle + ? offloadHalVersion + : OFFLOAD_HAL_VERSION_NONE, + mOffloadHw.initOffload(mOffloadHalCallback)); + verify(mIOffload, times(initNum)).initOffload(any(NativeHandle.class), + any(NativeHandle.class), eq(mOffloadHalCallback)); + if (mNativeHandle1 != null) verify(mNativeHandle1, times(handleCloseNum)).close(); + if (mNativeHandle2 != null) verify(mNativeHandle2, times(handleCloseNum)).close(); } @Test public void testInitFailureWithNoHal() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_NONE); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_NONE, true); } @Test public void testInitSuccessWithAidl() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_AIDL); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_AIDL, true); } @Test public void testInitSuccessWithHidl_1_0() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); } @Test public void testInitSuccessWithHidl_1_1() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_1); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_1, true); + } + + @Test + public void testInitFailWithAidl() throws Exception { + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_AIDL, false); + } + + @Test + public void testInitFailWithHidl_1_0() throws Exception { + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, false); + } + + @Test + public void testInitFailWithHidl_1_1() throws Exception { + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_1, false); + } + + @Test + public void testInitFailDueToNullHandles() throws Exception { + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_AIDL, true, mock(NativeHandle.class), + null); } @Test public void testGetForwardedStats() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); ForwardedStats stats = new ForwardedStats(12345, 56780); when(mIOffload.getForwardedStats(anyString())).thenReturn(stats); assertEquals(mOffloadHw.getForwardedStats(RMNET0), stats); @@ -152,7 +201,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSetLocalPrefixes() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); final ArrayList localPrefixes = new ArrayList<>(); localPrefixes.add("127.0.0.0/8"); localPrefixes.add("fe80::/64"); @@ -165,7 +214,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSetDataLimit() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); final long limit = 12345; when(mIOffload.setDataLimit(anyString(), anyLong())).thenReturn(true); assertTrue(mOffloadHw.setDataLimit(RMNET0, limit)); @@ -177,7 +226,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSetDataWarningAndLimitFailureWithHidl_1_0() throws Exception { // Verify V1.0 control HAL would reject the function call with exception. - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); final long warning = 12345; final long limit = 67890; assertThrows(UnsupportedOperationException.class, @@ -187,7 +236,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSetDataWarningAndLimit() throws Exception { // Verify V1.1 control HAL could receive this function call. - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_1); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_1, true); final long warning = 12345; final long limit = 67890; when(mIOffload.setDataWarningAndLimit(anyString(), anyLong(), anyLong())).thenReturn(true); @@ -199,7 +248,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSetUpstreamParameters() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); final String v4addr = "192.168.10.1"; final String v4gateway = "192.168.10.255"; final ArrayList v6gws = new ArrayList<>(0); @@ -220,7 +269,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testUpdateDownstream() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); final String ifName = "wlan1"; final String prefix = "192.168.43.0/24"; when(mIOffload.addDownstream(anyString(), anyString())).thenReturn(true); @@ -237,7 +286,7 @@ public final class OffloadHardwareInterfaceTest { @Test public void testSendIpv4NfGenMsg() throws Exception { - startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0); + startOffloadHardwareInterface(OFFLOAD_HAL_VERSION_HIDL_1_0, true); FileDescriptor writeSocket = new FileDescriptor(); FileDescriptor readSocket = new FileDescriptor(); try { @@ -246,9 +295,9 @@ public final class OffloadHardwareInterfaceTest { fail(); return; } - when(mNativeHandle.getFileDescriptor()).thenReturn(writeSocket); + when(mNativeHandle1.getFileDescriptor()).thenReturn(writeSocket); - mOffloadHw.sendIpv4NfGenMsg(mNativeHandle, TEST_TYPE, TEST_FLAGS); + mOffloadHw.sendIpv4NfGenMsg(mNativeHandle1, TEST_TYPE, TEST_FLAGS); ByteBuffer buffer = ByteBuffer.allocate(9823); // Arbitrary value > expectedLen. buffer.order(ByteOrder.nativeOrder());