aya: Implement .kconfig support #1017

davibe · 2024-09-02T09:25:18Z

Implement support for external data symbols (kconfig) following libbpf
logic.

netlify · 2024-09-02T09:25:33Z

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`12b3727`
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/66f6ac09a4e0d00008460d7b
😎 Deploy Preview	https://deploy-preview-1017--aya-rs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mergify · 2024-09-03T02:49:47Z

@davibe, this pull request is now in conflict and requires a rebase.

mergify · 2024-09-03T08:04:01Z

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

mergify · 2024-09-08T13:27:15Z

⚠️ The sha of the head commit of this PR conflicts with #631. Mergify cannot evaluate rules on this PR. ⚠️

marysaka

Overall looks pretty solid, thanks for doing this 🙏

I would like the fix you did for the first commit to be squashed in it if possible.
It also seems that the commit (co-)author got lost in the way but I guess that's fine.

With all comments fixed, assume my r-b/approval 👍

marysaka · 2024-09-27T07:54:03Z

aya/Cargo.toml

+tokio = { workspace = true, features = [
+    "rt",
+    "macros",
+    "rt-multi-thread",
+    "net",
+], optional = true }
+flate2 = "1.0"


Why was that changed?

marysaka · 2024-09-27T07:57:34Z

aya-obj/src/btf/btf.rs

+            let BtfType::DataSec(d) = t else { continue; };
+            let sec_name = self.string_at(d.name_offset)?;

