diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index f3a0af3f81..6d3ca04692 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -26,6 +26,7 @@ import static android.net.TestNetworkManager.TEST_TAP_PREFIX; import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE; import static com.android.net.module.util.netlink.NetlinkConstants.IFF_UP; +import static com.android.net.module.util.netlink.NetlinkConstants.RTM_GETLINK; import android.annotation.NonNull; import android.annotation.Nullable; @@ -64,12 +65,15 @@ import com.android.net.module.util.netlink.NetlinkMessage; import com.android.net.module.util.netlink.NetlinkUtils; import com.android.net.module.util.netlink.RtNetlinkLinkMessage; import com.android.net.module.util.netlink.StructIfinfoMsg; +import com.android.net.module.util.netlink.StructNlMsgHdr; import com.android.server.connectivity.ConnectivityResources; import java.io.FileDescriptor; import java.net.InetAddress; import java.net.NetworkInterface; import java.net.SocketException; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.util.ArrayList; import java.util.Enumeration; import java.util.Iterator; @@ -145,7 +149,7 @@ public class EthernetTracker { private final Handler mHandler; private final EthernetNetworkFactory mFactory; private final EthernetConfigStore mConfigStore; - private final NetlinkMonitor mNetlinkMonitor; + private final EthernetNetlinkMonitor mNetlinkMonitor; private final Dependencies mDeps; private final RemoteCallbackList mListeners = @@ -199,11 +203,34 @@ public class EthernetTracker { OsConstants.NETLINK_ROUTE, NetlinkConstants.RTMGRP_LINK); } + /** Request RTM_GETLINK dump. Resulting callbacks are handled by #processNetlinkMessage. */ + public void requestLinkDump() { + // TODO(b/422484024): Clean up NetlinkMessage classes and add support for building a + // generic GETLINK message. + // Allocate enough space for struct nlmsghdr + struct rtgenmsg + final ByteBuffer buf = ByteBuffer.allocate(StructNlMsgHdr.STRUCT_SIZE + 1); + buf.order(ByteOrder.nativeOrder()); + + final StructNlMsgHdr nlmsghdr = new StructNlMsgHdr(); + nlmsghdr.nlmsg_len = buf.capacity(); + nlmsghdr.nlmsg_type = RTM_GETLINK; + nlmsghdr.nlmsg_flags = StructNlMsgHdr.NLM_F_DUMP | StructNlMsgHdr.NLM_F_REQUEST; + nlmsghdr.pack(buf); + + // struct rtgenmsg { + // unsigned char rtgen_family; + // } + buf.put((byte) OsConstants.AF_UNSPEC); + buf.flip(); + + sendNetlinkMessage(buf); + } + private void onNewLink(EthernetPort port, boolean linkUp) { final String ifname = port.getInterfaceName(); if (!mFactory.hasInterface(ifname) && !ifname.equals(mTetheringInterface)) { Log.i(TAG, "onInterfaceAdded: " + port); - maybeTrackInterface(ifname); + maybeTrackInterface(port); } Log.i(TAG, "interfaceLinkStateChanged: " + port + ", up: " + linkUp); updateInterfaceState(ifname, linkUp); @@ -310,7 +337,7 @@ public class EthernetTracker { mHandler.post(() -> { mNetlinkMonitor.start(); - trackAvailableInterfaces(); + mNetlinkMonitor.requestLinkDump(); }); } @@ -503,7 +530,7 @@ public class EthernetTracker { mHandler.post(() -> { mIncludeTestInterfaces = include; if (include) { - trackAvailableInterfaces(); + mNetlinkMonitor.requestLinkDump(); } else { removeTestData(); // remove all test interfaces @@ -743,14 +770,15 @@ public class EthernetTracker { mTetheredInterfaceWasAvailable = available; } - private void maybeTrackInterface(String iface) { + private void maybeTrackInterface(EthernetPort port) { + final String iface = port.getInterfaceName(); // If we don't already track this interface, and if this interface matches // our regex, start tracking it. if (mFactory.hasInterface(iface) || iface.equals(mTetheringInterface)) { - if (DBG) Log.w(TAG, "Ignoring already-tracked interface " + iface); + if (DBG) Log.w(TAG, "Ignoring already-tracked " + port); return; } - if (DBG) Log.i(TAG, "maybeTrackInterface: " + iface); + if (DBG) Log.i(TAG, "maybeTrackInterface: " + port); // Do not use an interface for tethering if it has configured NetworkCapabilities. if (mTetheringInterface == null && !mNetworkCapabilities.containsKey(iface)) { @@ -762,13 +790,6 @@ public class EthernetTracker { broadcastInterfaceStateChange(iface); } - private void trackAvailableInterfaces() { - final List ifaces = getEthernetInterfaceList(); - for (String iface : ifaces) { - maybeTrackInterface(iface); - } - } - private static class ListenerInfo { boolean canUseRestrictedNetworks = false; diff --git a/staticlibs/device/com/android/net/module/util/FdEventsReader.java b/staticlibs/device/com/android/net/module/util/FdEventsReader.java index f88883bf22..e6c01416b9 100644 --- a/staticlibs/device/com/android/net/module/util/FdEventsReader.java +++ b/staticlibs/device/com/android/net/module/util/FdEventsReader.java @@ -141,6 +141,11 @@ public abstract class FdEventsReader { return mPacketsReceived; } + /** Returns the FileDescriptor if the FdEventsReader is running, null otherwise. */ + protected FileDescriptor getFd() { + return isRunning() ? mFd : null; + } + /** * Subclasses MUST create the listening socket here, including setting all desired socket * options, interface or address/port binding, etc. The socket MUST be created nonblocking. diff --git a/staticlibs/device/com/android/net/module/util/ip/NetlinkMonitor.java b/staticlibs/device/com/android/net/module/util/ip/NetlinkMonitor.java index 15a46337a3..b4309e76ba 100644 --- a/staticlibs/device/com/android/net/module/util/ip/NetlinkMonitor.java +++ b/staticlibs/device/com/android/net/module/util/ip/NetlinkMonitor.java @@ -41,6 +41,7 @@ import com.android.net.module.util.netlink.NetlinkMessage; import com.android.net.module.util.netlink.NetlinkUtils; import java.io.FileDescriptor; +import java.io.InterruptedIOException; import java.net.SocketAddress; import java.net.SocketException; import java.nio.ByteBuffer; @@ -95,6 +96,29 @@ public class NetlinkMonitor extends PacketReader { this(h, log, tag, family, bindGroups, DEFAULT_SOCKET_RECV_BUFSIZE); } + /** + * Send a netlink message on the underlying netlink socket. + * + * Any responses will be processed by {@link #processNetlinkMessage}. This is particularly + * useful to request a dump of the current state when the {@link NetlinkMonitor} is first + * created. + * This prevents having an application to open multiple netlink sockets of the same family (for + * example by using a utility function in {@link NetlinkUtils} which can introduce races. + * + * TODO(b/422484024): add builder support to NetlinkMessage and accept a NetlinkMessage object + * rather than a ByteBuffer. + */ + public void sendNetlinkMessage(ByteBuffer msg) { + if (!isRunning()) throw new IllegalStateException("NetlinkMonitor must be running"); + + final FileDescriptor fd = getFd(); + try { + Os.write(fd, msg); + } catch (ErrnoException | InterruptedIOException e) { + Log.e(mTag, "Failed to send netlink message", e); + } + } + @Override protected FileDescriptor createFd() { FileDescriptor fd = null; diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt index b1b237ed1d..6b3bfdaaf9 100644 --- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt @@ -667,6 +667,36 @@ class EthernetManagerTest { listener.assertNoCallback() } + @Test + fun testCallbacks_withRunningInterfaces() { + // Do not run this test when unrestricted interfaces are present. Refer to testCallbacks. + assumeNoInterfaceForTetheringAvailable() + + // The interfaces must be created before setIncludeTestInterfaces is set to false, so the + // test can ensure that the RTM_NEWLINK has been processed before proceeding. + val iface1 = createInterface() + val iface2 = createInterface() + val iface3 = createInterface() + + // Prevent test interfaces from being tracked and wait for the call to be processed by + // calling setEthernetEnabled() which waits on a completion callback. + setIncludeTestInterfaces(false) + setEthernetEnabled(true) + + val listener = EthernetStateListener() + addInterfaceStateListener(listener) + listener.assertNoCallback() + + // Include test interfaces again, and ensure that they are tracked properly. + setIncludeTestInterfaces(true) + setEthernetEnabled(true) + + listener.expectCallback(iface1, STATE_LINK_UP, ROLE_CLIENT) + listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener.expectCallback(iface3, STATE_LINK_UP, ROLE_CLIENT) + listener.assertNoCallback() + } + @Test fun testGetInterfaceList() { // Create two test interfaces and check the return list contains the interface names. diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java index c6ae03985c..def064a03f 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -259,37 +259,4 @@ public class EthernetTrackerTest { verify(mFactory).updateInterface( eq(TEST_IFACE), eq(ipConfig), eq(capabilities)); } - - @Test - public void testIsValidTestInterfaceIsFalseWhenTestInterfacesAreNotIncluded() { - final String validIfaceName = TEST_TAP_PREFIX + "123"; - tracker.setIncludeTestInterfaces(false); - waitForIdle(); - - final boolean isValidTestInterface = tracker.isValidTestInterface(validIfaceName); - - assertFalse(isValidTestInterface); - } - - @Test - public void testIsValidTestInterfaceIsFalseWhenTestInterfaceNameIsInvalid() { - final String invalidIfaceName = "123" + TEST_TAP_PREFIX; - tracker.setIncludeTestInterfaces(true); - waitForIdle(); - - final boolean isValidTestInterface = tracker.isValidTestInterface(invalidIfaceName); - - assertFalse(isValidTestInterface); - } - - @Test - public void testIsValidTestInterfaceIsTrueWhenTestInterfacesIncludedAndValidName() { - final String validIfaceName = TEST_TAP_PREFIX + "123"; - tracker.setIncludeTestInterfaces(true); - waitForIdle(); - - final boolean isValidTestInterface = tracker.isValidTestInterface(validIfaceName); - - assertTrue(isValidTestInterface); - } }