Merge "Add forwarding methods to RoutingCoordinator" into main

Tethering/src/android/net/ip/IpServer.java

@@ -839,7 +839,38 @@ public class IpServer extends StateMachineShim {
             }
         } catch (ServiceSpecificException | RemoteException e) {
             mLog.e("Failed to add " + mIfaceName + " to local table: ", e);
-            return;
+        }
+    }
+
+    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);
+            }
         }
     }
 
@@ -1349,16 +1380,7 @@ public class IpServer extends StateMachineShim {
             // to remove their rules, which generates errors.
             // Just do the best we can.
             mBpfCoordinator.maybeDetachProgram(mIfaceName, upstreamIface);
-            try {
+            removeInterfaceForward(mIfaceName, upstreamIface);
-                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
@@ -1414,10 +1436,9 @@ public class IpServer extends StateMachineShim {
 
                         mBpfCoordinator.maybeAttachProgram(mIfaceName, ifname);
                         try {
-                            mNetd.tetherAddForward(mIfaceName, ifname);
+                            addInterfaceForward(mIfaceName, ifname);
-                            mNetd.ipfwdAddInterfaceForward(mIfaceName, ifname);
                         } catch (RemoteException | ServiceSpecificException e) {
-                            mLog.e("Exception enabling NAT: " + e.toString());
+                            mLog.e("Exception enabling iface forward", e);
                             cleanupUpstream();
                             mLastError = TETHER_ERROR_ENABLE_FORWARDING_ERROR;
                             transitionTo(mInitialState);

Tethering/tests/unit/src/android/net/ip/IpServerTest.java

@@ -322,6 +322,23 @@ 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() {

framework/src/android/net/IRoutingCoordinator.aidl

@@ -72,4 +72,24 @@ 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);
 }

framework/src/android/net/RoutingCoordinatorManager.java

@@ -123,4 +123,36 @@ 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();
+        }
+    }
 }

service/src/com/android/server/connectivity/RoutingCoordinatorService.java

@@ -19,12 +19,16 @@ 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.
@@ -115,4 +119,89 @@ 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");
+            }
+        }
+    }
 }

tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt

@@ -0,0 +1,65 @@
+/*
+ * 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())
+    }
+}