-                            return Ok((sec_name.into(), var.clone()));
-                        }
+            for d in &d.entries {
+                let BtfType::Var(var) = self.types.type_by_id(d.btf_type)? else {
+                    continue;
+                };
+
+                if target_var_name == self.string_at(var.name_offset)? {
+                    if var.linkage == VarLinkage::Extern {
+                        return Ok((sec_name.into(), var.clone()));
+                    } else {
+                        return Err(BtfError::InvalidExternalSymbol {
+                            symbol_name: target_var_name.into(),
+                        });
                    }


Just a nit but I would squash that to the first commit (the same apply for next commit)

aya-obj/src/btf/btf.rs

test/integration-test/bpf/kconfig.bpf.c

marysaka · 2024-09-27T08:04:38Z

aya/src/bpf.rs

-                    if raw_value.len() > 2 || raw_value.ends_with('"') {
-                        continue;
-                    }
-


Oh yeah I completely inverted that check here...
Could you fix it and keep it for sanity checks as malformed data is still possible?

Implement support for external data symbols (kconfig) following libbpf logic. Co-authored-by: Mary <mary@mary.zone>

mergify · 2024-09-27T12:59:26Z

⚠️ The sha of the head commit of this PR conflicts with #631. Mergify cannot evaluate rules on this PR. ⚠️

alessandrod · 2024-09-27T12:46:32Z

aya/src/bpf.rs

+    static ref KCONFIG_DEFINITION: HashMap<String, Vec<u8>> = compute_kconfig_definition(&FEATURES);
+}
+
+fn to_bytes(value: u64) -> [u8; 8] {


isn't this value.to_ne_bytes()?

alessandrod · 2024-09-27T13:09:42Z

aya-obj/src/btf/btf.rs

@@ -610,6 +708,14 @@ impl Btf {
                                    }
                                };
                                e.offset = *offset as u32;
+
+                                if var.linkage == VarLinkage::Extern {


Please add a comment explaining that this is needed for kconfig support

alessandrod · 2024-09-27T13:11:36Z

aya-obj/src/btf/btf.rs

+                return Ok(None);
+            }
+
+            let mut kconfig_map_index = 0;


What is going on here? This is overridden at line 856?

alessandrod · 2024-09-27T13:22:26Z

aya-obj/src/btf/btf.rs

+        &self,
+        target_var_name: &str,
+    ) -> Result<(String, Var), BtfError> {
+        for t in &self.types.types {


Perf wise this isn't great, as we iterate all the types for each kconfig symbol.
We should probably do one pass where we find the datasecs, then only scan those?

alessandrod · 2024-09-27T13:28:43Z

test/integration-test/bpf/kconfig.bpf.c

+// clang-format on
+
+// CONFIG_BPF=y => 1
+extern unsigned int CONFIG_BPF __kconfig;


We should test char, short, int and long here, to test different alignments

alessandrod · 2024-09-27T13:32:02Z

aya/src/bpf.rs

@@ -109,6 +114,133 @@ pub fn features() -> &'static Features {
    &FEATURES
 }

+lazy_static! {
+    static ref KCONFIG_DEFINITION: HashMap<String, Vec<u8>> = compute_kconfig_definition(&FEATURES);


Instead of this lazy static, we could have a KConfig type. Then we could have
EbpfLoader::with_kconfig(kconfig). This way we don't force people who don't
use kconfig to take the extra memory hit. It's a small hit, but people are using
aya in embedded systems so we shouldn't increase mem usage if we can.

mergify bot added aya This is about aya (userspace) aya-obj Relating to the aya-obj crate test A PR that improves test cases or CI labels Sep 2, 2024

davibe force-pushed the feature/kconfig-support branch 4 times, most recently from 94aae7c to da75812 Compare September 2, 2024 09:50

mergify bot added the needs-rebase label Sep 3, 2024

davibe force-pushed the feature/kconfig-support branch 2 times, most recently from 3dd4a80 to 00d6b57 Compare September 3, 2024 07:41

mergify bot removed the needs-rebase label Sep 3, 2024

mergify bot added the api/needs-review Makes an API change that needs review label Sep 3, 2024

mergify bot requested a review from alessandrod September 3, 2024 08:04

davibe closed this Sep 5, 2024

davibe force-pushed the feature/kconfig-support branch from 6cba89d to fbb0930 Compare September 5, 2024 19:35

davibe reopened this Sep 5, 2024

davibe force-pushed the feature/kconfig-support branch from 4b1542f to a1fb970 Compare September 8, 2024 13:26

davibe force-pushed the feature/kconfig-support branch 7 times, most recently from 1d6718b to c60557e Compare September 14, 2024 20:11

davibe force-pushed the feature/kconfig-support branch 2 times, most recently from 9881292 to f3d8b7d Compare September 22, 2024 11:05

davibe force-pushed the feature/kconfig-support branch 4 times, most recently from 8859344 to eb618c2 Compare September 22, 2024 13:29

davibe marked this pull request as ready for review September 22, 2024 13:33

davibe force-pushed the feature/kconfig-support branch 4 times, most recently from b66fd84 to 67d8937 Compare September 25, 2024 20:19

mergify bot mentioned this pull request Sep 25, 2024

aya: Implement .kconfig support #631

Open

marysaka reviewed Sep 27, 2024

View reviewed changes

davibe force-pushed the feature/kconfig-support branch from 67d8937 to 913f6f3 Compare September 27, 2024 12:18

davibe and others added 7 commits September 27, 2024 14:50

aya: Implement .kconfig support

b7f919d

Implement support for external data symbols (kconfig) following libbpf logic. Co-authored-by: Mary <mary@mary.zone>

Autoload kernel config in the integration vm runner

ecbfea3

Update public-api

a917512

Cleanup

8b2ef5d

Improve failure loggin when reading kernel config

41a2461

Fix kconfig symbol address

a723cfa

Fix kconfig string loading

12b3727

davibe force-pushed the feature/kconfig-support branch from 913f6f3 to 12b3727 Compare September 27, 2024 12:58

alessandrod requested changes Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aya: Implement .kconfig support #1017

aya: Implement .kconfig support #1017

davibe commented Sep 2, 2024

netlify bot commented Sep 2, 2024 •

edited

Loading

mergify bot commented Sep 3, 2024

mergify bot commented Sep 3, 2024

mergify bot commented Sep 8, 2024

marysaka left a comment

marysaka Sep 27, 2024

marysaka Sep 27, 2024

marysaka Sep 27, 2024

mergify bot commented Sep 27, 2024

alessandrod Sep 27, 2024

alessandrod Sep 27, 2024

alessandrod Sep 27, 2024

alessandrod Sep 27, 2024

alessandrod Sep 27, 2024

alessandrod Sep 27, 2024

aya: Implement .kconfig support #1017

Are you sure you want to change the base?

aya: Implement .kconfig support #1017

Conversation

davibe commented Sep 2, 2024

netlify bot commented Sep 2, 2024 • edited Loading

✅ Deploy Preview for aya-rs-docs ready!

mergify bot commented Sep 3, 2024

mergify bot commented Sep 3, 2024

mergify bot commented Sep 8, 2024

marysaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Sep 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Sep 2, 2024 •

edited

Loading