commit 9ab28d0832cb3d0b5703da02f24645fecae96845
author Maciej Żenczykowski <maze@google.com> Wed Aug 17 14:45:31 2022 +0000
committer Maciej Żenczykowski <maze@google.com> Thu Aug 18 01:07:20 2022 +0000
tree 1bd15151bb04d7fd5bde2a899d8ca175c2a54e87
parent 09813ced3efc88d1b5f1019b00f9745aefe9b955

use getIfName in more spots to improve logging

Also just use 'int' for ifindex.  The kernel allocates these in order,
if we ever exceed 2 billion, we'll have other problems to worry about.

Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I33eb46185fde90cd4d158bb74f8f9c17b41434fb

Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java

diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index 5052eb5..05a2884 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -575,7 +575,7 @@
             if (!mBpfCoordinatorShim.startUpstreamIpv6Forwarding(downstream, upstream, rule.srcMac,
                     NULL_MAC_ADDRESS, NULL_MAC_ADDRESS, NetworkStackConstants.ETHER_MTU)) {
                 mLog.e("Failed to enable upstream IPv6 forwarding from "
-                        + mInterfaceNames.get(downstream) + " to " + mInterfaceNames.get(upstream));
+                        + getIfName(downstream) + " to " + getIfName(upstream));
             }
         }
 
@@ -616,7 +616,7 @@
             if (!mBpfCoordinatorShim.stopUpstreamIpv6Forwarding(downstream, upstream,
                     rule.srcMac)) {
                 mLog.e("Failed to disable upstream IPv6 forwarding from "
-                        + mInterfaceNames.get(downstream) + " to " + mInterfaceNames.get(upstream));
+                        + getIfName(downstream) + " to " + getIfName(upstream));
             }
         }
 
@@ -960,8 +960,12 @@
     }
 
     // TODO: make mInterfaceNames accessible to the shim and move this code to there.
-    private String getIfName(long ifindex) {
-        return mInterfaceNames.get((int) ifindex, Long.toString(ifindex));
+    // This function should only be used for logging/dump purposes.
+    private String getIfName(int ifindex) {
+        // TODO: return something more useful on lookup failure
+        // likely use the 'iface_index_name_map' bpf map and/or if_nametoindex
+        // perhaps should even check that all 3 match if available.
+        return mInterfaceNames.get(ifindex, Integer.toString(ifindex));
     }
 
     /**
@@ -1038,8 +1042,8 @@
         for (int i = 0; i < mStats.size(); i++) {
             final int upstreamIfindex = mStats.keyAt(i);
             final ForwardedStats stats = mStats.get(upstreamIfindex);
-            pw.println(String.format("%d(%s) - %s", upstreamIfindex, mInterfaceNames.get(
-                    upstreamIfindex), stats.toString()));
+            pw.println(String.format("%d(%s) - %s", upstreamIfindex, getIfName(upstreamIfindex),
+                    stats.toString()));
         }
     }
     private void dumpBpfStats(@NonNull IndentingPrintWriter pw) {
@@ -1082,8 +1086,9 @@
             for (Ipv6ForwardingRule rule : rules.values()) {
                 final int upstreamIfindex = rule.upstreamIfindex;
                 pw.println(String.format("%d(%s) %d(%s) %s [%s] [%s]", upstreamIfindex,
-                        mInterfaceNames.get(upstreamIfindex), rule.downstreamIfindex,
-                        downstreamIface, rule.address.getHostAddress(), rule.srcMac, rule.dstMac));
+                        getIfName(upstreamIfindex), rule.downstreamIfindex,
+                        getIfName(rule.downstreamIfindex), rule.address.getHostAddress(),
+                        rule.srcMac, rule.dstMac));
             }
             pw.decreaseIndent();
         }
@@ -1817,8 +1822,7 @@
         // TODO: Perhaps stop the coordinator.
         boolean success = updateDataLimit(upstreamIfindex);
         if (!success) {
-            final String iface = mInterfaceNames.get(upstreamIfindex);
-            mLog.e("Setting data limit for " + iface + " failed.");
+            mLog.e("Setting data limit for " + getIfName(upstreamIfindex) + " failed.");
         }
     }