Get rtnetlink address information from netlink event instead of Netd.
This change also refactors IpClientIntegrationTest to apply the new
update of getting netlink address information from netlink events
instead of depending on the netd callback such as:
- move mIsNetlinkEventParseEnabled parameterized value setup before
making IpClient instance;
- update isStablePrivacyAddress to get the correct address flags;
- call waitForIdle to see if IpClient has seen the new addresses.
Bug: 163492391
Test: atest NetworkStackTests NetworkStackIntegrationTests
Change-Id: I6b37962a6e121a29b19c9f33f66d2380c2bccb97
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java
index a94a2d6..3849c31 100644
--- a/src/android/net/ip/IpClientLinkObserver.java
+++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -40,7 +40,9 @@
import com.android.net.module.util.netlink.NduseroptMessage;
import com.android.net.module.util.netlink.NetlinkConstants;
import com.android.net.module.util.netlink.NetlinkMessage;
+import com.android.net.module.util.netlink.RtNetlinkAddressMessage;
import com.android.net.module.util.netlink.RtNetlinkLinkMessage;
+import com.android.net.module.util.netlink.StructIfaddrMsg;
import com.android.net.module.util.netlink.StructIfinfoMsg;
import com.android.net.module.util.netlink.StructNdOptPref64;
import com.android.net.module.util.netlink.StructNdOptRdnss;
@@ -158,6 +160,10 @@
}
}
+ private void maybeLog(String operation, int ifindex, LinkAddress address) {
+ maybeLog(operation, "ifindex " + ifindex, address);
+ }
+
private void maybeLog(String operation, Object o) {
if (DBG) {
Log.d(mTag, operation + ": " + o.toString());
@@ -196,34 +202,18 @@
@Override
public void onInterfaceAddressUpdated(LinkAddress address, String iface) {
- if (mInterfaceName.equals(iface)) {
- maybeLog("addressUpdated", iface, address);
- final boolean changed;
- final boolean linkState;
- synchronized (this) {
- changed = mLinkProperties.addLinkAddress(address);
- linkState = getInterfaceLinkStateLocked();
- }
- if (changed) {
- mCallback.update(linkState);
- }
- }
+ if (isNetlinkEventParsingEnabled()) return;
+ if (!mInterfaceName.equals(iface)) return;
+ maybeLog("addressUpdated", iface, address);
+ updateInterfaceAddress(address, true /* add address */);
}
@Override
public void onInterfaceAddressRemoved(LinkAddress address, String iface) {
- if (mInterfaceName.equals(iface)) {
- maybeLog("addressRemoved", iface, address);
- final boolean changed;
- final boolean linkState;
- synchronized (this) {
- changed = mLinkProperties.removeLinkAddress(address);
- linkState = getInterfaceLinkStateLocked();
- }
- if (changed) {
- mCallback.update(linkState);
- }
- }
+ if (isNetlinkEventParsingEnabled()) return;
+ if (!mInterfaceName.equals(iface)) return;
+ maybeLog("addressRemoved", iface, address);
+ updateInterfaceAddress(address, false /* remove address */);
}
@Override
@@ -282,6 +272,22 @@
}
}
+ private void updateInterfaceAddress(final LinkAddress address, boolean add) {
+ final boolean changed;
+ final boolean linkState;
+ synchronized (this) {
+ if (add) {
+ changed = mLinkProperties.addLinkAddress(address);
+ } else {
+ changed = mLinkProperties.removeLinkAddress(address);
+ }
+ linkState = getInterfaceLinkStateLocked();
+ }
+ if (changed) {
+ mCallback.update(linkState);
+ }
+ }
+
/**
* Returns a copy of this object's LinkProperties.
*/
@@ -331,7 +337,9 @@
super(h, log, tag, OsConstants.NETLINK_ROUTE,
!isNetlinkEventParsingEnabled()
? NetlinkConstants.RTMGRP_ND_USEROPT
- : (NetlinkConstants.RTMGRP_ND_USEROPT | NetlinkConstants.RTMGRP_LINK));
+ : (NetlinkConstants.RTMGRP_ND_USEROPT | NetlinkConstants.RTMGRP_LINK
+ | NetlinkConstants.RTMGRP_IPV4_IFADDR
+ | NetlinkConstants.RTMGRP_IPV6_IFADDR));
mHandler = h;
}
@@ -485,12 +493,37 @@
}
}
+ private void processRtNetlinkAddressMessage(RtNetlinkAddressMessage msg) {
+ if (!isNetlinkEventParsingEnabled()) return;
+
+ final StructIfaddrMsg ifaddrMsg = msg.getIfaddrHeader();
+ if (ifaddrMsg.index != mIfindex) return;
+ final LinkAddress la = new LinkAddress(msg.getIpAddress(), ifaddrMsg.prefixLen,
+ msg.getFlags(), ifaddrMsg.scope);
+
+ switch (msg.getHeader().nlmsg_type) {
+ case NetlinkConstants.RTM_NEWADDR:
+ maybeLog("addressUpdated", mIfindex, la);
+ updateInterfaceAddress(la, true /* add address */);
+ break;
+ case NetlinkConstants.RTM_DELADDR:
+ maybeLog("addressRemoved", mIfindex, la);
+ updateInterfaceAddress(la, false /* remove address */);
+ break;
+ default:
+ Log.e(mTag, "Unknown rtnetlink address msg type " + msg.getHeader().nlmsg_type);
+ return;
+ }
+ }
+
@Override
protected void processNetlinkMessage(NetlinkMessage nlMsg, long whenMs) {
if (nlMsg instanceof NduseroptMessage) {
processNduseroptMessage((NduseroptMessage) nlMsg, whenMs);
} else if (nlMsg instanceof RtNetlinkLinkMessage) {
processRtNetlinkLinkMessage((RtNetlinkLinkMessage) nlMsg);
+ } else if (nlMsg instanceof RtNetlinkAddressMessage) {
+ processRtNetlinkAddressMessage((RtNetlinkAddressMessage) nlMsg);
} else {
Log.e(mTag, "Unknown netlink message: " + nlMsg);
}
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
index e9a6e9d..47abaf7 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -255,7 +255,7 @@
@Parameterized.Parameters
public static Iterable<? extends Object> data() {
- return Arrays.asList(Boolean.valueOf("false"), Boolean.valueOf("true"));
+ return Arrays.asList(Boolean.FALSE, Boolean.TRUE);
}
/**
@@ -575,6 +575,17 @@
mIsSignatureRequiredTest = testMethod.getAnnotation(SignatureRequiredTest.class) != null;
assumeFalse(testSkipped());
+ // Depend on the parameterized value to enable/disable netlink message refactor flag.
+ // Make sure both of the old codepath(rely on the INetdUnsolicitedEventListener aidl)
+ // and new codepath(parse netlink event from kernel) will be executed.
+ //
+ // Note this must be called before making IpClient instance since MyNetlinkMontior ctor
+ // in IpClientLinkObserver will use mIsNetlinkEventParseEnabled to decide the proper
+ // bindGroups, otherwise, the parameterized value got from ArrayMap(integration test) is
+ // always false.
+ setFeatureEnabled(NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION,
+ mIsNetlinkEventParseEnabled /* default value */);
+
setUpTapInterface();
mCb = mock(IIpClientCallbacks.class);
@@ -584,12 +595,6 @@
}
mIIpClient = makeIIpClient(mIfaceName, mCb);
-
- // Depend on the parameterized value to enable/disable netlink message refactor flag.
- // Make sure both of the old codepath(rely on the INetdUnsolicitedEventListener aidl)
- // and new codepath(parse netlink event from kernel) will be executed.
- setFeatureEnabled(NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION,
- mIsNetlinkEventParseEnabled /* default value */);
}
protected void setUpMocks() throws Exception {
@@ -1676,8 +1681,10 @@
}
private boolean isStablePrivacyAddress(LinkAddress addr) {
- // TODO: move away from getting address updates from netd and make this work on Q as well.
- final int flag = ShimUtils.isAtLeastR() ? IFA_F_STABLE_PRIVACY : 0;
+ // The Q netd does not understand the IFA_F_STABLE_PRIVACY flag.
+ // See r.android.com/1295670.
+ final int flag = (mIsNetlinkEventParseEnabled || ShimUtils.isAtLeastR())
+ ? IFA_F_STABLE_PRIVACY : 0;
return addr.isGlobalPreferred() && hasFlag(addr, flag);
}
@@ -1903,13 +1910,10 @@
HandlerUtils.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS);
}
- private void addIpAddressAndWaitForIt(final String iface) throws Exception {
+ private void waitForAddressViaNetworkObserver(final String iface, final String addr1,
+ final String addr2, int prefixLength) throws Exception {
final CountDownLatch latch = new CountDownLatch(1);
- final String addr1 = "192.0.2.99";
- final String addr2 = "192.0.2.3";
- final int prefixLength = 26;
-
// Add two IPv4 addresses to the specified interface, and proceed when the NetworkObserver
// has seen the second one. This ensures that every other NetworkObserver registered with
// mNetworkObserverRegistry - in particular, IpClient's - has seen the addition of the first
@@ -1933,6 +1937,22 @@
} finally {
mNetworkObserverRegistry.unregisterObserver(observer);
}
+ }
+
+ private void addIpAddressAndWaitForIt(final String iface) throws Exception {
+ final String addr1 = "192.0.2.99";
+ final String addr2 = "192.0.2.3";
+ final int prefixLength = 26;
+
+ if (!mIsNetlinkEventParseEnabled) {
+ waitForAddressViaNetworkObserver(iface, addr1, addr2, prefixLength);
+ } else {
+ // IpClient gets IP addresses directly from netlink instead of from netd, unnecessary
+ // to rely on the NetworkObserver callbacks to confirm new added address update. Just
+ // add the addresses directly and wait to see if IpClient has seen the address
+ mNetd.interfaceAddAddress(iface, addr1, prefixLength);
+ mNetd.interfaceAddAddress(iface, addr2, prefixLength);
+ }
// Wait for IpClient to process the addition of the address.
HandlerUtils.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS);
@@ -1959,9 +1979,6 @@
// The address must be noticed before startProvisioning is called, or IpClient will
// immediately declare provisioning success due to the presence of an IPv4 address.
// The address must be IPv4 because IpClient clears IPv6 addresses on startup.
- //
- // TODO: once IpClient gets IP addresses directly from netlink instead of from netd, it
- // may be sufficient to call waitForIdle to see if IpClient has seen the address.
addIpAddressAndWaitForIt(mIfaceName);
}