Pass route add/delete errors back to CommandListener.
Change-Id: Id1d6d578963080e141f71bc1303801fc53bce40a
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index b3ff9ca..d858e60 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -1733,12 +1733,15 @@
const char* destination = argv[nextArg++];
const char* nexthop = argc > nextArg ? argv[nextArg] : NULL;
+ int ret;
if (add) {
- if (!sNetCtrl->addRoute(netId, interface, destination, nexthop, legacy, uid)) {
- return operationError(client, "addRoute() failed");
- }
- } else if (!sNetCtrl->removeRoute(netId, interface, destination, nexthop, legacy, uid)) {
- return operationError(client, "removeRoute() failed");
+ ret = sNetCtrl->addRoute(netId, interface, destination, nexthop, legacy, uid);
+ } else {
+ ret = sNetCtrl->removeRoute(netId, interface, destination, nexthop, legacy, uid);
+ }
+ if (ret != 0) {
+ errno = -ret;
+ return operationError(client, add ? "addRoute() failed" : "removeRoute() failed");
}
return success(client);
diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp
index f5739e9..de433b7 100644
--- a/server/NetworkController.cpp
+++ b/server/NetworkController.cpp
@@ -345,13 +345,13 @@
return status;
}
-bool NetworkController::addRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool legacy, unsigned uid) {
+int NetworkController::addRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool legacy, unsigned uid) {
return modifyRoute(netId, interface, destination, nexthop, true, legacy, uid);
}
-bool NetworkController::removeRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool legacy, unsigned uid) {
+int NetworkController::removeRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool legacy, unsigned uid) {
return modifyRoute(netId, interface, destination, nexthop, false, legacy, uid);
}
@@ -364,18 +364,16 @@
return mValidNetworks.find(netId) != mValidNetworks.end();
}
-bool NetworkController::modifyRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool add, bool legacy, unsigned uid) {
+int NetworkController::modifyRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool add, bool legacy, unsigned uid) {
if (!isValidNetwork(netId)) {
ALOGE("invalid netId %u", netId);
- errno = EINVAL;
- return false;
+ return -EINVAL;
}
if (getNetworkId(interface) != netId) {
ALOGE("netId %u has no such interface %s", netId, interface);
- errno = ENOENT;
- return false;
+ return -ENOENT;
}
RouteController::TableType tableType;
diff --git a/server/NetworkController.h b/server/NetworkController.h
index 30c9142..4e93fa1 100644
--- a/server/NetworkController.h
+++ b/server/NetworkController.h
@@ -67,10 +67,10 @@
// Routes are added to tables determined by the interface, so only |interface| is actually used.
// |netId| is given only to sanity check that the interface has the correct netId.
- bool addRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool legacy, unsigned uid);
- bool removeRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool legacy, unsigned uid);
+ int addRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool legacy, unsigned uid);
+ int removeRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool legacy, unsigned uid);
bool isValidNetwork(unsigned netId) const;
@@ -79,8 +79,8 @@
typedef std::multimap<unsigned, std::string>::iterator InterfaceIterator;
typedef std::pair<InterfaceIterator, InterfaceIterator> InterfaceRange;
- bool modifyRoute(unsigned netId, const char* interface, const char* destination,
- const char* nexthop, bool add, bool legacy, unsigned uid);
+ int modifyRoute(unsigned netId, const char* interface, const char* destination,
+ const char* nexthop, bool add, bool legacy, unsigned uid);
struct UidEntry {
int uid_start;
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 3c16fa3..9716b9e 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -309,7 +309,10 @@
mask.intValue, NULL);
}
-bool modifyRoute(const char* interface, const char* destination, const char* nexthop,
+// Adds or removes an IPv4 or IPv6 route to the specified table and, if it's directly-connected
+// route, to the main table as well.
+// Returns 0 on success or negative errno on failure.
+int modifyRoute(const char* interface, const char* destination, const char* nexthop,
int action, RouteController::TableType tableType, unsigned /* uid */) {
uint32_t table = 0;
switch (tableType) {
@@ -329,26 +332,27 @@
}
}
if (!table) {
- return false;
+ return -ESRCH;
}
- if (modifyIpRoute(action, table, interface, destination, nexthop)) {
- return false;
+ int ret = modifyIpRoute(action, table, interface, destination, nexthop);
+ if (ret != 0) {
+ return ret;
}
// If there's no nexthop, this is a directly connected route. Add it to the main table also, to
- // let the kernel find it when validating nexthops when global routes are added. Don't do this
- // for IPv6, since all directly-connected routes in v6 are link-local and should already be in
- // the main table.
- // TODO: A failure here typically means that the route already exists in the main table, so we
- // ignore it. It's wrong to ignore other kinds of failures, but we have no way to distinguish
- // them based on the return status of the 'ip' command. Fix this situation by ignoring errors
- // only when action == ADD && error == EEXIST.
- if (!nexthop && !strchr(destination, ':')) {
- modifyIpRoute(action, RT_TABLE_MAIN, interface, destination, NULL);
+ // let the kernel find it when validating nexthops when global routes are added.
+ if (!nexthop) {
+ ret = modifyIpRoute(action, RT_TABLE_MAIN, interface, destination, NULL);
+ // A failure with action == ADD && errno == EEXIST means that the route already exists in
+ // the main table, perhaps because the kernel added it automatically as part of adding the
+ // IP address to the interface. Ignore this, but complain about everything else.
+ if (ret != 0 && !(action == RTM_NEWROUTE && ret == -EEXIST)) {
+ return ret;
+ }
}
- return true;
+ return 0;
}
bool flushRoutes(const char* interface) {
@@ -450,12 +454,12 @@
return modifyDefaultNetworkRules(interface, permission, DEL);
}
-bool RouteController::addRoute(const char* interface, const char* destination,
- const char* nexthop, TableType tableType, unsigned uid) {
+int RouteController::addRoute(const char* interface, const char* destination,
+ const char* nexthop, TableType tableType, unsigned uid) {
return modifyRoute(interface, destination, nexthop, RTM_NEWROUTE, tableType, uid);
}
-bool RouteController::removeRoute(const char* interface, const char* destination,
- const char* nexthop, TableType tableType, unsigned uid) {
+int RouteController::removeRoute(const char* interface, const char* destination,
+ const char* nexthop, TableType tableType, unsigned uid) {
return modifyRoute(interface, destination, nexthop, RTM_DELROUTE, tableType, uid);
}
diff --git a/server/RouteController.h b/server/RouteController.h
index 7292579..4695a6c 100644
--- a/server/RouteController.h
+++ b/server/RouteController.h
@@ -41,10 +41,10 @@
static bool addToDefaultNetwork(const char* interface, Permission permission);
static bool removeFromDefaultNetwork(const char* interface, Permission permission);
- static bool addRoute(const char* interface, const char* destination, const char* nexthop,
- TableType tableType, unsigned uid);
- static bool removeRoute(const char* interface, const char* destination, const char* nexthop,
- TableType tableType, unsigned uid);
+ static int addRoute(const char* interface, const char* destination, const char* nexthop,
+ TableType tableType, unsigned uid);
+ static int removeRoute(const char* interface, const char* destination, const char* nexthop,
+ TableType tableType, unsigned uid);
};
#endif // NETD_SERVER_ROUTE_CONTROLLER_H