From 548e35eb4257ce8bed9772d7b32ddbc8291e9454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 22 Aug 2024 16:55:19 -0700 Subject: [PATCH 1/5] bpf: git mv tests/mts bpf/tests/mts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Another step towards moving all bpf related stuff into a common directory, which will ease development. I want to refactor this test so that it can also be run as a self-test in UML (for testing bpfloading functionality across a large variety of Linux kernel (both major and minor LTS) and Android (API/SDK level) platform versions. Bug: 205608149 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: Idc9104783b2fa60626e9c1fd463c4d94fd4a908c --- {tests => bpf/tests}/mts/Android.bp | 0 {tests => bpf/tests}/mts/OWNERS | 0 {tests => bpf/tests}/mts/bpf_existence_test.cpp | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename {tests => bpf/tests}/mts/Android.bp (100%) rename {tests => bpf/tests}/mts/OWNERS (100%) rename {tests => bpf/tests}/mts/bpf_existence_test.cpp (100%) diff --git a/tests/mts/Android.bp b/bpf/tests/mts/Android.bp similarity index 100% rename from tests/mts/Android.bp rename to bpf/tests/mts/Android.bp diff --git a/tests/mts/OWNERS b/bpf/tests/mts/OWNERS similarity index 100% rename from tests/mts/OWNERS rename to bpf/tests/mts/OWNERS diff --git a/tests/mts/bpf_existence_test.cpp b/bpf/tests/mts/bpf_existence_test.cpp similarity index 100% rename from tests/mts/bpf_existence_test.cpp rename to bpf/tests/mts/bpf_existence_test.cpp From d6c0db3b41a0b93d577b8841490f94ae59b1692d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 22 Aug 2024 18:11:30 +0000 Subject: [PATCH 2/5] dscpPolicy: switch to bool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 361492282 Test: TreeHugger, atest CtsNetTestCases:android.net.cts.DscpPolicyTest Signed-off-by: Maciej Żenczykowski Change-Id: I3bd83e19a814d811028f8bd60d3e0a794050338e --- bpf/progs/dscpPolicy.c | 8 ++-- bpf/progs/dscpPolicy.h | 15 +++---- .../server/connectivity/DscpPolicyValue.java | 41 +++++++------------ 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/bpf/progs/dscpPolicy.c b/bpf/progs/dscpPolicy.c index 4bdd3edfbb..baabb02efa 100644 --- a/bpf/progs/dscpPolicy.c +++ b/bpf/progs/dscpPolicy.c @@ -155,19 +155,19 @@ static inline __always_inline void match_policy(struct __sk_buff* skb, const boo int score = 0; - if (policy->present_fields & PROTO_MASK_FLAG) { + if (policy->match_proto) { if (protocol != policy->proto) continue; score += 0xFFFF; } - if (policy->present_fields & SRC_IP_MASK_FLAG) { + if (policy->match_src_ip) { if (v6_not_equal(src_ip, policy->src_ip)) continue; score += 0xFFFF; } - if (policy->present_fields & DST_IP_MASK_FLAG) { + if (policy->match_dst_ip) { if (v6_not_equal(dst_ip, policy->dst_ip)) continue; score += 0xFFFF; } - if (policy->present_fields & SRC_PORT_MASK_FLAG) { + if (policy->match_src_port) { if (sport != policy->src_port) continue; score += 0xFFFF; } diff --git a/bpf/progs/dscpPolicy.h b/bpf/progs/dscpPolicy.h index ea84655588..dc431a768e 100644 --- a/bpf/progs/dscpPolicy.h +++ b/bpf/progs/dscpPolicy.h @@ -17,11 +17,6 @@ #define CACHE_MAP_SIZE 1024 #define MAX_POLICIES 16 -#define SRC_IP_MASK_FLAG 1 -#define DST_IP_MASK_FLAG 2 -#define SRC_PORT_MASK_FLAG 4 -#define PROTO_MASK_FLAG 8 - #define STRUCT_SIZE(name, size) _Static_assert(sizeof(name) == (size), "Incorrect struct size.") // Retrieve the first (ie. high) 64 bits of an IPv6 address (in network order) @@ -46,10 +41,12 @@ typedef struct { uint16_t dst_port_end; uint8_t proto; int8_t dscp_val; // -1 none, or 0..63 DSCP value - uint8_t present_fields; - uint8_t pad[3]; + bool match_src_ip; + bool match_dst_ip; + bool match_src_port; + bool match_proto; } DscpPolicy; -STRUCT_SIZE(DscpPolicy, 2 * 16 + 4 + 3 * 2 + 3 * 1 + 3); // 48 +STRUCT_SIZE(DscpPolicy, 2 * 16 + 4 + 3 * 2 + 6 * 1); // 48 typedef struct { struct in6_addr src_ip; @@ -61,4 +58,4 @@ typedef struct { int8_t dscp_val; // -1 none, or 0..63 DSCP value uint8_t pad[2]; } RuleEntry; -STRUCT_SIZE(RuleEntry, 2 * 16 + 1 * 4 + 2 * 2 + 2 * 1 + 2); // 44 +STRUCT_SIZE(RuleEntry, 2 * 16 + 4 + 2 * 2 + 4 * 1); // 44 diff --git a/service/src/com/android/server/connectivity/DscpPolicyValue.java b/service/src/com/android/server/connectivity/DscpPolicyValue.java index 7b11eda23a..7162a4a355 100644 --- a/service/src/com/android/server/connectivity/DscpPolicyValue.java +++ b/service/src/com/android/server/connectivity/DscpPolicyValue.java @@ -55,13 +55,17 @@ public class DscpPolicyValue extends Struct { @Field(order = 7, type = Type.S8) public final byte dscp; - @Field(order = 8, type = Type.U8, padding = 3) - public final short mask; + @Field(order = 8, type = Type.Bool) + public final boolean match_src_ip; - private static final int SRC_IP_MASK = 0x1; - private static final int DST_IP_MASK = 0x02; - private static final int SRC_PORT_MASK = 0x4; - private static final int PROTO_MASK = 0x8; + @Field(order = 9, type = Type.Bool) + public final boolean match_dst_ip; + + @Field(order = 10, type = Type.Bool) + public final boolean match_src_port; + + @Field(order = 11, type = Type.Bool) + public final boolean match_proto; private boolean ipEmpty(final byte[] ip) { for (int i = 0; i < ip.length; i++) { @@ -98,24 +102,6 @@ public class DscpPolicyValue extends Struct { private static final byte[] EMPTY_ADDRESS_FIELD = InetAddress.parseNumericAddress("::").getAddress(); - private short makeMask(final byte[] src46, final byte[] dst46, final int srcPort, - final int dstPortStart, final short proto, final byte dscp) { - short mask = 0; - if (src46 != EMPTY_ADDRESS_FIELD) { - mask |= SRC_IP_MASK; - } - if (dst46 != EMPTY_ADDRESS_FIELD) { - mask |= DST_IP_MASK; - } - if (srcPort != -1) { - mask |= SRC_PORT_MASK; - } - if (proto != -1) { - mask |= PROTO_MASK; - } - return mask; - } - private DscpPolicyValue(final InetAddress src46, final InetAddress dst46, final int ifIndex, final int srcPort, final int dstPortStart, final int dstPortEnd, final short proto, final byte dscp) { @@ -131,9 +117,10 @@ public class DscpPolicyValue extends Struct { this.proto = proto != -1 ? proto : 0; this.dscp = dscp; - // Use member variables for IP since byte[] is needed and api variables for everything else - // so -1 is passed into mask if parameter is not present. - this.mask = makeMask(this.src46, this.dst46, srcPort, dstPortStart, proto, dscp); + this.match_src_ip = (this.src46 != EMPTY_ADDRESS_FIELD); + this.match_dst_ip = (this.dst46 != EMPTY_ADDRESS_FIELD); + this.match_src_port = (srcPort != -1); + this.match_proto = (proto != -1); } public DscpPolicyValue(final InetAddress src46, final InetAddress dst46, final int ifIndex, From 020eaf4dd5c6eb80950b079e3c0919dc461d718f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 26 Aug 2024 17:15:47 -0700 Subject: [PATCH 3/5] NetBpfLoad: remove spurious header includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (particularly problematic on host builds where host headers are ancient...) Bug: 205608149 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: Iae64b900464e7584ad657f18d8101993fd8c8e33 --- bpf/loader/NetBpfLoad.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 22f12d150b..99498bc07e 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -17,7 +17,6 @@ #define LOG_TAG "NetBpfLoad" #include -#include #include #include #include @@ -26,8 +25,6 @@ #include #include #include -#include -#include #include #include #include From 52be6a822d41731833442f578d5f0d2d23e036ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 26 Aug 2024 17:18:39 -0700 Subject: [PATCH 4/5] NetBpfLoad: sort BPF_PROG_LOAD bpf_attr req field order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some compilers warn/error and fail to build if it is not in proper order. (specifically: triggers during build for host) Bug: 205608149 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I84ddec9a2443939616fd78f522e8620680e1289e --- bpf/loader/NetBpfLoad.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 99498bc07e..80e1dc6e34 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -990,13 +990,13 @@ static int loadCodeSections(const char* elfPath, vector& cs, const union bpf_attr req = { .prog_type = cs[i].type, - .kern_version = kvers, - .license = ptr_to_u64(license.c_str()), - .insns = ptr_to_u64(cs[i].data.data()), .insn_cnt = static_cast<__u32>(cs[i].data.size() / sizeof(struct bpf_insn)), + .insns = ptr_to_u64(cs[i].data.data()), + .license = ptr_to_u64(license.c_str()), .log_level = 1, - .log_buf = ptr_to_u64(log_buf.data()), .log_size = static_cast<__u32>(log_buf.size()), + .log_buf = ptr_to_u64(log_buf.data()), + .kern_version = kvers, .expected_attach_type = cs[i].attach_type, }; if (isAtLeastKernelVersion(4, 15, 0)) From 78fa86107b27045a76d5e7198edf8cab27823b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 26 Aug 2024 17:22:25 -0700 Subject: [PATCH 5/5] NetBpfLoad: introduce APEXROOT/BPFROOT defines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows changing the location of bpf programs easily when building for the host to run on UML as a test. Bug: 205608149 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I24816b6660581e74f277df39495a43027b77d2cf --- bpf/loader/NetBpfLoad.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 80e1dc6e34..59712b1e11 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -1152,33 +1152,35 @@ static bool exists(const char* const path) { abort(); // can only hit this if permissions (likely selinux) are screwed up } +#define APEXROOT "/apex/com.android.tethering" +#define BPFROOT APEXROOT "/etc/bpf" const Location locations[] = { // S+ Tethering mainline module (network_stack): tether offload { - .dir = "/apex/com.android.tethering/etc/bpf/", + .dir = BPFROOT "/", .prefix = "tethering/", }, // T+ Tethering mainline module (shared with netd & system server) // netutils_wrapper (for iptables xt_bpf) has access to programs { - .dir = "/apex/com.android.tethering/etc/bpf/netd_shared/", + .dir = BPFROOT "/netd_shared/", .prefix = "netd_shared/", }, // T+ Tethering mainline module (shared with netd & system server) // netutils_wrapper has no access, netd has read only access { - .dir = "/apex/com.android.tethering/etc/bpf/netd_readonly/", + .dir = BPFROOT "/netd_readonly/", .prefix = "netd_readonly/", }, // T+ Tethering mainline module (shared with system server) { - .dir = "/apex/com.android.tethering/etc/bpf/net_shared/", + .dir = BPFROOT "/net_shared/", .prefix = "net_shared/", }, // T+ Tethering mainline module (not shared, just network_stack) { - .dir = "/apex/com.android.tethering/etc/bpf/net_private/", + .dir = BPFROOT "/net_private/", .prefix = "net_private/", }, };