Simplify and improve error logging in sendNetlinkRequest.
Bug: 32323979
Test: bullhead builds, boots, new error messages appear
Test: unit tests continue to pass
Change-Id: Ie60ed3a71fbd26b7a8a1d2f7fb8083b1b6b9626a
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 48c609d..a50dab3 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -133,6 +133,19 @@
// END CONSTANTS ----------------------------------------------------------------------------------
+const char *actionName(uint16_t action) {
+ static const char *ops[4] = {"adding", "deleting", "getting", "???"};
+ return ops[action % 4];
+}
+
+const char *familyName(uint8_t family) {
+ switch (family) {
+ case AF_INET: return "IPv4";
+ case AF_INET6: return "IPv6";
+ default: return "???";
+ }
+}
+
// No locks needed because RouteController is accessed only from one thread (in CommandListener).
std::map<std::string, uint32_t> interfaceToTable;
@@ -183,7 +196,7 @@
}
}
-// Sends a netlink request and expects an ack.
+// Sends a netlink request and possibly expects an ack.
// |iov| is an array of struct iovec that contains the netlink message payload.
// The netlink header is generated by this function based on |action| and |flags|.
// Returns -errno if there was an error or if the kernel reported an error.
@@ -207,30 +220,37 @@
nlmsg.nlmsg_len += iov[i].iov_len;
}
- int ret;
+ int sock = socket(AF_NETLINK, SOCK_DGRAM | SOCK_CLOEXEC, NETLINK_ROUTE);
+ if (sock == -1) {
+ return -errno;
+ }
+
+ int ret = 0;
struct {
nlmsghdr msg;
nlmsgerr err;
} response;
- int sock = socket(AF_NETLINK, SOCK_DGRAM | SOCK_CLOEXEC, NETLINK_ROUTE);
- if (sock != -1 &&
- connect(sock, reinterpret_cast<const sockaddr*>(&NETLINK_ADDRESS),
- sizeof(NETLINK_ADDRESS)) != -1 &&
- writev(sock, iov, iovlen) != -1 &&
- (ret = recv(sock, &response, sizeof(response), 0)) != -1) {
+
+ if (connect(sock, reinterpret_cast<const sockaddr*>(&NETLINK_ADDRESS),
+ sizeof(NETLINK_ADDRESS)) == -1 || writev(sock, iov, iovlen) == -1) {
+ ret = -errno;
+ ALOGE("netlink socket connect/writev failed (%s)", strerror(-ret));
+ close(sock);
+ return ret;
+ }
+
+ if (flags & NLM_F_ACK) {
+ ret = recv(sock, &response, sizeof(response), 0);
if (ret == sizeof(response)) {
ret = response.err.error; // Netlink errors are negative errno.
- if (ret) {
- ALOGE("netlink response contains error (%s)", strerror(-ret));
- }
+ } else if (ret == -1) {
+ ret = -errno;
+ ALOGE("netlink recv failed (%s)", strerror(-ret));
} else {
ALOGE("bad netlink response message size (%d != %zu)", ret, sizeof(response));
ret = -EBADMSG;
}
- } else {
- ALOGE("netlink socket/connect/writev/recv failed (%s)", strerror(errno));
- ret = -errno;
}
if (sock != -1) {
@@ -353,6 +373,8 @@
for (size_t i = 0; i < ARRAY_SIZE(AF_FAMILIES); ++i) {
rule.family = AF_FAMILIES[i];
if (int ret = sendNetlinkRequest(action, flags, iov, ARRAY_SIZE(iov))) {
+ ALOGE("Error %s %s rule: %s", actionName(action), familyName(rule.family),
+ strerror(-ret));
return ret;
}
}
@@ -458,7 +480,12 @@
uint16_t flags = (action == RTM_NEWROUTE) ? NETLINK_CREATE_REQUEST_FLAGS :
NETLINK_REQUEST_FLAGS;
- return sendNetlinkRequest(action, flags, iov, ARRAY_SIZE(iov));
+ int ret = sendNetlinkRequest(action, flags, iov, ARRAY_SIZE(iov));
+ if (ret) {
+ ALOGE("Error %s route %s -> %s %s to table %u: %s",
+ actionName(action), destination, nexthop, interface, table, strerror(-ret));
+ }
+ return ret;
}
// An iptables rule to mark incoming packets on a network with the netId of the network.
@@ -720,12 +747,10 @@
}
if ((ret = modifyIpRoute(RTM_NEWROUTE, table, interface, "0.0.0.0/0", NULL))) {
- ALOGE("Can't add IPv4 default route to %s: %s", interface, strerror(-ret));
return ret;
}
if ((ret = modifyIpRoute(RTM_NEWROUTE, table, interface, "::/0", NULL))) {
- ALOGE("Can't add IPv6 default route to %s: %s", interface, strerror(-ret));
return ret;
}