Snap for 8710048 from d1c5545015fce4cf098349a0640a7b14a53b8ea2 to tm-release Change-Id: I6594e006cd06ffbf239eb8533aab47e9789d604e
diff --git a/common/moduleutils/src/android/net/ip/NetlinkMonitor.java b/common/moduleutils/src/android/net/ip/NetlinkMonitor.java index 4efc480..e58b44e 100644 --- a/common/moduleutils/src/android/net/ip/NetlinkMonitor.java +++ b/common/moduleutils/src/android/net/ip/NetlinkMonitor.java
@@ -18,6 +18,7 @@ import static android.net.util.SocketUtils.makeNetlinkSocketAddress; import static android.system.OsConstants.AF_NETLINK; +import static android.system.OsConstants.ENOBUFS; import static android.system.OsConstants.SOCK_DGRAM; import static android.system.OsConstants.SOCK_NONBLOCK; import static android.system.OsConstants.SOL_SOCKET; @@ -101,7 +102,11 @@ try { fd = Os.socket(AF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, mFamily); if (mSockRcvbufSize != DEFAULT_SOCKET_RECV_BUFSIZE) { - Os.setsockoptInt(fd, SOL_SOCKET, SO_RCVBUF, mSockRcvbufSize); + try { + Os.setsockoptInt(fd, SOL_SOCKET, SO_RCVBUF, mSockRcvbufSize); + } catch (ErrnoException e) { + Log.wtf(mTag, "Failed to set SO_RCVBUF to " + mSockRcvbufSize, e); + } } Os.bind(fd, makeNetlinkSocketAddress(0, mBindGroups)); NetlinkSocket.connectToKernel(fd); @@ -152,6 +157,27 @@ mLog.e(msg, e); } + // Ignoring ENOBUFS may miss any important netlink messages, there are some messages which + // cannot be recovered by dumping current state once missed since kernel doesn't keep state + // for it. In addition, dumping current state will not result in any RTM_DELxxx messages, so + // reconstructing current state from a dump will be difficult. However, for those netlink + // messages don't cause any state changes, e.g. RTM_NEWLINK with current link state, maybe + // it's okay to ignore them, because these netlink messages won't cause any changes on the + // LinkProperties. Given the above trade-offs, try to ignore ENOBUFS and that's similar to + // what netd does today. + // + // TODO: log metrics when ENOBUFS occurs, or even force a disconnect, it will help see how + // often this error occurs on fields with the associated socket receive buffer size. + @Override + protected boolean handleReadError(ErrnoException e) { + logError("readPacket error: ", e); + if (e.errno == ENOBUFS) { + Log.wtf(mTag, "Errno: ENOBUFS"); + return false; + } + return true; + } + // TODO: move NetworkStackUtils to frameworks/libs/net for NetworkStackUtils#closeSocketQuietly. private void closeSocketQuietly(FileDescriptor fd) { try {
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java index 407a7af..bebe7c9 100644 --- a/src/android/net/ip/IpClientLinkObserver.java +++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -45,6 +45,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.netlink.NduseroptMessage; import com.android.net.module.util.netlink.NetlinkConstants; @@ -162,6 +163,13 @@ protected static final String CLAT_PREFIX = "v4-"; private static final boolean DBG = true; + // The default socket receive buffer size in bytes(4MB). If too many netlink messages are + // sent too quickly, those messages can overflow the socket receive buffer. Set a large-enough + // recv buffer size to avoid the ENOBUFS as much as possible. + @VisibleForTesting + static final String CONFIG_SOCKET_RECV_BUFSIZE = "ipclient_netlink_sock_recv_buf_size"; + private static final int SOCKET_RECV_BUFSIZE = 4 * 1024 * 1024; + public IpClientLinkObserver(Context context, Handler h, String iface, Callback callback, Configuration config, SharedLog log, IpClient.Dependencies deps) { mContext = context; @@ -211,6 +219,15 @@ isAtLeastT() /* default value */); } + private int getSocketReceiveBufferSize() { + final int size = mDependencies.getDeviceConfigPropertyInt(CONFIG_SOCKET_RECV_BUFSIZE, + SOCKET_RECV_BUFSIZE /* default value */); + if (size < 0) { + throw new IllegalArgumentException("Invalid SO_RCVBUF " + size); + } + return size; + } + @Override public void onInterfaceAdded(String iface) { if (isNetlinkEventParsingEnabled()) return; @@ -406,11 +423,12 @@ MyNetlinkMonitor(Handler h, SharedLog log, String tag) { super(h, log, tag, OsConstants.NETLINK_ROUTE, !isNetlinkEventParsingEnabled() - ? NetlinkConstants.RTMGRP_ND_USEROPT - : (NetlinkConstants.RTMGRP_ND_USEROPT | NetlinkConstants.RTMGRP_LINK - | NetlinkConstants.RTMGRP_IPV4_IFADDR - | NetlinkConstants.RTMGRP_IPV6_IFADDR - | NetlinkConstants.RTMGRP_IPV6_ROUTE)); + ? NetlinkConstants.RTMGRP_ND_USEROPT + : (NetlinkConstants.RTMGRP_ND_USEROPT | NetlinkConstants.RTMGRP_LINK + | NetlinkConstants.RTMGRP_IPV4_IFADDR + | NetlinkConstants.RTMGRP_IPV6_IFADDR + | NetlinkConstants.RTMGRP_IPV6_ROUTE), + getSocketReceiveBufferSize()); mHandler = h; }
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java index 78d0bca..2f3987c 100644 --- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java +++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -29,6 +29,7 @@ import static android.net.dhcp.DhcpPacket.MIN_V6ONLY_WAIT_MS; import static android.net.dhcp.DhcpResultsParcelableUtil.fromStableParcelable; import static android.net.ip.IpClientLinkObserver.CLAT_PREFIX; +import static android.net.ip.IpClientLinkObserver.CONFIG_SOCKET_RECV_BUFSIZE; import static android.net.ip.IpReachabilityMonitor.MIN_NUD_SOLICIT_NUM; import static android.net.ip.IpReachabilityMonitor.NUD_MCAST_RESOLICIT_NUM; import static android.net.ip.IpReachabilityMonitor.nudEventTypeToInt; @@ -185,6 +186,7 @@ import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestName; @@ -632,6 +634,11 @@ mDependencies.setDeviceConfigProperty(DhcpClient.ARP_PROBE_MAX_MS, 20); mDependencies.setDeviceConfigProperty(DhcpClient.ARP_FIRST_ANNOUNCE_DELAY_MS, 10); mDependencies.setDeviceConfigProperty(DhcpClient.ARP_ANNOUNCE_INTERVAL_MS, 10); + + // Set the initial netlink socket receive buffer size to a minimum of 10KB to ensure test + // cases are still working, meanwhile in order to easily overflow the receive buffer by + // sending as few RAs as possible for test case where it's used to verify ENOBUFS. + mDependencies.setDeviceConfigProperty(CONFIG_SOCKET_RECV_BUFSIZE, 10 * 1024); } private void awaitIpClientShutdown() throws Exception { @@ -3784,4 +3791,47 @@ removeTestInterface(clatIface.getFileDescriptor().getFileDescriptor()); verify(mCb, timeout(TEST_TIMEOUT_MS)).setNeighborDiscoveryOffload(true); } + + @Ignore("TODO: temporarily ignore tests until prebuilts are updated") + @Test @SignatureRequiredTest(reason = "requires mock callback object") + public void testNetlinkSocketReceiveENOBUFS() throws Exception { + if (!mIsNetlinkEventParseEnabled) return; + + ProvisioningConfiguration config = new ProvisioningConfiguration.Builder() + .withoutIPv4() + .build(); + startIpClientProvisioning(config); + doIpv6OnlyProvisioning(); + HandlerUtils.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS); + + // Block IpClient handler. + final CountDownLatch latch = new CountDownLatch(1); + mIpc.getHandler().post(() -> { + try { + latch.await(10_000L /* 10s */, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + // do nothing + } + }); + + // Send large amount of RAs to overflow the netlink socket receive buffer. + for (int i = 0; i < 100; i++) { + sendBasicRouterAdvertisement(false /* waitRs */); + } + + // Unblock the IpClient handler. + latch.countDown(); + HandlerUtils.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS); + + reset(mCb); + + // Send RA with 0 router lifetime to see if IpClient can see the loss of IPv6 default route. + // Due to ignoring the ENOBUFS and wait until handler gets idle, IpClient should be still + // able to see the RA with 0 router lifetime and the IPv6 default route will be removed. + sendRouterAdvertisementWithZeroLifetime(); + final ArgumentCaptor<LinkProperties> captor = ArgumentCaptor.forClass(LinkProperties.class); + verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningFailure(captor.capture()); + final LinkProperties lp = captor.getValue(); + assertFalse(lp.hasIpv6DefaultRoute()); + } }