diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index 2a25a86f34c49f2fd1b2c53107204b541607c749..246e5bcce2419b2a469f80176554ba264ab37dc0 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -827,38 +827,7 @@ public class IpServer extends StateMachine { } } catch (ServiceSpecificException | RemoteException e) { mLog.e("Failed to add " + mIfaceName + " to local table: ", e); - } - } - - private void addInterfaceForward(@NonNull final String fromIface, - @NonNull final String toIface) throws ServiceSpecificException, RemoteException { - if (null != mRoutingCoordinator.value) { - mRoutingCoordinator.value.addInterfaceForward(fromIface, toIface); - } else { - mNetd.tetherAddForward(fromIface, toIface); - mNetd.ipfwdAddInterfaceForward(fromIface, toIface); - } - } - - private void removeInterfaceForward(@NonNull final String fromIface, - @NonNull final String toIface) { - if (null != mRoutingCoordinator.value) { - try { - mRoutingCoordinator.value.removeInterfaceForward(fromIface, toIface); - } catch (ServiceSpecificException e) { - mLog.e("Exception in removeInterfaceForward", e); - } - } else { - try { - mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); - } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception in ipfwdRemoveInterfaceForward", e); - } - try { - mNetd.tetherRemoveForward(fromIface, toIface); - } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception in disableNat", e); - } + return; } } @@ -1368,7 +1337,16 @@ public class IpServer extends StateMachine { // to remove their rules, which generates errors. // Just do the best we can. mBpfCoordinator.maybeDetachProgram(mIfaceName, upstreamIface); - removeInterfaceForward(mIfaceName, upstreamIface); + try { + mNetd.ipfwdRemoveInterfaceForward(mIfaceName, upstreamIface); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception in ipfwdRemoveInterfaceForward: " + e.toString()); + } + try { + mNetd.tetherRemoveForward(mIfaceName, upstreamIface); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception in disableNat: " + e.toString()); + } } @Override @@ -1424,9 +1402,10 @@ public class IpServer extends StateMachine { mBpfCoordinator.maybeAttachProgram(mIfaceName, ifname); try { - addInterfaceForward(mIfaceName, ifname); + mNetd.tetherAddForward(mIfaceName, ifname); + mNetd.ipfwdAddInterfaceForward(mIfaceName, ifname); } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception enabling iface forward", e); + mLog.e("Exception enabling NAT: " + e.toString()); cleanupUpstream(); mLastError = TETHER_ERROR_ENABLE_FORWARDING_ERROR; transitionTo(mInitialState); diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 15df3babcd59a8f5ac7e26d19297e4b36c7ad386..fc9928d6a7a29f610dcfc72fbd218ad412006069 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -321,23 +321,6 @@ public class IpServerTest { when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(DEFAULT_USING_BPF_OFFLOAD); when(mTetherConfig.useLegacyDhcpServer()).thenReturn(false /* default value */); - // Simulate the behavior of RoutingCoordinator - if (null != mRoutingCoordinatorManager.value) { - doAnswer(it -> { - final String fromIface = (String) it.getArguments()[0]; - final String toIface = (String) it.getArguments()[1]; - mNetd.tetherAddForward(fromIface, toIface); - mNetd.ipfwdAddInterfaceForward(fromIface, toIface); - return null; - }).when(mRoutingCoordinatorManager.value).addInterfaceForward(any(), any()); - doAnswer(it -> { - final String fromIface = (String) it.getArguments()[0]; - final String toIface = (String) it.getArguments()[1]; - mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); - mNetd.tetherRemoveForward(fromIface, toIface); - return null; - }).when(mRoutingCoordinatorManager.value).removeInterfaceForward(any(), any()); - } mBpfDeps = new BpfCoordinator.Dependencies() { @NonNull public Handler getHandler() { diff --git a/framework/src/android/net/IRoutingCoordinator.aidl b/framework/src/android/net/IRoutingCoordinator.aidl index cf02ec47aa42de9e8ed52ff7d94455fa8a333841..a5cda9874e1606337e02e693bcd8c3efa5ca3540 100644 --- a/framework/src/android/net/IRoutingCoordinator.aidl +++ b/framework/src/android/net/IRoutingCoordinator.aidl @@ -72,24 +72,4 @@ interface IRoutingCoordinator { * unix errno. */ void removeInterfaceFromNetwork(int netId, in String iface); - - /** - * Add forwarding ip rule - * - * @param fromIface interface name to add forwarding ip rule - * @param toIface interface name to add forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - void addInterfaceForward(in String fromIface, in String toIface); - - /** - * Remove forwarding ip rule - * - * @param fromIface interface name to remove forwarding ip rule - * @param toIface interface name to remove forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - void removeInterfaceForward(in String fromIface, in String toIface); } diff --git a/framework/src/android/net/RoutingCoordinatorManager.java b/framework/src/android/net/RoutingCoordinatorManager.java index a9e7eef51e2c5381dea9cedb4eeffdc79fb4e633..5576cb0f23316b3bef880d338cf5a3111c122e55 100644 --- a/framework/src/android/net/RoutingCoordinatorManager.java +++ b/framework/src/android/net/RoutingCoordinatorManager.java @@ -123,36 +123,4 @@ public class RoutingCoordinatorManager { throw e.rethrowFromSystemServer(); } } - - /** - * Add forwarding ip rule - * - * @param fromIface interface name to add forwarding ip rule - * @param toIface interface name to add forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void addInterfaceForward(final String fromIface, final String toIface) { - try { - mService.addInterfaceForward(fromIface, toIface); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - } - - /** - * Remove forwarding ip rule - * - * @param fromIface interface name to remove forwarding ip rule - * @param toIface interface name to remove forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void removeInterfaceForward(final String fromIface, final String toIface) { - try { - mService.removeInterfaceForward(fromIface, toIface); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - } } diff --git a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java index 92ea610235f52e977a86f20704328c0e4f5d4827..50e84d4d220209efc19ba63066afe6c6f2bb3b4d 100644 --- a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java +++ b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java @@ -19,16 +19,12 @@ package com.android.server.connectivity; import static com.android.net.module.util.NetdUtils.toRouteInfoParcel; import android.annotation.NonNull; +import android.content.Context; import android.net.INetd; import android.net.IRoutingCoordinator; import android.net.RouteInfo; import android.os.RemoteException; import android.os.ServiceSpecificException; -import android.util.ArraySet; - -import com.android.internal.annotations.GuardedBy; - -import java.util.Objects; /** * Class to coordinate routing across multiple clients. @@ -119,89 +115,4 @@ public class RoutingCoordinatorService extends IRoutingCoordinator.Stub { throws ServiceSpecificException, RemoteException { mNetd.networkRemoveInterface(netId, iface); } - - private final Object mIfacesLock = new Object(); - private static final class ForwardingPair { - @NonNull public final String fromIface; - @NonNull public final String toIface; - ForwardingPair(@NonNull final String fromIface, @NonNull final String toIface) { - this.fromIface = fromIface; - this.toIface = toIface; - } - - @Override - public boolean equals(final Object o) { - if (this == o) return true; - if (!(o instanceof ForwardingPair)) return false; - - final ForwardingPair that = (ForwardingPair) o; - - return fromIface.equals(that.fromIface) && toIface.equals(that.toIface); - } - - @Override - public int hashCode() { - int result = fromIface.hashCode(); - result = 2 * result + toIface.hashCode(); - return result; - } - } - - @GuardedBy("mIfacesLock") - private final ArraySet<ForwardingPair> mForwardedInterfaces = new ArraySet<>(); - - /** - * Add forwarding ip rule - * - * @param fromIface interface name to add forwarding ip rule - * @param toIface interface name to add forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void addInterfaceForward(final String fromIface, final String toIface) - throws ServiceSpecificException, RemoteException { - Objects.requireNonNull(fromIface); - Objects.requireNonNull(toIface); - synchronized (mIfacesLock) { - if (mForwardedInterfaces.size() == 0) { - mNetd.ipfwdEnableForwarding("RoutingCoordinator"); - } - final ForwardingPair fwp = new ForwardingPair(fromIface, toIface); - if (mForwardedInterfaces.contains(fwp)) { - throw new IllegalStateException("Forward already exists between ifaces " - + fromIface + " → " + toIface); - } - mForwardedInterfaces.add(fwp); - // Enables NAT for v4 and filters packets from unknown interfaces - mNetd.tetherAddForward(fromIface, toIface); - mNetd.ipfwdAddInterfaceForward(fromIface, toIface); - } - } - - /** - * Remove forwarding ip rule - * - * @param fromIface interface name to remove forwarding ip rule - * @param toIface interface name to remove forwarding ip rule - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void removeInterfaceForward(final String fromIface, final String toIface) - throws ServiceSpecificException, RemoteException { - Objects.requireNonNull(fromIface); - Objects.requireNonNull(toIface); - synchronized (mIfacesLock) { - final ForwardingPair fwp = new ForwardingPair(fromIface, toIface); - if (!mForwardedInterfaces.contains(fwp)) { - throw new IllegalStateException("No forward set up between interfaces " - + fromIface + " → " + toIface); - } - mForwardedInterfaces.remove(fwp); - mNetd.ipfwdRemoveInterfaceForward(fromIface, toIface); - mNetd.tetherRemoveForward(fromIface, toIface); - if (mForwardedInterfaces.size() == 0) { - mNetd.ipfwdDisableForwarding("RoutingCoordinator"); - } - } - } } diff --git a/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt b/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt deleted file mode 100644 index 8adf309aadab26416df148f79a4914dbf680f959..0000000000000000000000000000000000000000 --- a/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.server.connectivity - -import android.net.INetd -import android.os.Build -import androidx.test.platform.app.InstrumentationRegistry -import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo -import com.android.testutils.DevSdkIgnoreRunner -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito.inOrder -import org.mockito.Mockito.mock -import kotlin.test.assertFailsWith - -@RunWith(DevSdkIgnoreRunner::class) -@IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) -class RoutingCoordinatorServiceTest { - val mNetd = mock(INetd::class.java) - val mService = RoutingCoordinatorService(mNetd) - - @Test - fun testInterfaceForward() { - val inOrder = inOrder(mNetd) - - mService.addInterfaceForward("from1", "to1") - inOrder.verify(mNetd).ipfwdEnableForwarding(any()) - inOrder.verify(mNetd).tetherAddForward("from1", "to1") - inOrder.verify(mNetd).ipfwdAddInterfaceForward("from1", "to1") - - mService.addInterfaceForward("from2", "to1") - inOrder.verify(mNetd).tetherAddForward("from2", "to1") - inOrder.verify(mNetd).ipfwdAddInterfaceForward("from2", "to1") - - assertFailsWith<IllegalStateException> { - // Can't add the same pair again - mService.addInterfaceForward("from2", "to1") - } - - mService.removeInterfaceForward("from1", "to1") - inOrder.verify(mNetd).ipfwdRemoveInterfaceForward("from1", "to1") - inOrder.verify(mNetd).tetherRemoveForward("from1", "to1") - - mService.removeInterfaceForward("from2", "to1") - inOrder.verify(mNetd).ipfwdRemoveInterfaceForward("from2", "to1") - inOrder.verify(mNetd).tetherRemoveForward("from2", "to1") - - inOrder.verify(mNetd).ipfwdDisableForwarding(any()) - } -}