From 9ab28d0832cb3d0b5703da02f24645fecae96845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Wed, 17 Aug 2022 14:45:31 +0000 Subject: [PATCH] use getIfName in more spots to improve logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I33eb46185fde90cd4d158bb74f8f9c17b41434fb --- .../tethering/BpfCoordinator.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 5052eb53f6..05a28841b3 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -575,7 +575,7 @@ public class BpfCoordinator { 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 @@ public class BpfCoordinator { 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 @@ public class BpfCoordinator { } // 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 @@ public class BpfCoordinator { 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 @@ public class BpfCoordinator { 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 @@ public class BpfCoordinator { // 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."); } }