Track local network ifindices in sInterfaceToTable as well.
RouteController tracks in the sInterfaceToTable map the the
interface indices of every interface used in physical and virtual
networks. This ensures that when an interface is removed from a
network (by passing in the interface name), the rules (which
specify interface indices) are correctly deleted even if the
interface has been deleted or has been deleted and re-added with
a new interface index.
Currently this does not happen for interfaces added to the local
network. That means that when those interfaces are deleted, the
rules might not be deleted. It also results in spurious messages
logged by NetworkController such as:
03-24 00:38:47.553 16612 16635 E Netd : getIfIndex: cannot find interface testtap1
03-24 00:38:47.553 16612 16635 E Netd : inconceivable! added interface testtap1 with no index
Note that since P this map is read by RouteController::getIfIndex
and used by code that assumes this will always return an
interface index. In the case of an interface in the local
network, this is not possible because the map stores mappings
between ifindex and routing table. In the case of the local
network, the routing table is always ROUTE_TABLE_LOCAL_NETWORK,
so there is no way to get the interface index. Fix these callers
so they do not attempt do to this.
Bug: 150644681
Test: TetheringIntegrationTests:EthernetTetheringTest
Original-Change: https://android-review.googlesource.com/1277438
Merged-In: I8042e5b91bcb3175d9ad540526df396a139976f0
Change-Id: I8042e5b91bcb3175d9ad540526df396a139976f0
diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp
index 4a5b5b3..20ae44b 100644
--- a/server/NetworkController.cpp
+++ b/server/NetworkController.cpp
@@ -506,12 +506,20 @@
return ret;
}
- int ifIndex = RouteController::getIfIndex(interface);
- if (ifIndex) {
- mIfindexToLastNetId[ifIndex] = netId;
- } else {
- // Cannot happen, since addInterface() above will have failed.
- ALOGE("inconceivable! added interface %s with no index", interface);
+ // Only populate mIfindexToLastNetId for non-local networks, because for these getIfIndex will
+ // return 0. That's fine though, because that map is only used to prevent force-closing sockets
+ // when the same IP address is handed over from one interface to another interface that is in
+ // the same network but not in the same netId (for now this is done only on VPNs). That is not
+ // useful for the local network because IP addresses in the local network are always assigned by
+ // the device itself and never meaningful on any other network.
+ if (netId != LOCAL_NET_ID) {
+ int ifIndex = RouteController::getIfIndex(interface);
+ if (ifIndex) {
+ mIfindexToLastNetId[ifIndex] = netId;
+ } else {
+ // Cannot happen, since addInterface() above will have failed.
+ ALOGE("inconceivable! added interface %s with no index", interface);
+ }
}
return 0;
}
@@ -647,7 +655,7 @@
}
// Then, check for VPN handover condition
if (mIfindexToLastNetId.find(ifindex) == mIfindexToLastNetId.end()) {
- ALOGE("Interface index %u was never in a currently-connected netId", ifindex);
+ ALOGW("Interface index %u was never in a currently-connected non-local netId", ifindex);
return true;
}
unsigned lastNetId = mIfindexToLastNetId[ifindex];
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 52fdcda..134bbca 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -164,7 +164,7 @@
uint32_t index = if_nametoindex(interface);
if (index == 0) {
- ALOGE("cannot find interface %s", interface);
+ ALOGE("cannot find interface %s: %s", interface, strerror(errno));
return RT_TABLE_UNSPEC;
}
index += RouteController::ROUTE_TABLE_OFFSET_FROM_INDEX;
@@ -181,7 +181,17 @@
return 0;
}
- return iter->second - ROUTE_TABLE_OFFSET_FROM_INDEX;
+ // For interfaces that are not in the local network, the routing table is always the interface
+ // index plus ROUTE_TABLE_OFFSET_FROM_INDEX. But for interfaces in the local network, there's no
+ // way to know the interface index from this table. Return 0 here so callers of this method do
+ // not get confused.
+ // TODO: stop calling this method from any caller that only wants interfaces in client mode.
+ int ifindex = iter->second;
+ if (ifindex == ROUTE_TABLE_LOCAL_NETWORK) {
+ return 0;
+ }
+
+ return ifindex - ROUTE_TABLE_OFFSET_FROM_INDEX;
}
uint32_t RouteController::getRouteTableForInterface(const char* interface) {
@@ -1019,11 +1029,21 @@
}
int RouteController::addInterfaceToLocalNetwork(unsigned netId, const char* interface) {
- return modifyLocalNetwork(netId, interface, ACTION_ADD);
+ if (int ret = modifyLocalNetwork(netId, interface, ACTION_ADD)) {
+ return ret;
+ }
+ std::lock_guard lock(sInterfaceToTableLock);
+ sInterfaceToTable[interface] = ROUTE_TABLE_LOCAL_NETWORK;
+ return 0;
}
int RouteController::removeInterfaceFromLocalNetwork(unsigned netId, const char* interface) {
- return modifyLocalNetwork(netId, interface, ACTION_DEL);
+ if (int ret = modifyLocalNetwork(netId, interface, ACTION_DEL)) {
+ return ret;
+ }
+ std::lock_guard lock(sInterfaceToTableLock);
+ sInterfaceToTable.erase(interface);
+ return 0;
}
int RouteController::addInterfaceToPhysicalNetwork(unsigned netId, const char* interface,