From 1feb8b4ac834013def54014330fad1662b813480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 25 Jan 2021 12:01:31 -0800 Subject: [PATCH 1/3] merge Tether{Down,Up}stream4{Key,Value} - part 1 - C portion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keys are identical, and the values nearly so, this will make everyone's life easier. Test: git grep 'Tether(Down|Up)stream4(Key|Value)' finds nothing (note this requires follow up commits) Signed-off-by: Maciej Żenczykowski Change-Id: Ifbff2c617ac5834ea80f827eaf89ca81e862baec --- Tethering/bpf_progs/offload.c | 36 +++++++++++------------------------ 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/Tethering/bpf_progs/offload.c b/Tethering/bpf_progs/offload.c index 44c25e5d42..3a3291d6c9 100644 --- a/Tethering/bpf_progs/offload.c +++ b/Tethering/bpf_progs/offload.c @@ -285,11 +285,9 @@ DEFINE_BPF_PROG_KVER_RANGE("schedcls/tether_upstream6_rawip$stub", AID_ROOT, AID // ----- IPv4 Support ----- -DEFINE_BPF_MAP_GRW(tether_downstream4_map, HASH, TetherDownstream4Key, TetherDownstream4Value, 64, - AID_NETWORK_STACK) +DEFINE_BPF_MAP_GRW(tether_downstream4_map, HASH, Tether4Key, Tether4Value, 64, AID_NETWORK_STACK) -DEFINE_BPF_MAP_GRW(tether_upstream4_map, HASH, TetherUpstream4Key, TetherUpstream4Value, 64, - AID_NETWORK_STACK) +DEFINE_BPF_MAP_GRW(tether_upstream4_map, HASH, Tether4Key, Tether4Value, 64, AID_NETWORK_STACK) static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool is_ethernet, const bool downstream) { @@ -358,7 +356,7 @@ static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool if (data + l2_header_size + sizeof(*ip) + sizeof(*udph) > data_end) return TC_ACT_OK; } - TetherDownstream4Key kd = { + Tether4Key k = { .iif = skb->ifindex, .l4Proto = ip->protocol, .src4.s_addr = ip->saddr, @@ -366,26 +364,15 @@ static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool .srcPort = is_tcp ? tcph->source : udph->source, .dstPort = is_tcp ? tcph->dest : udph->dest, }; - if (is_ethernet) for (int i = 0; i < ETH_ALEN; ++i) kd.dstMac[i] = eth->h_dest[i]; + if (is_ethernet) for (int i = 0; i < ETH_ALEN; ++i) k.dstMac[i] = eth->h_dest[i]; - TetherUpstream4Key ku = { - .iif = skb->ifindex, - .l4Proto = ip->protocol, - .src4.s_addr = ip->saddr, - .dst4.s_addr = ip->daddr, - .srcPort = is_tcp ? tcph->source : udph->source, - .dstPort = is_tcp ? tcph->dest : udph->dest, - }; - if (is_ethernet) for (int i = 0; i < ETH_ALEN; ++i) ku.dstMac[i] = eth->h_dest[i]; - - TetherDownstream4Value* vd = downstream ? bpf_tether_downstream4_map_lookup_elem(&kd) : NULL; - TetherUpstream4Value* vu = downstream ? NULL : bpf_tether_upstream4_map_lookup_elem(&ku); + Tether4Value* v = downstream ? bpf_tether_downstream4_map_lookup_elem(&k) + : bpf_tether_upstream4_map_lookup_elem(&k); // If we don't find any offload information then simply let the core stack handle it... - if (downstream && !vd) return TC_ACT_OK; - if (!downstream && !vu) return TC_ACT_OK; + if (!v) return TC_ACT_OK; - uint32_t stat_and_limit_k = downstream ? skb->ifindex : vu->oif; + uint32_t stat_and_limit_k = downstream ? skb->ifindex : v->oif; TetherStatsValue* stat_v = bpf_tether_stats_map_lookup_elem(&stat_and_limit_k); @@ -398,8 +385,7 @@ static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool if (!limit_v) return TC_ACT_OK; // Required IPv4 minimum mtu is 68, below that not clear what we should do, abort... - const int pmtu = downstream ? vd->pmtu : vu->pmtu; - if (pmtu < 68) return TC_ACT_OK; + if (v->pmtu < 68) return TC_ACT_OK; // Approximate handling of TCP/IPv4 overhead for incoming LRO/GRO packets: default // outbound path mtu of 1500 is not necessarily correct, but worst case we simply @@ -410,9 +396,9 @@ static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool // (This is also blindly assuming 12 bytes of tcp timestamp option in tcp header) uint64_t packets = 1; uint64_t bytes = skb->len; - if (bytes > pmtu) { + if (bytes > v->pmtu) { const int tcp_overhead = sizeof(struct iphdr) + sizeof(struct tcphdr) + 12; - const int mss = pmtu - tcp_overhead; + const int mss = v->pmtu - tcp_overhead; const uint64_t payload = bytes - tcp_overhead; packets = (payload + mss - 1) / mss; bytes = tcp_overhead * packets + payload; From 32874eb6bcc782648b69028cc2c7b1c97a695370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 25 Jan 2021 18:06:34 -0800 Subject: [PATCH 2/3] merge Tether{Down,Up}stream4{Key,Value} - part 2 - java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated via: git grep 'Tether(Down|Up)stream4(Key|Value)' | cut -d: -f1 | sort -u | while read i; do sed -r -i 's@TetherUpstream4Value@Tether4Value@g' "$i" sed -r -i 's@TetherDownstream4Value@Tether4Value@g' "$i" sed -r -i 's@TetherDownstream4Key@Tether4Key@g' "$i" sed -r -i 's@TetherUpstream4Key@Tether4Key@g' "$i" done cd Tethering/src/com/android/networkstack/tethering git mv TetherUpstream4Key.java Tether4Key.java git mv TetherUpstream4Value.java Tether4Value.java git diff TetherDownstream4Key.java Tether4Key.java git diff TetherDownstream4Value.java Tether4Value.java git rm TetherDownstream4Key.java git rm TetherDownstream4Value.java Fixup resulting 'import' duplication mcedit Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java mcedit Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java mcedit Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java mcedit Tethering/tests/unit/src/android/net/ip/IpServerTest.java Test: N/A, requires follow up commit Signed-off-by: Maciej Żenczykowski Change-Id: I1dfc3108ca4bbd0cefc3420bc7e421594b62619c --- .../apishim/api30/BpfCoordinatorShimImpl.java | 18 ++-- .../apishim/api31/BpfCoordinatorShimImpl.java | 22 ++--- .../apishim/common/BpfCoordinatorShim.java | 18 ++-- .../tethering/BpfCoordinator.java | 32 +++--- ...etherUpstream4Key.java => Tether4Key.java} | 4 +- ...rUpstream4Value.java => Tether4Value.java} | 4 +- .../tethering/TetherDownstream4Key.java | 79 --------------- .../tethering/TetherDownstream4Value.java | 97 ------------------- .../unit/src/android/net/ip/IpServerTest.java | 14 ++- .../tethering/BpfCoordinatorTest.java | 8 +- 10 files changed, 56 insertions(+), 240 deletions(-) rename Tethering/src/com/android/networkstack/tethering/{TetherUpstream4Key.java => Tether4Key.java} (94%) rename Tethering/src/com/android/networkstack/tethering/{TetherUpstream4Value.java => Tether4Value.java} (96%) delete mode 100644 Tethering/src/com/android/networkstack/tethering/TetherDownstream4Key.java delete mode 100644 Tethering/src/com/android/networkstack/tethering/TetherDownstream4Value.java diff --git a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java index 90b9b3f7b2..d0a5af4283 100644 --- a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java @@ -28,11 +28,9 @@ import androidx.annotation.Nullable; import com.android.networkstack.tethering.BpfCoordinator.Dependencies; import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; -import com.android.networkstack.tethering.TetherDownstream4Key; -import com.android.networkstack.tethering.TetherDownstream4Value; +import com.android.networkstack.tethering.Tether4Key; +import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.TetherStatsValue; -import com.android.networkstack.tethering.TetherUpstream4Key; -import com.android.networkstack.tethering.TetherUpstream4Value; /** * Bpf coordinator class for API shims. @@ -136,27 +134,27 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleAdd(@NonNull TetherDownstream4Key key, - @NonNull TetherDownstream4Value value) { + public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value) { /* no op */ return true; } @Override - public boolean tetherOffloadRuleRemove(@NonNull TetherDownstream4Key key) { + public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { /* no op */ return true; } @Override - public boolean tetherOffloadRuleAdd(@NonNull TetherUpstream4Key key, - @NonNull TetherUpstream4Value value) { + public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value) { /* no op */ return true; } @Override - public boolean tetherOffloadRuleRemove(@NonNull TetherUpstream4Key key) { + public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { /* no op */ return true; } diff --git a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java index b9ce769f38..e6a2823a20 100644 --- a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java @@ -30,16 +30,14 @@ import androidx.annotation.Nullable; import com.android.networkstack.tethering.BpfCoordinator.Dependencies; import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; import com.android.networkstack.tethering.BpfMap; -import com.android.networkstack.tethering.TetherDownstream4Key; -import com.android.networkstack.tethering.TetherDownstream4Value; +import com.android.networkstack.tethering.Tether4Key; +import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.TetherDownstream6Key; import com.android.networkstack.tethering.TetherDownstream6Value; import com.android.networkstack.tethering.TetherLimitKey; import com.android.networkstack.tethering.TetherLimitValue; import com.android.networkstack.tethering.TetherStatsKey; import com.android.networkstack.tethering.TetherStatsValue; -import com.android.networkstack.tethering.TetherUpstream4Key; -import com.android.networkstack.tethering.TetherUpstream4Value; import java.io.FileDescriptor; @@ -61,12 +59,12 @@ public class BpfCoordinatorShimImpl // BPF map of ingress queueing discipline which pre-processes the packets by the IPv4 // downstream rules. @Nullable - private final BpfMap mBpfDownstream4Map; + private final BpfMap mBpfDownstream4Map; // BPF map of ingress queueing discipline which pre-processes the packets by the IPv4 // upstream rules. @Nullable - private final BpfMap mBpfUpstream4Map; + private final BpfMap mBpfUpstream4Map; // BPF map of ingress queueing discipline which pre-processes the packets by the IPv6 // forwarding rules. @@ -250,8 +248,8 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleAdd(@NonNull TetherDownstream4Key key, - @NonNull TetherDownstream4Value value) { + public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value) { if (!isInitialized()) return false; try { @@ -268,7 +266,7 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleRemove(@NonNull TetherDownstream4Key key) { + public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { if (!isInitialized()) return false; try { @@ -284,8 +282,8 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleAdd(@NonNull TetherUpstream4Key key, - @NonNull TetherUpstream4Value value) { + public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value) { if (!isInitialized()) return false; try { @@ -302,7 +300,7 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleRemove(@NonNull TetherUpstream4Key key) { + public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { if (!isInitialized()) return false; try { diff --git a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java index 36d2de1a5c..6bcec18095 100644 --- a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java +++ b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java @@ -23,11 +23,9 @@ import androidx.annotation.Nullable; import com.android.networkstack.tethering.BpfCoordinator.Dependencies; import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; -import com.android.networkstack.tethering.TetherDownstream4Key; -import com.android.networkstack.tethering.TetherDownstream4Value; +import com.android.networkstack.tethering.Tether4Key; +import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.TetherStatsValue; -import com.android.networkstack.tethering.TetherUpstream4Key; -import com.android.networkstack.tethering.TetherUpstream4Value; /** * Bpf coordinator class for API shims. @@ -116,23 +114,23 @@ public abstract class BpfCoordinatorShim { /** * Adds a tethering IPv4 downstream offload rule to BPF map. */ - public abstract boolean tetherOffloadRuleAdd(@NonNull TetherDownstream4Key key, - @NonNull TetherDownstream4Value value); + public abstract boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value); /** * Deletes a tethering IPv4 downstream offload rule from the BPF map. */ - public abstract boolean tetherOffloadRuleRemove(@NonNull TetherDownstream4Key key); + public abstract boolean tetherOffloadRuleRemove(@NonNull Tether4Key key); /** * Adds a tethering IPv4 upstream offload rule to BPF map. */ - public abstract boolean tetherOffloadRuleAdd(@NonNull TetherUpstream4Key key, - @NonNull TetherUpstream4Value value); + public abstract boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + @NonNull Tether4Value value); /** * Deletes a tethering IPv4 upstream offload rule from the BPF map. */ - public abstract boolean tetherOffloadRuleRemove(@NonNull TetherUpstream4Key key); + public abstract boolean tetherOffloadRuleRemove(@NonNull Tether4Key key); } diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index e4216d85b8..0c8784bf91 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -237,11 +237,11 @@ public class BpfCoordinator { } /** Get downstream4 BPF map. */ - @Nullable public BpfMap + @Nullable public BpfMap getBpfDownstream4Map() { try { return new BpfMap<>(TETHER_DOWNSTREAM4_MAP_PATH, - BpfMap.BPF_F_RDWR, TetherDownstream4Key.class, TetherDownstream4Value.class); + BpfMap.BPF_F_RDWR, Tether4Key.class, Tether4Value.class); } catch (ErrnoException e) { Log.e(TAG, "Cannot create downstream4 map: " + e); return null; @@ -249,11 +249,11 @@ public class BpfCoordinator { } /** Get upstream4 BPF map. */ - @Nullable public BpfMap + @Nullable public BpfMap getBpfUpstream4Map() { try { return new BpfMap<>(TETHER_UPSTREAM4_MAP_PATH, - BpfMap.BPF_F_RDWR, TetherUpstream4Key.class, TetherUpstream4Value.class); + BpfMap.BPF_F_RDWR, Tether4Key.class, Tether4Value.class); } catch (ErrnoException e) { Log.e(TAG, "Cannot create upstream4 map: " + e); return null; @@ -906,25 +906,25 @@ public class BpfCoordinator { // TODO: add ether ip support. private class BpfConntrackEventConsumer implements ConntrackEventConsumer { @NonNull - private TetherUpstream4Key makeTetherUpstream4Key( + private Tether4Key makeTether4Key( @NonNull ConntrackEvent e, @NonNull ClientInfo c) { - return new TetherUpstream4Key(c.downstreamIfindex, c.downstreamMac, + return new Tether4Key(c.downstreamIfindex, c.downstreamMac, e.tupleOrig.protoNum, e.tupleOrig.srcIp.getAddress(), e.tupleOrig.dstIp.getAddress(), e.tupleOrig.srcPort, e.tupleOrig.dstPort); } @NonNull - private TetherDownstream4Key makeTetherDownstream4Key( + private Tether4Key makeTether4Key( @NonNull ConntrackEvent e, @NonNull ClientInfo c, int upstreamIndex) { - return new TetherDownstream4Key(upstreamIndex, NULL_MAC_ADDRESS /* dstMac (rawip) */, + return new Tether4Key(upstreamIndex, NULL_MAC_ADDRESS /* dstMac (rawip) */, e.tupleReply.protoNum, e.tupleReply.srcIp.getAddress(), e.tupleReply.dstIp.getAddress(), e.tupleReply.srcPort, e.tupleReply.dstPort); } @NonNull - private TetherUpstream4Value makeTetherUpstream4Value(@NonNull ConntrackEvent e, + private Tether4Value makeTether4Value(@NonNull ConntrackEvent e, int upstreamIndex) { - return new TetherUpstream4Value(upstreamIndex, + return new Tether4Value(upstreamIndex, NULL_MAC_ADDRESS /* ethDstMac (rawip) */, NULL_MAC_ADDRESS /* ethSrcMac (rawip) */, ETH_P_IP, NetworkStackConstants.ETHER_MTU, toIpv4MappedAddressBytes(e.tupleReply.dstIp), @@ -933,9 +933,9 @@ public class BpfCoordinator { } @NonNull - private TetherDownstream4Value makeTetherDownstream4Value(@NonNull ConntrackEvent e, + private Tether4Value makeTether4Value(@NonNull ConntrackEvent e, @NonNull ClientInfo c, int upstreamIndex) { - return new TetherDownstream4Value(c.downstreamIfindex, + return new Tether4Value(c.downstreamIfindex, c.clientMac, c.downstreamMac, ETH_P_IP, NetworkStackConstants.ETHER_MTU, e.tupleOrig.dstIp.getAddress(), e.tupleOrig.srcIp.getAddress(), e.tupleOrig.dstPort, e.tupleOrig.srcPort, @@ -962,8 +962,8 @@ public class BpfCoordinator { final Integer upstreamIndex = mIpv4UpstreamIndices.get(e.tupleReply.dstIp); if (upstreamIndex == null) return; - final TetherUpstream4Key upstream4Key = makeTetherUpstream4Key(e, tetherClient); - final TetherDownstream4Key downstream4Key = makeTetherDownstream4Key(e, + final Tether4Key upstream4Key = makeTether4Key(e, tetherClient); + final Tether4Key downstream4Key = makeTether4Key(e, tetherClient, upstreamIndex); if (e.msgType == (NetlinkConstants.NFNL_SUBSYS_CTNETLINK << 8 @@ -973,9 +973,9 @@ public class BpfCoordinator { return; } - final TetherUpstream4Value upstream4Value = makeTetherUpstream4Value(e, + final Tether4Value upstream4Value = makeTether4Value(e, upstreamIndex); - final TetherDownstream4Value downstream4Value = makeTetherDownstream4Value(e, + final Tether4Value downstream4Value = makeTether4Value(e, tetherClient, upstreamIndex); mBpfCoordinatorShim.tetherOffloadRuleAdd(upstream4Key, upstream4Value); diff --git a/Tethering/src/com/android/networkstack/tethering/TetherUpstream4Key.java b/Tethering/src/com/android/networkstack/tethering/Tether4Key.java similarity index 94% rename from Tethering/src/com/android/networkstack/tethering/TetherUpstream4Key.java rename to Tethering/src/com/android/networkstack/tethering/Tether4Key.java index 77cfa996fe..5884dceaee 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetherUpstream4Key.java +++ b/Tethering/src/com/android/networkstack/tethering/Tether4Key.java @@ -29,7 +29,7 @@ import java.net.UnknownHostException; import java.util.Objects; /** The key of BpfMap which is used for IPv4 bpf offload. */ -public class TetherUpstream4Key extends Struct { +public class Tether4Key extends Struct { @Field(order = 0, type = Type.U32) public final long iif; @@ -51,7 +51,7 @@ public class TetherUpstream4Key extends Struct { @Field(order = 6, type = Type.UBE16) public final int dstPort; - public TetherUpstream4Key(final long iif, @NonNull final MacAddress dstMac, final short l4proto, + public Tether4Key(final long iif, @NonNull final MacAddress dstMac, final short l4proto, final byte[] src4, final byte[] dst4, final int srcPort, final int dstPort) { Objects.requireNonNull(dstMac); diff --git a/Tethering/src/com/android/networkstack/tethering/TetherUpstream4Value.java b/Tethering/src/com/android/networkstack/tethering/Tether4Value.java similarity index 96% rename from Tethering/src/com/android/networkstack/tethering/TetherUpstream4Value.java rename to Tethering/src/com/android/networkstack/tethering/Tether4Value.java index e1ff688dd5..3770f1ad6b 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetherUpstream4Value.java +++ b/Tethering/src/com/android/networkstack/tethering/Tether4Value.java @@ -29,7 +29,7 @@ import java.net.UnknownHostException; import java.util.Objects; /** The value of BpfMap which is used for IPv4 bpf offload. */ -public class TetherUpstream4Value extends Struct { +public class Tether4Value extends Struct { @Field(order = 0, type = Type.U32) public final long oif; @@ -60,7 +60,7 @@ public class TetherUpstream4Value extends Struct { @Field(order = 9, type = Type.U63) public final long lastUsed; - public TetherUpstream4Value(final long oif, @NonNull final MacAddress ethDstMac, + public Tether4Value(final long oif, @NonNull final MacAddress ethDstMac, @NonNull final MacAddress ethSrcMac, final int ethProto, final int pmtu, final byte[] src46, final byte[] dst46, final int srcPort, final int dstPort, final long lastUsed) { diff --git a/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Key.java b/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Key.java deleted file mode 100644 index 51b1f76f35..0000000000 --- a/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Key.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.networkstack.tethering; - -import android.net.MacAddress; - -import com.android.net.module.util.Struct; -import com.android.net.module.util.Struct.Field; -import com.android.net.module.util.Struct.Type; - -import java.net.Inet4Address; -import java.net.UnknownHostException; -import java.util.Objects; - -/** The key of BpfMap which is used for IPv4 bpf offload. */ -public class TetherDownstream4Key extends Struct { - @Field(order = 0, type = Type.U32) - public final long iif; - - @Field(order = 1, type = Type.EUI48) - public final MacAddress dstMac; - - @Field(order = 2, type = Type.U8, padding = 1) - public final short l4proto; - - @Field(order = 3, type = Type.ByteArray, arraysize = 4) - public final byte[] src4; - - @Field(order = 4, type = Type.ByteArray, arraysize = 4) - public final byte[] dst4; - - @Field(order = 5, type = Type.UBE16) - public final int srcPort; - - @Field(order = 6, type = Type.UBE16) - public final int dstPort; - - public TetherDownstream4Key(final long iif, final MacAddress dstMac, final short l4proto, - final byte[] src4, final byte[] dst4, final int srcPort, - final int dstPort) { - Objects.requireNonNull(dstMac); - - this.iif = iif; - this.dstMac = dstMac; - this.l4proto = l4proto; - this.src4 = src4; - this.dst4 = dst4; - this.srcPort = srcPort; - this.dstPort = dstPort; - } - - @Override - public String toString() { - try { - return String.format( - "iif: %d, dstMac: %s, l4proto: %d, src4: %s, dst4: %s, " - + "srcPort: %d, dstPort: %d", - iif, dstMac, l4proto, - Inet4Address.getByAddress(src4), Inet4Address.getByAddress(dst4), - Short.toUnsignedInt((short) srcPort), Short.toUnsignedInt((short) dstPort)); - } catch (UnknownHostException | IllegalArgumentException e) { - return String.format("Invalid IP address", e); - } - } -} diff --git a/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Value.java b/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Value.java deleted file mode 100644 index 56ea56675b..0000000000 --- a/Tethering/src/com/android/networkstack/tethering/TetherDownstream4Value.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.networkstack.tethering; - -import android.net.MacAddress; - -import androidx.annotation.NonNull; - -import com.android.net.module.util.Struct; -import com.android.net.module.util.Struct.Field; -import com.android.net.module.util.Struct.Type; - -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.Objects; - -/** The value of BpfMap which is used for IPv4 bpf offload. */ -public class TetherDownstream4Value extends Struct { - @Field(order = 0, type = Type.U32) - public final long oif; - - // The ethhdr struct which is defined in uapi/linux/if_ether.h - @Field(order = 1, type = Type.EUI48) - public final MacAddress ethDstMac; - @Field(order = 2, type = Type.EUI48) - public final MacAddress ethSrcMac; - @Field(order = 3, type = Type.UBE16) - public final int ethProto; // Packet type ID field. - - @Field(order = 4, type = Type.U16) - public final int pmtu; - - @Field(order = 5, type = Type.ByteArray, arraysize = 4) - public final byte[] src4; - - @Field(order = 6, type = Type.ByteArray, arraysize = 4) - public final byte[] dst4; - - @Field(order = 7, type = Type.UBE16) - public final int srcPort; - - @Field(order = 8, type = Type.UBE16) - public final int dstPort; - - // TODO: consider using U64. - @Field(order = 9, type = Type.U63) - public final long lastUsed; - - public TetherDownstream4Value(final long oif, @NonNull final MacAddress ethDstMac, - @NonNull final MacAddress ethSrcMac, final int ethProto, final int pmtu, - final byte[] src4, final byte[] dst4, final int srcPort, - final int dstPort, final long lastUsed) { - Objects.requireNonNull(ethDstMac); - Objects.requireNonNull(ethSrcMac); - - this.oif = oif; - this.ethDstMac = ethDstMac; - this.ethSrcMac = ethSrcMac; - this.ethProto = ethProto; - this.pmtu = pmtu; - this.src4 = src4; - this.dst4 = dst4; - this.srcPort = srcPort; - this.dstPort = dstPort; - this.lastUsed = lastUsed; - } - - @Override - public String toString() { - try { - return String.format( - "oif: %d, ethDstMac: %s, ethSrcMac: %s, ethProto: %d, pmtu: %d, " - + "src4: %s, dst4: %s, srcPort: %d, dstPort: %d, " - + "lastUsed: %d", - oif, ethDstMac, ethSrcMac, ethProto, pmtu, - InetAddress.getByAddress(src4), InetAddress.getByAddress(dst4), - Short.toUnsignedInt((short) srcPort), Short.toUnsignedInt((short) dstPort), - lastUsed); - } catch (UnknownHostException | IllegalArgumentException e) { - return String.format("Invalid IP address", e); - } - } -} diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 9b42c73c79..e5cc4cc944 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -104,16 +104,14 @@ import com.android.networkstack.tethering.BpfCoordinator; import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; import com.android.networkstack.tethering.BpfMap; import com.android.networkstack.tethering.PrivateAddressCoordinator; -import com.android.networkstack.tethering.TetherDownstream4Key; -import com.android.networkstack.tethering.TetherDownstream4Value; +import com.android.networkstack.tethering.Tether4Key; +import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.TetherDownstream6Key; import com.android.networkstack.tethering.TetherDownstream6Value; import com.android.networkstack.tethering.TetherLimitKey; import com.android.networkstack.tethering.TetherLimitValue; import com.android.networkstack.tethering.TetherStatsKey; import com.android.networkstack.tethering.TetherStatsValue; -import com.android.networkstack.tethering.TetherUpstream4Key; -import com.android.networkstack.tethering.TetherUpstream4Value; import com.android.networkstack.tethering.TetheringConfiguration; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter; @@ -177,8 +175,8 @@ public class IpServerTest { @Mock private NetworkStatsManager mStatsManager; @Mock private TetheringConfiguration mTetherConfig; @Mock private ConntrackMonitor mConntrackMonitor; - @Mock private BpfMap mBpfDownstream4Map; - @Mock private BpfMap mBpfUpstream4Map; + @Mock private BpfMap mBpfDownstream4Map; + @Mock private BpfMap mBpfUpstream4Map; @Mock private BpfMap mBpfDownstream6Map; @Mock private BpfMap mBpfStatsMap; @Mock private BpfMap mBpfLimitMap; @@ -309,13 +307,13 @@ public class IpServerTest { } @Nullable - public BpfMap + public BpfMap getBpfDownstream4Map() { return mBpfDownstream4Map; } @Nullable - public BpfMap + public BpfMap getBpfUpstream4Map() { return mBpfUpstream4Map; } 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 30b4bf4eeb..af8053a7e9 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -158,8 +158,8 @@ public class BpfCoordinatorTest { @Mock private IpServer mIpServer2; @Mock private TetheringConfiguration mTetherConfig; @Mock private ConntrackMonitor mConntrackMonitor; - @Mock private BpfMap mBpfDownstream4Map; - @Mock private BpfMap mBpfUpstream4Map; + @Mock private BpfMap mBpfDownstream4Map; + @Mock private BpfMap mBpfUpstream4Map; @Mock private BpfMap mBpfDownstream6Map; // Late init since methods must be called by the thread that created this object. @@ -205,13 +205,13 @@ public class BpfCoordinatorTest { } @Nullable - public BpfMap + public BpfMap getBpfDownstream4Map() { return mBpfDownstream4Map; } @Nullable - public BpfMap + public BpfMap getBpfUpstream4Map() { return mBpfUpstream4Map; } From 911a7267f5a11b70941c1f0c786c76e4a36689b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 25 Jan 2021 20:19:47 -0800 Subject: [PATCH 3/3] merge Tether{Down,Up}stream4{Key,Value} - part 3 - fixups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: atest, TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: Ia7840698e80ded33d8e0b59efe1ca7267254b892 --- .../apishim/api30/BpfCoordinatorShimImpl.java | 17 +------ .../apishim/api31/BpfCoordinatorShimImpl.java | 48 +++++-------------- .../apishim/common/BpfCoordinatorShim.java | 19 ++------ .../tethering/BpfCoordinator.java | 8 ++-- 4 files changed, 21 insertions(+), 71 deletions(-) diff --git a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java index d0a5af4283..715e3a5643 100644 --- a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java @@ -134,27 +134,14 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + public boolean tetherOffloadRuleAdd(boolean downstream, @NonNull Tether4Key key, @NonNull Tether4Value value) { /* no op */ return true; } @Override - public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { - /* no op */ - return true; - } - - @Override - public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, - @NonNull Tether4Value value) { - /* no op */ - return true; - } - - @Override - public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { + public boolean tetherOffloadRuleRemove(boolean downstream, @NonNull Tether4Key key) { /* no op */ return true; } diff --git a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java index e6a2823a20..389ba7b7a8 100644 --- a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java @@ -248,7 +248,7 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + public boolean tetherOffloadRuleAdd(boolean downstream, @NonNull Tether4Key key, @NonNull Tether4Value value) { if (!isInitialized()) return false; @@ -257,41 +257,11 @@ public class BpfCoordinatorShimImpl // map pair twice causes the unexpected refresh. Must be fixed before starting the // conntrack timeout extension implementation. // TODO: consider using insertEntry. - mBpfDownstream4Map.updateEntry(key, value); - } catch (ErrnoException e) { - mLog.e("Could not update entry: ", e); - return false; - } - return true; - } - - @Override - public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { - if (!isInitialized()) return false; - - try { - mBpfDownstream4Map.deleteEntry(key); - } catch (ErrnoException e) { - // Silent if the rule did not exist. - if (e.errno != OsConstants.ENOENT) { - mLog.e("Could not delete entry: ", e); - return false; + if (downstream) { + mBpfDownstream4Map.updateEntry(key, value); + } else { + mBpfUpstream4Map.updateEntry(key, value); } - } - return true; - } - - @Override - public boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, - @NonNull Tether4Value value) { - if (!isInitialized()) return false; - - try { - // The last used time field of the value is updated by the bpf program. Adding the same - // map pair twice causes the unexpected refresh. Must be fixed before starting the - // conntrack timeout extension implementation. - // TODO: consider using insertEntry. - mBpfUpstream4Map.updateEntry(key, value); } catch (ErrnoException e) { mLog.e("Could not update entry: ", e); return false; @@ -300,11 +270,15 @@ public class BpfCoordinatorShimImpl } @Override - public boolean tetherOffloadRuleRemove(@NonNull Tether4Key key) { + public boolean tetherOffloadRuleRemove(boolean downstream, @NonNull Tether4Key key) { if (!isInitialized()) return false; try { - mBpfUpstream4Map.deleteEntry(key); + if (downstream) { + mBpfDownstream4Map.deleteEntry(key); + } else { + mBpfUpstream4Map.deleteEntry(key); + } } catch (ErrnoException e) { // Silent if the rule did not exist. if (e.errno != OsConstants.ENOENT) { diff --git a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java index 6bcec18095..cbd843b074 100644 --- a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java +++ b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java @@ -112,25 +112,14 @@ public abstract class BpfCoordinatorShim { public abstract TetherStatsValue tetherOffloadGetAndClearStats(int ifIndex); /** - * Adds a tethering IPv4 downstream offload rule to BPF map. + * Adds a tethering IPv4 offload rule to appropriate BPF map. */ - public abstract boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, + public abstract boolean tetherOffloadRuleAdd(boolean downstream, @NonNull Tether4Key key, @NonNull Tether4Value value); /** - * Deletes a tethering IPv4 downstream offload rule from the BPF map. + * Deletes a tethering IPv4 offload rule from the appropriate BPF map. */ - public abstract boolean tetherOffloadRuleRemove(@NonNull Tether4Key key); - - /** - * Adds a tethering IPv4 upstream offload rule to BPF map. - */ - public abstract boolean tetherOffloadRuleAdd(@NonNull Tether4Key key, - @NonNull Tether4Value value); - - /** - * Deletes a tethering IPv4 upstream offload rule from the BPF map. - */ - public abstract boolean tetherOffloadRuleRemove(@NonNull Tether4Key key); + public abstract boolean tetherOffloadRuleRemove(boolean downstream, @NonNull Tether4Key key); } diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 0c8784bf91..ffb45570a7 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -968,8 +968,8 @@ public class BpfCoordinator { if (e.msgType == (NetlinkConstants.NFNL_SUBSYS_CTNETLINK << 8 | NetlinkConstants.IPCTNL_MSG_CT_DELETE)) { - mBpfCoordinatorShim.tetherOffloadRuleRemove(upstream4Key); - mBpfCoordinatorShim.tetherOffloadRuleRemove(downstream4Key); + mBpfCoordinatorShim.tetherOffloadRuleRemove(false, upstream4Key); + mBpfCoordinatorShim.tetherOffloadRuleRemove(true, downstream4Key); return; } @@ -978,8 +978,8 @@ public class BpfCoordinator { final Tether4Value downstream4Value = makeTether4Value(e, tetherClient, upstreamIndex); - mBpfCoordinatorShim.tetherOffloadRuleAdd(upstream4Key, upstream4Value); - mBpfCoordinatorShim.tetherOffloadRuleAdd(downstream4Key, downstream4Value); + mBpfCoordinatorShim.tetherOffloadRuleAdd(false, upstream4Key, upstream4Value); + mBpfCoordinatorShim.tetherOffloadRuleAdd(true, downstream4Key, downstream4Value); } }