commit f89e9113caa22ebcd881573574cbdea1c353c9e6
author Maciej Żenczykowski <maze@google.com> Wed Feb 12 05:22:56 2020 -0800
committer Maciej Żenczykowski <maze@google.com> Sat Feb 15 02:47:14 2020 +0000
tree 87832b571be903564bb26aa94019d73fea77c51a
parent 66ce72dad011b1c6ee6e1adc5c19dbcbb659ee71

OffloadUtils - simplify netlink socket lifetime

None of this stuff is performance critical enough that we can't afford
to simply create a new clean netlink socket, and this significantly
simplifies things and makes it all much more thread-safe.

Test: builds, atest
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: If1000d3cead66631d524cb48d581c0f6e53f6d50

server/OffloadUtils.cpp

diff --git a/server/OffloadUtils.cpp b/server/OffloadUtils.cpp
index c3b1d9e..4281b20 100644
--- a/server/OffloadUtils.cpp
+++ b/server/OffloadUtils.cpp
@@ -59,7 +59,8 @@
 }
 
 // TODO: use //system/netd/server/NetlinkCommands.cpp:openNetlinkSocket(protocol)
-int openNetlinkSocket(void) {
+// and //system/netd/server/SockDiag.cpp:checkError(fd)
+static int sendAndProcessNetlinkResponse(const void* req, int len) {
     base::unique_fd fd(socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE));
     if (fd == -1) {
         const int err = errno;
@@ -67,10 +68,8 @@
         return -err;
     }
 
-    int rv;
-
     const int on = 1;
-    rv = setsockopt(fd, SOL_NETLINK, NETLINK_CAP_ACK, &on, sizeof(on));
+    int rv = setsockopt(fd, SOL_NETLINK, NETLINK_CAP_ACK, &on, sizeof(on));
     if (rv) ALOGE("setsockopt(fd, SOL_NETLINK, NETLINK_CAP_ACK, %d)", on);
 
     // this is needed to get sane strace netlink parsing, it allocates the pid
@@ -89,12 +88,7 @@
         return -err;
     }
 
-    return fd.release();
-}
-
-// TODO: merge with //system/netd/server/SockDiag.cpp:checkError(fd)
-static int sendAndProcessNetlinkResponse(int fd, const void* req, int len) {
-    int rv = send(fd, req, len, 0);
+    rv = send(fd, req, len, 0);
     if (rv == -1) return -errno;
     if (rv != len) return -EMSGSIZE;
 
@@ -133,7 +127,7 @@
 // ADD:     nlMsgType=RTM_NEWQDISC nlMsgFlags=NLM_F_EXCL|NLM_F_CREATE
 // REPLACE: nlMsgType=RTM_NEWQDISC nlMsgFlags=NLM_F_CREATE|NLM_F_REPLACE
 // DEL:     nlMsgType=RTM_DELQDISC nlMsgFlags=0
-int doTcQdiscClsact(int fd, int ifIndex, uint16_t nlMsgType, uint16_t nlMsgFlags) {
+int doTcQdiscClsact(int ifIndex, uint16_t nlMsgType, uint16_t nlMsgFlags) {
     // This is the name of the qdisc we are attaching.
     // Some hoop jumping to make this compile time constant with known size,
     // so that the structure declaration is well defined at compile time.
@@ -176,12 +170,12 @@
 #undef ASCIIZ_LEN_CLSACT
 #undef CLSACT
 
-    return sendAndProcessNetlinkResponse(fd, &req, sizeof(req));
+    return sendAndProcessNetlinkResponse(&req, sizeof(req));
 }
 
 // tc filter add dev .. in/egress prio 1 protocol ipv6/ip bpf object-pinned /sys/fs/bpf/...
 // direct-action
-int tcFilterAddDevBpf(int fd, int ifIndex, int bpfFd, bool ethernet, bool ingress, bool ipv6) {
+int tcFilterAddDevBpf(int ifIndex, int bpfFd, bool ethernet, bool ingress, bool ipv6) {
     // The priority doesn't matter until we actually start attaching multiple
     // things to the same interface's in/egress point.
     const __u32 prio = 1;
@@ -351,11 +345,11 @@
 #undef ASCIIZ_LEN_BPF
 #undef BPF
 
-    return sendAndProcessNetlinkResponse(fd, &req, sizeof(req));
+    return sendAndProcessNetlinkResponse(&req, sizeof(req));
 }
 
 // tc filter del dev .. in/egress prio .. protocol ..
-int tcFilterDelDev(int fd, int ifIndex, bool ingress, uint16_t prio, uint16_t proto) {
+int tcFilterDelDev(int ifIndex, bool ingress, uint16_t prio, uint16_t proto) {
     struct {
         nlmsghdr n;
         tcmsg t;
@@ -377,7 +371,7 @@
                     },
     };
 
-    return sendAndProcessNetlinkResponse(fd, &req, sizeof(req));
+    return sendAndProcessNetlinkResponse(&req, sizeof(req));
 }
 
 }  // namespace net