From 51c1a95c82349b152294abacefa1f40b79abdb8b Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 26 Jan 2021 22:30:05 +0900 Subject: [PATCH 1/2] Address comments on aosp/1559686. Also remove some unnecessary line wrapping. Test: atest TetheringTests Change-Id: Ia7638b3198d7811cdbb34e959c50608cf1a656bf --- .../tethering/BpfCoordinator.java | 33 +++++++++---------- .../unit/src/android/net/ip/IpServerTest.java | 12 +++---- .../tethering/BpfCoordinatorTest.java | 12 +++---- 3 files changed, 23 insertions(+), 34 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 8c3f565527..35de4003ea 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -245,8 +245,7 @@ public class BpfCoordinator { } /** Get downstream4 BPF map. */ - @Nullable public BpfMap - getBpfDownstream4Map() { + @Nullable public BpfMap getBpfDownstream4Map() { try { return new BpfMap<>(TETHER_DOWNSTREAM4_MAP_PATH, BpfMap.BPF_F_RDWR, Tether4Key.class, Tether4Value.class); @@ -257,20 +256,18 @@ public class BpfCoordinator { } /** Get upstream4 BPF map. */ - @Nullable public BpfMap - getBpfUpstream4Map() { + @Nullable public BpfMap getBpfUpstream4Map() { try { return new BpfMap<>(TETHER_UPSTREAM4_MAP_PATH, BpfMap.BPF_F_RDWR, Tether4Key.class, Tether4Value.class); } catch (ErrnoException e) { Log.e(TAG, "Cannot create upstream4 map: " + e); + return null; } - return null; } /** Get downstream6 BPF map. */ - @Nullable public BpfMap - getBpfDownstream6Map() { + @Nullable public BpfMap getBpfDownstream6Map() { try { return new BpfMap<>(TETHER_DOWNSTREAM6_FS_PATH, BpfMap.BPF_F_RDWR, TetherDownstream6Key.class, Tether6Value.class); @@ -958,7 +955,7 @@ public class BpfCoordinator { // TODO: add ether ip support. private class BpfConntrackEventConsumer implements ConntrackEventConsumer { @NonNull - private Tether4Key makeTether4Key( + private Tether4Key makeTetherUpstream4Key( @NonNull ConntrackEvent e, @NonNull ClientInfo c) { return new Tether4Key(c.downstreamIfindex, c.downstreamMac, e.tupleOrig.protoNum, e.tupleOrig.srcIp.getAddress(), @@ -966,7 +963,7 @@ public class BpfCoordinator { } @NonNull - private Tether4Key makeTether4Key( + private Tether4Key makeTetherDownstream4Key( @NonNull ConntrackEvent e, @NonNull ClientInfo c, int upstreamIndex) { return new Tether4Key(upstreamIndex, NULL_MAC_ADDRESS /* dstMac (rawip) */, e.tupleReply.protoNum, e.tupleReply.srcIp.getAddress(), @@ -974,7 +971,7 @@ public class BpfCoordinator { } @NonNull - private Tether4Value makeTether4Value(@NonNull ConntrackEvent e, + private Tether4Value makeTetherUpstream4Value(@NonNull ConntrackEvent e, int upstreamIndex) { return new Tether4Value(upstreamIndex, NULL_MAC_ADDRESS /* ethDstMac (rawip) */, @@ -985,11 +982,12 @@ public class BpfCoordinator { } @NonNull - private Tether4Value makeTether4Value(@NonNull ConntrackEvent e, + private Tether4Value makeTetherDownstream4Value(@NonNull ConntrackEvent e, @NonNull ClientInfo c, int upstreamIndex) { return new Tether4Value(c.downstreamIfindex, c.clientMac, c.downstreamMac, ETH_P_IP, NetworkStackConstants.ETHER_MTU, - e.tupleOrig.dstIp.getAddress(), e.tupleOrig.srcIp.getAddress(), + toIpv4MappedAddressBytes(e.tupleOrig.dstIp), + toIpv4MappedAddressBytes(e.tupleOrig.srcIp), e.tupleOrig.dstPort, e.tupleOrig.srcPort, 0 /* lastUsed, filled by bpf prog only */); } @@ -1014,9 +1012,9 @@ public class BpfCoordinator { final Integer upstreamIndex = mIpv4UpstreamIndices.get(e.tupleReply.dstIp); if (upstreamIndex == null) return; - final Tether4Key upstream4Key = makeTether4Key(e, tetherClient); - final Tether4Key downstream4Key = makeTether4Key(e, - tetherClient, upstreamIndex); + final Tether4Key upstream4Key = makeTetherUpstream4Key(e, tetherClient); + final Tether4Key downstream4Key = makeTetherDownstream4Key(e, tetherClient, + upstreamIndex); if (e.msgType == (NetlinkConstants.NFNL_SUBSYS_CTNETLINK << 8 | NetlinkConstants.IPCTNL_MSG_CT_DELETE)) { @@ -1025,10 +1023,9 @@ public class BpfCoordinator { return; } - final Tether4Value upstream4Value = makeTether4Value(e, + final Tether4Value upstream4Value = makeTetherUpstream4Value(e, upstreamIndex); + final Tether4Value downstream4Value = makeTetherDownstream4Value(e, tetherClient, upstreamIndex); - final Tether4Value downstream4Value = makeTether4Value(e, - tetherClient, upstreamIndex); mBpfCoordinatorShim.tetherOffloadRuleAdd(false, upstream4Key, upstream4Value); mBpfCoordinatorShim.tetherOffloadRuleAdd(true, downstream4Key, downstream4Value); diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index c26458ccae..16315cfc1b 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -309,26 +309,22 @@ public class IpServerTest { } @Nullable - public BpfMap - getBpfDownstream4Map() { + public BpfMap getBpfDownstream4Map() { return mBpfDownstream4Map; } @Nullable - public BpfMap - getBpfUpstream4Map() { + public BpfMap getBpfUpstream4Map() { return mBpfUpstream4Map; } @Nullable - public BpfMap - getBpfDownstream6Map() { + public BpfMap getBpfDownstream6Map() { return mBpfDownstream6Map; } @Nullable - public BpfMap - getBpfUpstream6Map() { + public BpfMap getBpfUpstream6Map() { return mBpfUpstream6Map; } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index 0f9ca3db23..1270e50c42 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -206,26 +206,22 @@ public class BpfCoordinatorTest { } @Nullable - public BpfMap - getBpfDownstream4Map() { + public BpfMap getBpfDownstream4Map() { return mBpfDownstream4Map; } @Nullable - public BpfMap - getBpfUpstream4Map() { + public BpfMap getBpfUpstream4Map() { return mBpfUpstream4Map; } @Nullable - public BpfMap - getBpfDownstream6Map() { + public BpfMap getBpfDownstream6Map() { return mBpfDownstream6Map; } @Nullable - public BpfMap - getBpfUpstream6Map() { + public BpfMap getBpfUpstream6Map() { return mBpfUpstream6Map; } From f3b201f819edbb7c5fe1fb103dd5ac154da35eb8 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 26 Jan 2021 22:46:27 +0900 Subject: [PATCH 2/2] Remove IpServer.Dependencies#getIfIndex. This code is unused. Test: atest TetheringTests Change-Id: Iaac422d72e8538b67798cb3ae3737deb7b426401 --- Tethering/src/android/net/ip/IpServer.java | 12 ------------ .../tests/unit/src/android/net/ip/IpServerTest.java | 3 --- 2 files changed, 15 deletions(-) diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index 7f3e80fb27..194737af21 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -71,10 +71,8 @@ import com.android.networkstack.tethering.BpfCoordinator.ClientInfo; import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; import com.android.networkstack.tethering.PrivateAddressCoordinator; -import java.io.IOException; import java.net.Inet4Address; import java.net.Inet6Address; -import java.net.NetworkInterface; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; @@ -187,16 +185,6 @@ public class IpServer extends StateMachine { return InterfaceParams.getByName(ifName); } - /** Get |ifName|'s interface index. */ - public int getIfindex(String ifName) { - try { - return NetworkInterface.getByName(ifName).getIndex(); - } catch (IOException | NullPointerException e) { - Log.e(TAG, "Can't determine interface index for interface " + ifName); - return 0; - } - } - /** Create a DhcpServer instance to be used by IpServer. */ public abstract void makeDhcpServer(String ifName, DhcpServingParamsParcel params, DhcpServerCallbacks cb); diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 16315cfc1b..b45db7edff 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -206,9 +206,6 @@ public class IpServerTest { when(mDependencies.getInterfaceParams(UPSTREAM_IFACE)).thenReturn(UPSTREAM_IFACE_PARAMS); when(mDependencies.getInterfaceParams(UPSTREAM_IFACE2)).thenReturn(UPSTREAM_IFACE_PARAMS2); - when(mDependencies.getIfindex(eq(UPSTREAM_IFACE))).thenReturn(UPSTREAM_IFINDEX); - when(mDependencies.getIfindex(eq(UPSTREAM_IFACE2))).thenReturn(UPSTREAM_IFINDEX2); - mInterfaceConfiguration = new InterfaceConfigurationParcel(); mInterfaceConfiguration.flags = new String[0]; if (interfaceType == TETHERING_BLUETOOTH) {