From 8d2bbd865a683fc3bc7f83c21117d6c373586512 Mon Sep 17 00:00:00 2001
From: Hungming Chen <nuccachen@google.com>
Date: Fri, 9 Jul 2021 12:42:08 +0800
Subject: [PATCH] [CTT-4] Improve conntrack timeout update logging

Bug: 190783768
Bug: 192804833

Test: atest TetheringCoverageTests

Change-Id: I56792d75d12a0207e0a83a6d64e303f3bbfc3c6a
---
 .../tethering/BpfCoordinator.java             | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index b16416996e..8952da9484 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -1860,7 +1860,15 @@ public class BpfCoordinator {
     // coming a conntrack event to notify updated timeout.
     private void updateConntrackTimeout(byte proto, Inet4Address src4, short srcPort,
             Inet4Address dst4, short dstPort) {
-        if (src4 == null || dst4 == null) return;
+        if (src4 == null || dst4 == null) {
+            mLog.e("Either source or destination IPv4 address is invalid ("
+                    + "proto: " + proto + ", "
+                    + "src4: " + src4 + ", "
+                    + "srcPort: " + Short.toUnsignedInt(srcPort) + ", "
+                    + "dst4: " + dst4 + ", "
+                    + "dstPort: " + Short.toUnsignedInt(dstPort) + ")");
+            return;
+        }
 
         // TODO: consider acquiring the timeout setting from nf_conntrack_* variables.
         // - proc/sys/net/netfilter/nf_conntrack_tcp_timeout_established
@@ -1874,14 +1882,25 @@ public class BpfCoordinator {
         try {
             NetlinkSocket.sendOneShotKernelMessage(OsConstants.NETLINK_NETFILTER, msg);
         } catch (ErrnoException e) {
-            mLog.e("Error updating conntrack entry ("
+            // Lower the log level for the entry not existing. The conntrack entry may have been
+            // deleted and not handled by the conntrack event monitor yet. In other words, the
+            // rule has not been deleted from the BPF map yet. Deleting a non-existent entry may
+            // happen during the conntrack timeout refreshing iteration. Note that ENOENT may be
+            // a real error but is hard to distinguish.
+            // TODO: Figure out a better way to handle this.
+            final String errMsg = "Failed to update conntrack entry ("
                     + "proto: " + proto + ", "
                     + "src4: " + src4 + ", "
                     + "srcPort: " + Short.toUnsignedInt(srcPort) + ", "
                     + "dst4: " + dst4 + ", "
                     + "dstPort: " + Short.toUnsignedInt(dstPort) + "), "
                     + "msg: " + NetlinkConstants.hexify(msg) + ", "
-                    + "e: " + e);
+                    + "e: " + e;
+            if (OsConstants.ENOENT == e.errno) {
+                mLog.w(errMsg);
+            } else {
+                mLog.e(errMsg);
+            }
         }
     }