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/OffloadUtilsTest.cpp

diff --git a/server/OffloadUtilsTest.cpp b/server/OffloadUtilsTest.cpp
index 8179563..6566f3b 100644
--- a/server/OffloadUtilsTest.cpp
+++ b/server/OffloadUtilsTest.cpp
@@ -149,12 +149,6 @@
     close(fd);
 }
 
-TEST_F(OffloadUtilsTest, TryOpeningNetlinkSocket) {
-    int fd = openNetlinkSocket();
-    ASSERT_LE(3, fd);
-    close(fd);
-}
-
 // The SKIP_IF_BPF_NOT_SUPPORTED macro is effectively a check for 4.9+ kernel
 // combined with a launched on P device.  Ie. it's a test for 4.9-P or better.
 
@@ -206,16 +200,12 @@
     SKIP_IF_BPF_NOT_SUPPORTED;
     if (!kernelSupportsNetSchIngress()) return;
 
-    int fd = openNetlinkSocket();
-    ASSERT_LE(3, fd);
-
     // This attaches and detaches a configuration-less and thus no-op clsact
     // qdisc to loopback interface (and it takes fractions of a second)
-    EXPECT_EQ(0, tcQdiscAddDevClsact(fd, LOOPBACK_IFINDEX));
-    EXPECT_EQ(0, tcQdiscReplaceDevClsact(fd, LOOPBACK_IFINDEX));
-    EXPECT_EQ(0, tcQdiscDelDevClsact(fd, LOOPBACK_IFINDEX));
-    EXPECT_EQ(-EINVAL, tcQdiscDelDevClsact(fd, LOOPBACK_IFINDEX));
-    close(fd);
+    EXPECT_EQ(0, tcQdiscAddDevClsact(LOOPBACK_IFINDEX));
+    EXPECT_EQ(0, tcQdiscReplaceDevClsact(LOOPBACK_IFINDEX));
+    EXPECT_EQ(0, tcQdiscDelDevClsact(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-EINVAL, tcQdiscDelDevClsact(LOOPBACK_IFINDEX));
 }
 
 static void checkAttachDetachBpfFilterClsactLo(const bool ingress, const bool ethernet) {
@@ -231,32 +221,27 @@
     int bpf_fd = ingress ? getClatIngressProgFd(ethernet) : getClatEgressProgFd(ethernet);
     ASSERT_LE(3, bpf_fd);
 
-    int fd = openNetlinkSocket();
-    EXPECT_LE(3, fd);
-    if (fd >= 0) {
-        // This attaches and detaches a clsact plus ebpf program to loopback
-        // interface, but it should not affect traffic by virtue of us not
-        // actually populating the ebpf control map.
-        // Furthermore: it only takes fractions of a second.
-        EXPECT_EQ(-EINVAL, tcFilterDelDevIngressClatIpv6(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-EINVAL, tcFilterDelDevEgressClatIpv4(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(0, tcQdiscAddDevClsact(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-errNOENT, tcFilterDelDevIngressClatIpv6(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-errNOENT, tcFilterDelDevEgressClatIpv4(fd, LOOPBACK_IFINDEX));
-        if (ingress) {
-            EXPECT_EQ(0, tcFilterAddDevIngressBpf(fd, LOOPBACK_IFINDEX, bpf_fd, ethernet));
-            EXPECT_EQ(0, tcFilterDelDevIngressClatIpv6(fd, LOOPBACK_IFINDEX));
-        } else {
-            EXPECT_EQ(0, tcFilterAddDevEgressBpf(fd, LOOPBACK_IFINDEX, bpf_fd, ethernet));
-            EXPECT_EQ(0, tcFilterDelDevEgressClatIpv4(fd, LOOPBACK_IFINDEX));
-        }
-        EXPECT_EQ(-errNOENT, tcFilterDelDevIngressClatIpv6(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-errNOENT, tcFilterDelDevEgressClatIpv4(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(0, tcQdiscDelDevClsact(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-EINVAL, tcFilterDelDevIngressClatIpv6(fd, LOOPBACK_IFINDEX));
-        EXPECT_EQ(-EINVAL, tcFilterDelDevEgressClatIpv4(fd, LOOPBACK_IFINDEX));
-        close(fd);
+    // This attaches and detaches a clsact plus ebpf program to loopback
+    // interface, but it should not affect traffic by virtue of us not
+    // actually populating the ebpf control map.
+    // Furthermore: it only takes fractions of a second.
+    EXPECT_EQ(-EINVAL, tcFilterDelDevIngressClatIpv6(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-EINVAL, tcFilterDelDevEgressClatIpv4(LOOPBACK_IFINDEX));
+    EXPECT_EQ(0, tcQdiscAddDevClsact(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-errNOENT, tcFilterDelDevIngressClatIpv6(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-errNOENT, tcFilterDelDevEgressClatIpv4(LOOPBACK_IFINDEX));
+    if (ingress) {
+        EXPECT_EQ(0, tcFilterAddDevIngressBpf(LOOPBACK_IFINDEX, bpf_fd, ethernet));
+        EXPECT_EQ(0, tcFilterDelDevIngressClatIpv6(LOOPBACK_IFINDEX));
+    } else {
+        EXPECT_EQ(0, tcFilterAddDevEgressBpf(LOOPBACK_IFINDEX, bpf_fd, ethernet));
+        EXPECT_EQ(0, tcFilterDelDevEgressClatIpv4(LOOPBACK_IFINDEX));
     }
+    EXPECT_EQ(-errNOENT, tcFilterDelDevIngressClatIpv6(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-errNOENT, tcFilterDelDevEgressClatIpv4(LOOPBACK_IFINDEX));
+    EXPECT_EQ(0, tcQdiscDelDevClsact(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-EINVAL, tcFilterDelDevIngressClatIpv6(LOOPBACK_IFINDEX));
+    EXPECT_EQ(-EINVAL, tcFilterDelDevEgressClatIpv4(LOOPBACK_IFINDEX));
 
     close(bpf_fd);
 }