Merge "ethernet: replace netd link observer with NetlinkMonitor"

service-t/src/com/android/server/ethernet/EthernetTracker.java

@@ -43,15 +43,21 @@ import android.os.Handler;
 import android.os.RemoteCallbackList;
 import android.os.RemoteException;
 import android.os.ServiceSpecificException;
+import android.system.OsConstants;
 import android.text.TextUtils;
 import android.util.ArrayMap;
 import android.util.Log;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.IndentingPrintWriter;
-import com.android.net.module.util.BaseNetdUnsolicitedEventListener;
 import com.android.net.module.util.NetdUtils;
 import com.android.net.module.util.PermissionUtils;
+import com.android.net.module.util.SharedLog;
+import com.android.net.module.util.ip.NetlinkMonitor;
+import com.android.net.module.util.netlink.NetlinkConstants;
+import com.android.net.module.util.netlink.NetlinkMessage;
+import com.android.net.module.util.netlink.RtNetlinkLinkMessage;
+import com.android.net.module.util.netlink.StructIfinfoMsg;
 
 import java.io.FileDescriptor;
 import java.net.InetAddress;
@@ -86,6 +92,9 @@ public class EthernetTracker {
 
     private static final String TEST_IFACE_REGEXP = TEST_TAP_PREFIX + "\\d+";
 
+    // TODO: consider using SharedLog consistently across ethernet service.
+    private static final SharedLog sLog = new SharedLog(TAG);
+
     /**
      * Interface names we track. This is a product-dependent regular expression.
      * Use isValidEthernetInterface to check if a interface name is a valid ethernet interface (this
@@ -110,6 +119,7 @@ public class EthernetTracker {
     private final Handler mHandler;
     private final EthernetNetworkFactory mFactory;
     private final EthernetConfigStore mConfigStore;
+    private final NetlinkMonitor mNetlinkMonitor;
     private final Dependencies mDeps;
 
     private final RemoteCallbackList<IEthernetServiceListener> mListeners =
@@ -148,6 +158,69 @@ public class EthernetTracker {
         }
     }
 
+    private class EthernetNetlinkMonitor extends NetlinkMonitor {
+        EthernetNetlinkMonitor(Handler handler) {
+            super(handler, sLog, EthernetNetlinkMonitor.class.getSimpleName(),
+                    OsConstants.NETLINK_ROUTE, NetlinkConstants.RTMGRP_LINK);
+        }
+
+        private void onNewLink(String ifname, boolean linkUp) {
+            if (!mFactory.hasInterface(ifname) && !ifname.equals(mDefaultInterface)) {
+                Log.i(TAG, "onInterfaceAdded, iface: " + ifname);
+                maybeTrackInterface(ifname);
+            }
+            Log.i(TAG, "interfaceLinkStateChanged, iface: " + ifname + ", up: " + linkUp);
+            updateInterfaceState(ifname, linkUp);
+        }
+
+        private void onDelLink(String ifname) {
+            Log.i(TAG, "onInterfaceRemoved, iface: " + ifname);
+            stopTrackingInterface(ifname);
+        }
+
+        private void processRtNetlinkLinkMessage(RtNetlinkLinkMessage msg) {
+            final StructIfinfoMsg ifinfomsg = msg.getIfinfoHeader();
+            // check if the message is valid
+            if (ifinfomsg.family != OsConstants.AF_UNSPEC) return;
+
+            // ignore messages for the loopback interface
+            if ((ifinfomsg.flags & OsConstants.IFF_LOOPBACK) != 0) return;
+
+            // check if the received message applies to an ethernet interface.
+            final String ifname = msg.getInterfaceName();
+            if (!isValidEthernetInterface(ifname)) return;
+
+            switch (msg.getHeader().nlmsg_type) {
+                case NetlinkConstants.RTM_NEWLINK:
+                    final boolean linkUp = (ifinfomsg.flags & NetlinkConstants.IFF_LOWER_UP) != 0;
+                    onNewLink(ifname, linkUp);
+                    break;
+
+                case NetlinkConstants.RTM_DELLINK:
+                    onDelLink(ifname);
+                    break;
+
+                default:
+                    Log.e(TAG, "Unknown rtnetlink link msg type: " + msg);
+                    break;
+            }
+        }
+
+        // Note: processNetlinkMessage is called on the handler thread.
+        @Override
+        protected void processNetlinkMessage(NetlinkMessage nlMsg, long whenMs) {
+            // ignore all updates when ethernet is disabled.
+            if (mEthernetState == ETHERNET_STATE_DISABLED) return;
+
+            if (nlMsg instanceof RtNetlinkLinkMessage) {
+                processRtNetlinkLinkMessage((RtNetlinkLinkMessage) nlMsg);
+            } else {
+                Log.e(TAG, "Unknown netlink message: " + nlMsg);
+            }
+        }
+    }
+
+
     EthernetTracker(@NonNull final Context context, @NonNull final Handler handler,
             @NonNull final EthernetNetworkFactory factory, @NonNull final INetd netd) {
         this(context, handler, factory, netd, new Dependencies());
@@ -173,6 +246,7 @@ public class EthernetTracker {
         }
 
         mConfigStore = new EthernetConfigStore();
+        mNetlinkMonitor = new EthernetNetlinkMonitor(mHandler);
     }
 
     void start() {
@@ -186,14 +260,10 @@ public class EthernetTracker {
             mIpConfigurations.put(configs.keyAt(i), configs.valueAt(i));
         }
 
-        try {
+        mHandler.post(() -> {
-            PermissionUtils.enforceNetworkStackPermission(mContext);
+            mNetlinkMonitor.start();
-            mNetd.registerUnsolicitedEventListener(new InterfaceObserver());
+            trackAvailableInterfaces();
-        } catch (RemoteException | ServiceSpecificException e) {
+        });
-            Log.e(TAG, "Could not register InterfaceObserver " + e);
-        }
-
-        mHandler.post(this::trackAvailableInterfaces);
     }
 
     void updateIpConfiguration(String iface, IpConfiguration ipConfiguration) {
@@ -617,43 +687,6 @@ public class EthernetTracker {
         }
     }
 
-    @VisibleForTesting
-    class InterfaceObserver extends BaseNetdUnsolicitedEventListener {
-
-        @Override
-        public void onInterfaceLinkStateChanged(String iface, boolean up) {
-            if (DBG) {
-                Log.i(TAG, "interfaceLinkStateChanged, iface: " + iface + ", up: " + up);
-            }
-            mHandler.post(() -> {
-                if (mEthernetState == ETHERNET_STATE_DISABLED) return;
-                updateInterfaceState(iface, up);
-            });
-        }
-
-        @Override
-        public void onInterfaceAdded(String iface) {
-            if (DBG) {
-                Log.i(TAG, "onInterfaceAdded, iface: " + iface);
-            }
-            mHandler.post(() -> {
-                if (mEthernetState == ETHERNET_STATE_DISABLED) return;
-                maybeTrackInterface(iface);
-            });
-        }
-
-        @Override
-        public void onInterfaceRemoved(String iface) {
-            if (DBG) {
-                Log.i(TAG, "onInterfaceRemoved, iface: " + iface);
-            }
-            mHandler.post(() -> {
-                if (mEthernetState == ETHERNET_STATE_DISABLED) return;
-                stopTrackingInterface(iface);
-            });
-        }
-    }
-
     private static class ListenerInfo {
 
         boolean canUseRestrictedNetworks = false;

service/Android.bp

@@ -162,6 +162,7 @@ java_library {
         "modules-utils-shell-command-handler",
         "net-utils-device-common",
         "net-utils-device-common-bpf",
+        "net-utils-device-common-ip",
         "net-utils-device-common-netlink",
         "net-utils-services-common",
         "netd-client",

tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java

@@ -30,10 +30,8 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -64,7 +62,6 @@ import org.junit.After;
 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;
 
@@ -476,43 +473,4 @@ public class EthernetTrackerTest {
         verify(listener).onInterfaceStateChanged(eq(testIface), eq(EthernetManager.STATE_LINK_UP),
                 anyInt(), any());
     }
-
-    @Test
-    public void testListenEthernetStateChange_unsolicitedEventListener() throws Exception {
-        when(mNetd.interfaceGetList()).thenReturn(new String[] {});
-        doReturn(new String[] {}).when(mFactory).getAvailableInterfaces(anyBoolean());
-
-        tracker.setIncludeTestInterfaces(true);
-        tracker.start();
-
-        final ArgumentCaptor<EthernetTracker.InterfaceObserver> captor =
-                ArgumentCaptor.forClass(EthernetTracker.InterfaceObserver.class);
-        verify(mNetd, timeout(TIMEOUT_MS)).registerUnsolicitedEventListener(captor.capture());
-        final EthernetTracker.InterfaceObserver observer = captor.getValue();
-
-        tracker.setEthernetEnabled(false);
-        waitForIdle();
-        reset(mFactory);
-        reset(mNetd);
-
-        final String testIface = "testtap1";
-        observer.onInterfaceAdded(testIface);
-        verify(mFactory, never()).addInterface(eq(testIface), anyString(), any(), any());
-        observer.onInterfaceRemoved(testIface);
-        verify(mFactory, never()).removeInterface(eq(testIface));
-
-        final String testHwAddr = "11:22:33:44:55:66";
-        final InterfaceConfigurationParcel testIfaceParce =
-                createMockedIfaceParcel(testIface, testHwAddr);
-        when(mNetd.interfaceGetList()).thenReturn(new String[] {testIface});
-        when(mNetd.interfaceGetCfg(eq(testIface))).thenReturn(testIfaceParce);
-        doReturn(new String[] {testIface}).when(mFactory).getAvailableInterfaces(anyBoolean());
-        tracker.setEthernetEnabled(true);
-        waitForIdle();
-        reset(mFactory);
-
-        final String testIface2 = "testtap2";
-        observer.onInterfaceRemoved(testIface2);
-        verify(mFactory, timeout(TIMEOUT_MS)).removeInterface(eq(testIface2));
-    }
 }