From 66b98ba2b127346baa12bd95cd4a265f89be9d78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 16 Sep 2022 03:52:11 +0000 Subject: [PATCH] Migrate BpfInterfaceMapUpdater from U32 to S32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit continuation of https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2184446/ replace U32 ifindex with S32 These are allocated in order by the kernel, if we go over 2 billion, we've got other problems... besides U32 to S32 conversion will work just fine anyway. Bug: 245472520 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I48df3bf86f0590fbd0e21b7cf9f19d1a6bbacd31 --- .../server/net/BpfInterfaceMapUpdater.java | 14 ++++++------- .../server/net/NetworkStatsService.java | 5 ++--- .../com/android/server/net/StatsMapKey.java | 6 +++--- .../net/BpfInterfaceMapUpdaterTest.java | 20 +++++++++---------- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/service-t/src/com/android/server/net/BpfInterfaceMapUpdater.java b/service-t/src/com/android/server/net/BpfInterfaceMapUpdater.java index 57466a6ea1..ceae9ba890 100644 --- a/service-t/src/com/android/server/net/BpfInterfaceMapUpdater.java +++ b/service-t/src/com/android/server/net/BpfInterfaceMapUpdater.java @@ -31,7 +31,7 @@ import com.android.net.module.util.BpfDump; import com.android.net.module.util.BpfMap; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.InterfaceParams; -import com.android.net.module.util.Struct.U32; +import com.android.net.module.util.Struct.S32; /** * Monitor interface added (without removed) and right interface name and its index to bpf map. @@ -41,7 +41,7 @@ public class BpfInterfaceMapUpdater { // This is current path but may be changed soon. private static final String IFACE_INDEX_NAME_MAP_PATH = "/sys/fs/bpf/netd_shared/map_netd_iface_index_name_map"; - private final IBpfMap mBpfMap; + private final IBpfMap mBpfMap; private final INetd mNetd; private final Handler mHandler; private final Dependencies mDeps; @@ -64,10 +64,10 @@ public class BpfInterfaceMapUpdater { @VisibleForTesting public static class Dependencies { /** Create BpfMap for updating interface and index mapping. */ - public IBpfMap getInterfaceMap() { + public IBpfMap getInterfaceMap() { try { return new BpfMap<>(IFACE_INDEX_NAME_MAP_PATH, BpfMap.BPF_F_RDWR, - U32.class, InterfaceMapValue.class); + S32.class, InterfaceMapValue.class); } catch (ErrnoException e) { Log.e(TAG, "Cannot create interface map: " + e); return null; @@ -126,7 +126,7 @@ public class BpfInterfaceMapUpdater { } try { - mBpfMap.updateEntry(new U32(iface.index), new InterfaceMapValue(ifaceName)); + mBpfMap.updateEntry(new S32(iface.index), new InterfaceMapValue(ifaceName)); } catch (ErrnoException e) { Log.e(TAG, "Unable to update entry for " + ifaceName + ", " + e); } @@ -140,9 +140,9 @@ public class BpfInterfaceMapUpdater { } /** get interface name by interface index from bpf map */ - public String getIfNameByIndex(final long index) { + public String getIfNameByIndex(final int index) { try { - final InterfaceMapValue value = mBpfMap.getValue(new U32(index)); + final InterfaceMapValue value = mBpfMap.getValue(new S32(index)); if (value == null) { Log.e(TAG, "No if name entry for index " + index); return null; diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 807f5d78c1..39a705f13a 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -2896,9 +2896,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { BpfDump.dumpMap(statsMap, pw, mapName, "ifaceIndex ifaceName tag_hex uid_int cnt_set rxBytes rxPackets txBytes txPackets", (key, value) -> { - final long ifIndex = key.ifaceIndex; - final String ifName = mInterfaceMapUpdater.getIfNameByIndex(ifIndex); - return ifIndex + " " + final String ifName = mInterfaceMapUpdater.getIfNameByIndex(key.ifaceIndex); + return key.ifaceIndex + " " + (ifName != null ? ifName : "unknown") + " " + "0x" + Long.toHexString(key.tag) + " " + key.uid + " " diff --git a/service-t/src/com/android/server/net/StatsMapKey.java b/service-t/src/com/android/server/net/StatsMapKey.java index ea8d836383..a26fa5be42 100644 --- a/service-t/src/com/android/server/net/StatsMapKey.java +++ b/service-t/src/com/android/server/net/StatsMapKey.java @@ -33,11 +33,11 @@ public class StatsMapKey extends Struct { @Field(order = 2, type = Type.U32) public final long counterSet; - @Field(order = 3, type = Type.U32) - public final long ifaceIndex; + @Field(order = 3, type = Type.S32) + public final int ifaceIndex; public StatsMapKey(final long uid, final long tag, final long counterSet, - final long ifaceIndex) { + final int ifaceIndex) { this.uid = uid; this.tag = tag; this.counterSet = counterSet; diff --git a/tests/unit/java/com/android/server/net/BpfInterfaceMapUpdaterTest.java b/tests/unit/java/com/android/server/net/BpfInterfaceMapUpdaterTest.java index 83e6b5f4cc..c730856815 100644 --- a/tests/unit/java/com/android/server/net/BpfInterfaceMapUpdaterTest.java +++ b/tests/unit/java/com/android/server/net/BpfInterfaceMapUpdaterTest.java @@ -42,7 +42,7 @@ import androidx.test.filters.SmallTest; import com.android.net.module.util.BaseNetdUnsolicitedEventListener; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.InterfaceParams; -import com.android.net.module.util.Struct.U32; +import com.android.net.module.util.Struct.S32; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRunner; import com.android.testutils.TestBpfMap; @@ -69,14 +69,14 @@ public final class BpfInterfaceMapUpdaterTest { private final TestLooper mLooper = new TestLooper(); private BaseNetdUnsolicitedEventListener mListener; private BpfInterfaceMapUpdater mUpdater; - private IBpfMap mBpfMap = - spy(new TestBpfMap<>(U32.class, InterfaceMapValue.class)); + private IBpfMap mBpfMap = + spy(new TestBpfMap<>(S32.class, InterfaceMapValue.class)); @Mock private INetd mNetd; @Mock private Context mContext; private class TestDependencies extends BpfInterfaceMapUpdater.Dependencies { @Override - public IBpfMap getInterfaceMap() { + public IBpfMap getInterfaceMap() { return mBpfMap; } @@ -114,7 +114,7 @@ public final class BpfInterfaceMapUpdaterTest { ArgumentCaptor.forClass(BaseNetdUnsolicitedEventListener.class); verify(mNetd).registerUnsolicitedEventListener(listenerCaptor.capture()); mListener = listenerCaptor.getValue(); - verify(mBpfMap).updateEntry(eq(new U32(TEST_INDEX)), + verify(mBpfMap).updateEntry(eq(new S32(TEST_INDEX)), eq(new InterfaceMapValue(TEST_INTERFACE_NAME))); } @@ -124,7 +124,7 @@ public final class BpfInterfaceMapUpdaterTest { mListener.onInterfaceAdded(TEST_INTERFACE_NAME2); mLooper.dispatchAll(); - verify(mBpfMap).updateEntry(eq(new U32(TEST_INDEX2)), + verify(mBpfMap).updateEntry(eq(new S32(TEST_INDEX2)), eq(new InterfaceMapValue(TEST_INTERFACE_NAME2))); // Check that when onInterfaceRemoved is called, nothing happens. @@ -135,7 +135,7 @@ public final class BpfInterfaceMapUpdaterTest { @Test public void testGetIfNameByIndex() throws Exception { - mBpfMap.updateEntry(new U32(TEST_INDEX), new InterfaceMapValue(TEST_INTERFACE_NAME)); + mBpfMap.updateEntry(new S32(TEST_INDEX), new InterfaceMapValue(TEST_INTERFACE_NAME)); assertEquals(TEST_INTERFACE_NAME, mUpdater.getIfNameByIndex(TEST_INDEX)); } @@ -146,7 +146,7 @@ public final class BpfInterfaceMapUpdaterTest { @Test public void testGetIfNameByIndexException() throws Exception { - doThrow(new ErrnoException("", EPERM)).when(mBpfMap).getValue(new U32(TEST_INDEX)); + doThrow(new ErrnoException("", EPERM)).when(mBpfMap).getValue(new S32(TEST_INDEX)); assertNull(mUpdater.getIfNameByIndex(TEST_INDEX)); } @@ -163,8 +163,8 @@ public final class BpfInterfaceMapUpdaterTest { @Test public void testDump() throws ErrnoException { - mBpfMap.updateEntry(new U32(TEST_INDEX), new InterfaceMapValue(TEST_INTERFACE_NAME)); - mBpfMap.updateEntry(new U32(TEST_INDEX2), new InterfaceMapValue(TEST_INTERFACE_NAME2)); + mBpfMap.updateEntry(new S32(TEST_INDEX), new InterfaceMapValue(TEST_INTERFACE_NAME)); + mBpfMap.updateEntry(new S32(TEST_INDEX2), new InterfaceMapValue(TEST_INTERFACE_NAME2)); final String dump = getDump(); assertDumpContains(dump, "IfaceIndexNameMap: OK");