aya: Support loading programs with ksyms

[altugbozkurt07]

Hi,

this commit covers the first iteration of supporting programs with ksyms variables in aya. I tried to replicate the functionality based on this commit. So to wrap it up;

BTF Parsing and Collection (aya-obj/src/obj.rs):

Added collect_ksyms_from_btf() to parse .ksyms DATASEC from BTF
Implemented ExternCollection to store extern symbol metadata
Created dummy variable handling for function externs in DATASEC
Added BTF fixup logic via fixup_ksyms_datasec() to make externs kernel-acceptable

Extern Type System (aya-obj/src/extern_types.rs):

Implemented ExternDesc to track extern functions and variables
Added resolve_extern_ksyms() for resolving externs against kernel BTF
Created separate resolution paths for functions (resolve_extern_function_internal) and variables (resolve_extern_variable_internal)
Added type compatibility checking using types_are_compatible()
For variables, if they cant be resolved against target kernel btf, resolution through kallsyms is implemented as a fallback

Relocation Support (aya-obj/src/relocation.rs):

Added relocate_externs() to patch LD_IMM64 and CALL instructions with resolved kernel BTF IDs
Implemented instruction patching for both function calls and variable references.

Integration (aya/src/bpf.rs):

Integrated extern resolution into the EbpfLoader flow
Added kernel BTF loading and extern resolution before program loading

What we are missing:

1- We need more integration tests to cover different use-cases to make sure everything is properly implemented.
Some of the tests that needs to be covered are:

• Weak symbols tests
• Extern variable tests
• Error handlings tests(non-existent extern symbol, function with wrong signature, etc)
• BTF fixup tests

2- We are missing externs in kernel modules, this is something that is supported by libbpf, at this point it should not be that hard to implement it but before that i would like to first make sure the changes i introduce to aya aligns with the aya infra structure in terms of API desing.

3- Kconfigs are also missing, but again once this iteration goes through, it would be easier to extend this to cover Kconfigs as well.

I would really appreciate your feedback on this and let me know if i am missing anything else, or my implementation is not properly covering the described use-cases.

---

This change is 

[netlify]

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	afe0356
🔍 Latest deploy log	https://app.netlify.com/projects/aya-rs-docs/deploys/69eaa57783d3b20008a600a7
😎 Deploy Preview	https://deploy-preview-1372--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 project configuration.

[vadorovsky]

Thanks for working on this and kudos for making it work!

The major issue causing the CI failures is usage of std which needs to be guarded.

I still need to play with the code, but I left some nitpicks about dosctrings and comments.

Reviewable status: 0 of 11 files reviewed, 25 unresolved discussions

---

aya-obj/src/lib.rs line 79 at r1 (raw file):

pub mod btf;
/// ksym implementation

Let's remove this. If you want to have a top-level documentation of the module, add it in the top of the module file with !//.

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

use std::{

This is the reason why CI is failing. The aya-obj crate supports no_std, so everything requiring std needs to be guarded with #[cfg(feature = "std")]. See the other modules

I still need to take a deeper look, but given that this whole ksym detection/sanitization mechanism clearly requires allocations and string operations, we could probably just guard the whole module with #[cfg(feature = "std")].

Another option would be trying to make it no_std, but I don't think it's possible.

---

aya-obj/src/extern_types.rs line 58 at r1 (raw file):

            }

            // Dispatch to appropriate resolver

I would remove all the comments in this function except that one.

---

aya-obj/src/extern_types.rs line 99 at r1 (raw file):

    fn resolve_kallsyms(&mut self) -> std::result::Result<(), KsymResolveError> {
        use std::{

Move the imports up.

---

aya-obj/src/extern_types.rs line 122 at r1 (raw file):

        let file =
            File::open("/proc/kallsyms").map_err(|e| KsymResolveError::KallsymsReadError {
                error: e.to_string(),

We shouldn't include other errors with string.thiserror can wrap other error types inside your own one.

---

aya-obj/src/extern_types.rs line 227 at r1 (raw file):

    /// Internal: Resolve a single extern variable
    /// Returns Some(btf_id) on success, None for weak externs not found

Code snippet:

/// Resolves a single exterm variable. Returns BTF ID if found, otherwise
/// returns `None`.

---

aya/src/bpf.rs line 97 at r1 (raw file):

}

pub(crate) static KERNEL_BTF: LazyLock<Option<Btf>> = LazyLock::new(|| match Btf::from_sys_fs() {

btw, I still need to play with the code myself, but I don't like having it as a global variable. If it's really difficult to pass it across functions, we could make it a part of some struct and write methods.

---

aya/src/bpf.rs line 1195 at r1 (raw file):

    /// Error resolving extern kernel symbols (functions and variables)
    #[error("kernel symbol resolution error: {0}")]
    KsymResolve(#[from] aya_obj::KsymResolveError),

Usually we are trying to keep the error variants in EbpfError very general, and treat each variant as a category of errors. Given that these two are about ksyms, I would try to pack them in one error type first (let's stay, KsymError), then include it here.

---

test/integration-test/bpf/ksysm.bpf.c line 18 at r1 (raw file):

extern void bpf_rcu_read_lock(void) __attribute__((section(".ksyms")));
extern void bpf_rcu_read_unlock(void) __attribute__((section(".ksyms")));

Would be nice to add a similar Rust eBPF program.

---

aya-obj/src/btf/btf.rs line 819 at r1 (raw file):

    /// This modifies extern functions and variables to make them acceptable to the kernel:
    /// - Functions: Change linkage to GLOBAL, fix param names, replace with dummy var in datasec
    /// - Variables: Change linkage to GLOBAL_ALLOCATED, replace type with int

Code snippet:

/// Fixes up BTF for `.ksyms` datasec entries containing extern kernel symbol and
/// makes it acceptable by the kernel:
///
/// * Changes linkage of extern functions to `GLOBAL`, fixes parameter names, injects
///   a dummy variable representing them in datasec.
/// * Changes linkage of extern variables to `GLOBAL_ALLOCATED`, replaces their type
///   with `int`.

---

aya-obj/src/btf/btf.rs line 825 at r1 (raw file):

        dummy_var_id: Option<u32>,
    ) -> Result<(), BtfError> {
        // Get dummy var info and int type ID upfront

I would try to make this comment more descriptive (for the whole let ... else block`) and remove all the others.

---

aya-obj/src/relocation.rs line 184 at r1 (raw file):

    pub fn relocate_externs(&mut self) -> Result<(), EbpfRelocationError> {
        for (name, extern_desc) in &self.externs.externs {
            eprintln!(

Use debug!, instead of eprintln!

---

aya-obj/src/relocation.rs line 246 at r1 (raw file):

fn relocate_externs<'a, I: Iterator<Item = &'a Relocation>>(
    fun: &mut Function, // ← Different first parameter (not &mut self)

Remove this comment, it brings no value. It's not even a method, so why would it have self?

---

aya-obj/src/obj.rs line 872 at r1 (raw file):

        };

        info!("Found/created int type_id: {}", int_type_id);

I would make it debug! and print it only if we create the int type.

---

aya-obj/src/obj.rs line 890 at r1 (raw file):

        // After creating dummy_var
        info!("Created dummy_var type_id: {}", dummy_var_id);

debug!

---

aya-obj/src/obj.rs line 898 at r1 (raw file):

    }

    /// Collect extern kernel symbols from BTF DATASEC entries

Function/method documentation should be written using the 3rd person, so starting from "Collects". And if you use full sentences, finish them with ..

Let's also be consistent with capitalizing (or not capitalizing) names. We either write "DATASEC" or "datasec" everywhere. I'm leaning towards the latter, since it's just a convenient shortened form of "data section", not really a proper name.

Code snippet:

/// Collects extern kernel symbols from BTF datasec entries.

---

aya-obj/src/obj.rs line 900 at r1 (raw file):

    /// Collect extern kernel symbols from BTF DATASEC entries
    fn collect_ksyms_from_btf(&mut self) -> Result<(), ParseError> {
        // Find .ksyms DATASEC

Remove all these comments, the code is self-explanatory enough.

---

aya-obj/src/obj.rs line 927 at r1 (raw file):

    }

    /// Find .ksyms DATASEC in BTF, returns (id, datasec) if found

Code snippet:

/// Searches for the `.ksyms` datasec in BTF, returns it if found.

---

aya-obj/src/obj.rs line 945 at r1 (raw file):

    }

    /// Check if datasec contains any functions

Code snippet:

/// Checks if datasec dontains any functions.

---

aya-obj/src/obj.rs line 954 at r1 (raw file):

    }

    /// Collect extern descriptors from datasec entries

Code snippet:

/// Collects extern descriptors from datasec entries.

---

aya-obj/src/obj.rs line 961 at r1 (raw file):

        for entry in &datasec.entries {
            let Some(extern_desc) = self.process_datasec_entry(btf, entry)? else {
                continue; // Skip non-extern entries

Remove this comment.

---

aya-obj/src/obj.rs line 970 at r1 (raw file):

    }

    /// Process a single datasec entry, returning ExternDesc if it's an extern

Aside from what I mentioned earlier, try to use ``` for type names. If the types are inside the same crate, you can even link them like:

Code snippet:

/// Processes a single datasec entry, returns [`ExternDesc`] if it's an extern.

---

aya-obj/src/obj.rs line 978 at r1 (raw file):

        let btf_type = btf.type_by_id(entry.btf_type)?;

        // Extract extern info based on type

Remove these overly explicit comments.

---

aya-obj/src/obj.rs line 1017 at r1 (raw file):

    }

    /// Apply BTF fixups for .ksyms DATASEC

Code snippet:

/// Applies BTF fixups for `.ksyms` datasec.

---

aya-obj/src/obj.rs line 1026 at r1 (raw file):

    }

    /// Helper to find symbol by name

Don't start function/method documentation with "Helper", use the 3rd person and explain what it does.

Code snippet:

/// Return a symbol with the given `name` from the symbol table.

[vadorovsky]

Some more comments about code organization, the kind of comments you were asking for. 😄

[altugbozkurt07]

Reviewable status: 0 of 15 files reviewed, 36 unresolved discussions (waiting on @vadorovsky)

---

aya/src/bpf.rs line 97 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

btw, I still need to play with the code myself, but I don't like having it as a global variable. If it's really difficult to pass it across functions, we could make it a part of some struct and write methods.

removed, now we are using already parsed kernel btf.

---

aya/src/bpf.rs line 1195 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Usually we are trying to keep the error variants in EbpfError very general, and treat each variant as a category of errors. Given that these two are about ksyms, I would try to pack them in one error type first (let's stay, KsymError), then include it here.

Done.

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

This is the reason why CI is failing. The aya-obj crate supports no_std, so everything requiring std needs to be guarded with #[cfg(feature = "std")]. See the other modules

I still need to take a deeper look, but given that this whole ksym detection/sanitization mechanism clearly requires allocations and string operations, we could probably just guard the whole module with #[cfg(feature = "std")].

Another option would be trying to make it no_std, but I don't think it's possible.

Done.

---

aya-obj/src/extern_types.rs line 58 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

I would remove all the comments in this function except that one.

Done.

---

aya-obj/src/extern_types.rs line 99 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Move the imports up.

would not it be better to be stayed in the function scope since this is only enabled with std feature

---

aya-obj/src/extern_types.rs line 122 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

We shouldn't include other errors with string.thiserror can wrap other error types inside your own one.

Done.

---

aya-obj/src/lib.rs line 79 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Let's remove this. If you want to have a top-level documentation of the module, add it in the top of the module file with !//.

Done.

---

aya-obj/src/obj.rs line 872 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

I would make it debug! and print it only if we create the int type.

Done.

---

aya-obj/src/obj.rs line 890 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

debug!

Done.

---

aya-obj/src/obj.rs line 898 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Function/method documentation should be written using the 3rd person, so starting from "Collects". And if you use full sentences, finish them with ..

Let's also be consistent with capitalizing (or not capitalizing) names. We either write "DATASEC" or "datasec" everywhere. I'm leaning towards the latter, since it's just a convenient shortened form of "data section", not really a proper name.

Done.

---

aya-obj/src/obj.rs line 900 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Remove all these comments, the code is self-explanatory enough.

Done.

---

aya-obj/src/obj.rs line 961 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Remove this comment.

Done.

---

aya-obj/src/obj.rs line 970 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Aside from what I mentioned earlier, try to use ``` for type names. If the types are inside the same crate, you can even link them like:

Done.

---

aya-obj/src/obj.rs line 978 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Remove these overly explicit comments.

Done.

---

aya-obj/src/obj.rs line 1026 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Don't start function/method documentation with "Helper", use the 3rd person and explain what it does.

Done.

---

aya-obj/src/relocation.rs line 184 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Use debug!, instead of eprintln!

Done.

---

aya-obj/src/relocation.rs line 246 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Remove this comment, it brings no value. It's not even a method, so why would it have self?

Done.

---

aya-obj/src/btf/btf.rs line 825 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

I would try to make this comment more descriptive (for the whole let ... else block`) and remove all the others.

Done.

---

test/integration-test/bpf/ksysm.bpf.c line 18 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

Would be nice to add a similar Rust eBPF program.

I added a similar rust program, but extern definitions debug information is not emitted by rustc so that the proper btf is generated by llvm. We need a thread in discord regarding this.

---

aya-obj/src/extern_types.rs line 227 at r1 (raw file):

    /// Internal: Resolve a single extern variable
    /// Returns Some(btf_id) on success, None for weak externs not found

Done.

---

aya-obj/src/btf/btf.rs line 819 at r1 (raw file):

    /// This modifies extern functions and variables to make them acceptable to the kernel:
    /// - Functions: Change linkage to GLOBAL, fix param names, replace with dummy var in datasec
    /// - Variables: Change linkage to GLOBAL_ALLOCATED, replace type with int

Done.

---

aya-obj/src/obj.rs line 927 at r1 (raw file):

    }

    /// Find .ksyms DATASEC in BTF, returns (id, datasec) if found

Done.

---

aya-obj/src/obj.rs line 945 at r1 (raw file):

    }

    /// Check if datasec contains any functions

Done.

---

aya-obj/src/obj.rs line 954 at r1 (raw file):

    }

    /// Collect extern descriptors from datasec entries

Done.

---

aya-obj/src/obj.rs line 1017 at r1 (raw file):

    }

    /// Apply BTF fixups for .ksyms DATASEC

Done.

[vadorovsky]

Please add a new line.

[vadorovsky]

not a full review yet, just skimmed through like 1/5 of code

[banditopazzo]

hey @altugbozkurt07, do you have any plans to keep working on this PR?

[altugbozkurt07]

@banditopazzo yep, i have some time to work on this during this weekend. But even if i finalize my work, it needs to be reviewed and approved by the maintainers. So do you have a use case dependent on this feature ?

[banditopazzo]

ok good.

yes I have something dependent on this and right now I am using some workarounds

[altugbozkurt07]

Hello @vadorovsky, a quick update on the current state of this PR after the latest round of refactoring and test coverage work.

The implementation now models ksym handling as two explicit phases:

1. resolving extern ksym entries parsed from the ELF/BTF object
2. applying the corresponding extern relocations to program bytecode

That split is also reflected in the object-level API exposed by aya-obj, which I kept intentionally small:

• Object::resolve_externs()
• Object::relocate_externs()
• KsymsError

The main goal of the refactor was to make the API boundary between aya-obj and aya clearer. aya-obj now exposes the object-level operations that seem generally useful to custom loaders and object-processing tools, while keeping the internals of extern collection, .ksyms DATASEC fixup, placeholder handling, and instruction patching private.

In terms of behavior, the current implementation:

• collects .ksyms extern entries from object BTF
• fixes up .ksyms BTF/DATASEC state so the resulting object BTF remains acceptable to the kernel
• resolves typed extern vars through kernel BTF
• resolves typed kfuncs through kernel BTF
• resolves typeless ksym vars through /proc/kallsyms
• relocates typed vars as BPF_PSEUDO_BTF_ID
• relocates typeless vars as absolute addresses
• relocates typed kfunc calls as BPF_PSEUDO_KFUNC_CALL
• null-patches unresolved weak var references
• poisons unresolved weak kfunc calls so they can be safely guarded
• defensively rejects unresolved strong relocations

I also expanded and reorganized the tests around the main success and failure paths. The integration coverage now includes:

• strong typed ksyms
• weak typed ksyms
• weak typed kfuncs
• typeless kallsyms-backed ksyms
• missing strong typed var load failures
• missing strong kfunc load failures
• missing strong typeless symbol load failures

The main remaining gap relative to libbpf is resolving typed ksyms/kfuncs from kernel module BTFs, not just vmlinux BTF. That is the next piece I plan to add.

At this point, I’d especially appreciate feedback on whether this split and API shape look reasonable for Aya’s layering, in addition to any implementation concerns.

[vadorovsky]

Thanks a lot, it looks good to me (apart from one tiny nit)! I will let @alessandrod and @tamird take a look if they want.

[vadorovsky]

super nit: let's move it to the mod tests block

[altugbozkurt07]

done

[tamird]

@vadorovsky FYI it would be helpful to mark your comments resolved if you're happy with the resolution - otherwise it quite clutters the review for other reviewers. Haven't completed my review yet, would appreciate you doing a cleanup pass before I do.

@tamird reviewed 5 files and all commit messages, and made 7 comments.
Reviewable status: 5 of 24 files reviewed, 69 unresolved discussions (waiting on altugbozkurt07 and vadorovsky).

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, altugbozkurt07 wrote…

Done.

@vadorovsky do you remember why no_std is needed for aya_obj? I found #473 but it isn't completely clear to me from that issue why deps on libstd are not ok but deps on liballoc are.

---

-- commits line 2 at r9:
could you add a bit of information here? this is what will survive when this is merged.

also if the PR description is meant to be an inventory of work still to be done, could you update it?

---

aya-obj/src/extern_types.rs line 26 at r9 (raw file):

        }

        #[cfg(feature = "std")]

this appears 8 times in this patch not including test code. can we centralize this a bit?

---

aya-obj/src/extern_types.rs line 92 at r9 (raw file):

    #[cfg(feature = "std")]
    pub(crate) fn resolve_typeless_externs(&mut self) -> Result<(), KsymsError> {
        use std::fs::File;

odd use, there's only one reference to File in this function

---

aya-obj/src/extern_types.rs line 108 at r9 (raw file):

    /// Sets `ksym_addr = Some(0)` for all unresolved weak ksym externs.
    #[cfg(feature = "std")]

why does this function rely on std?

[qjerome]

Isn't it dangerous to have this in the middle of the function without alternative if std feature is disabled ?
Shouldn't be the whole function conditionally compiled in that case ?

[altugbozkurt07]

I don’t think the whole function should be #[cfg(feature = "std")], because resolve_externs() still does meaningful work in no_std: typed externs can still be resolved from caller-provided kernel BTF, and typed ksyms should still fail early if kernel BTF is missing.

What I think is missing today is an explicit no_std failure path for strong typeless ksyms. Right now the std-gated kallsyms pass is skipped, so those symbols only fail later during relocation. I’d prefer to make that explicit in resolve_externs() itself: keep the std path as-is, but in no_std detect unresolved strong typeless ksyms and return a dedicated error such as TypelessKsymRequiresResolver.

That feels like the right capability boundary to me: the real missing dependency is not std itself, but a resolver for typeless ksyms. In a constrained no_std environment Aya can still do the pure BTF-based work, but it should fail early and clearly once typeless strong entries require an address source that is not available. What do you guys think ?

[vadorovsky]

@altugbozkurt07 See the discussion https://reviewable.io/reviews/aya-rs/aya/1372#-OcElKu1Bx4MZ61G7mI0 - I'm starting to think that we should just remove the no_std support entirely, and I'm about to propose a PR for that. It should simplify your code a lot.

[altugbozkurt07]

thanks, that makes sense actually, i will wait on your PR.

[altugbozkurt07]

I believe my pr wont need to wait for no_std support to be merged today ?

[qjerome]

This can be probably filtered out with a call to filter() ?

[qjerome]

I think you replied in the wrong comment :)

[altugbozkurt07]

Yep, i updated the intended comment, thanks :)

[qjerome]

More idiomatic to use &[&str] or S: AsRef<str> with &[S] maybe more appropriate here

[altugbozkurt07]

done

[altugbozkurt07]

done

[qjerome]

Can be simplified with if let

 let Some(obj_btf) = self.btf.as_mut() else {
            return Ok(());
        };

[altugbozkurt07]

done

[qjerome]

I tend to prefer a functional style for this kind of things where you just need to inspect some value, but it technically does not bring anything ! Up to you to decide

       let int_type_id = maybe_int_type_id
            .inspect(|id| debug!("found 4-byte int type_id: {id}"))
            .unwrap_or_else(|| {
                let name_offset = self.add_string("int");
                let int_type_id = self.add_type(BtfType::Int(crate::btf::Int::new(
                    name_offset,
                    4,
                    crate::btf::IntEncoding::Signed,
                    0,
                )));
                debug!("created 4-byte int type_id: {int_type_id}");
                int_type_id
            });

[altugbozkurt07]

done

[vadorovsky]

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

@vadorovsky do you remember why no_std is needed for aya_obj? I found #473 but it isn't completely clear to me from that issue why deps on libstd are not ok but deps on liballoc are.

I don't. I only remember that split of aya-obj as a separate crate was motivated by a need of parsing BPF objects in some project that uses rbpf. But I don't recall anything requiring no_std.

[vadorovsky]

@vadorovsky made 2 comments and resolved 57 discussions.
Reviewable status: 5 of 24 files reviewed, 17 unresolved discussions (waiting on altugbozkurt07, qjerome, and tamird).

---

aya-obj/src/extern_types.rs line 108 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

why does this function rely on std?

It's called only by resolve_typeless_externs, there is no other caller. Without the guard, compiler would complain that it's unused.

[vadorovsky]

@vadorovsky resolved 6 discussions.
Reviewable status: 5 of 24 files reviewed, 11 unresolved discussions (waiting on altugbozkurt07, qjerome, and tamird).

[tamird]

@tamird partially reviewed 12 files and made 5 comments.
Reviewable status: 17 of 24 files reviewed, 13 unresolved discussions (waiting on altugbozkurt07, qjerome, and vadorovsky).

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

I don't. I only remember that split of aya-obj as a separate crate was motivated by a need of parsing BPF objects in some project that uses rbpf. But I don't recall anything requiring no_std.

30f1fab

"Since it should be platform-independent in principle, making it no_std like the object crate would seem reasonable."

sigh.

---

aya-obj/src/extern_types.rs line 108 at r9 (raw file):

Previously, vadorovsky (Michal R) wrote…

It's called only by resolve_typeless_externs, there is no other caller. Without the guard, compiler would complain that it's unused.

suggests perhaps it should just be inlined?

---

aya-obj/src/obj.rs line 520 at r9 (raw file):

                }
            }

nit: could revert this to reduce the diff

---

test/integration-test/src/tests/ksyms.rs line 9 at r9 (raw file):

use aya::{Btf, Ebpf, EbpfError, maps::Array, programs::BtfTracePoint, util::KernelVersion};
use aya_obj::extern_types::KsymsError;

as far as i can tell the only reason the whole extern_types module is pub is to expose this one error type. is it worth it?

[vadorovsky]

@vadorovsky made 3 comments and resolved 1 discussion.
Reviewable status: 9 of 24 files reviewed, 12 unresolved discussions (waiting on altugbozkurt07, qjerome, and tamird).

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

30f1fab

"Since it should be platform-independent in principle, making it no_std like the object crate would seem reasonable."

sigh.

The only one theoretical use-case I could see for aya-obj to be no_std is if someone wanted to implement a BPF VM in some new niche OS written in Rust, like Redox or Fuchsia. But the current no_std support is not sufficient for that, because there is still a lot of Linux-only logic that's here, including the one introduced by this PR. Currently aya-obj is not really equivalent to object, because its scope is larger than "parse the BPF binary object", it's also "perform Linux-specific shenanigans to make it loadable by Linux BPF VM".

If we ever want aya-obj to be like object, we'd need to split these two concerns into separate crates - or move the fixups entirely to aya. Theoretically I think it makes sense to do that eventually, but not now, and definitely not as a blocker for this PR.

So yeah, as for today, I would just remove the no_std support. I'm going to propose a PR.

---

aya-obj/src/extern_types.rs line 108 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

suggests perhaps it should just be inlined?

Yeah, you're right

[vadorovsky]

@altugbozkurt07 See the discussion https://reviewable.io/reviews/aya-rs/aya/1372#-OcElKu1Bx4MZ61G7mI0 - I'm starting to think that we should just remove the no_std support entirely, and I'm about to propose a PR for that. It should simplify your code a lot.

[altugbozkurt07]

@altugbozkurt07 made 10 comments and resolved 1 discussion.
Reviewable status: 9 of 24 files reviewed, 11 unresolved discussions (waiting on qjerome, tamird, and vadorovsky).

---

-- commits line 2 at r9:

Previously, tamird (Tamir Duberstein) wrote…

could you add a bit of information here? this is what will survive when this is merged.

also if the PR description is meant to be an inventory of work still to be done, could you update it?

is it okay now ?

---

aya-obj/src/extern_types.rs line 1 at r1 (raw file):

Previously, vadorovsky (Michal R) wrote…

The only one theoretical use-case I could see for aya-obj to be no_std is if someone wanted to implement a BPF VM in some new niche OS written in Rust, like Redox or Fuchsia. But the current no_std support is not sufficient for that, because there is still a lot of Linux-only logic that's here, including the one introduced by this PR. Currently aya-obj is not really equivalent to object, because its scope is larger than "parse the BPF binary object", it's also "perform Linux-specific shenanigans to make it loadable by Linux BPF VM".

If we ever want aya-obj to be like object, we'd need to split these two concerns into separate crates - or move the fixups entirely to aya. Theoretically I think it makes sense to do that eventually, but not now, and definitely not as a blocker for this PR.

So yeah, as for today, I would just remove the no_std support. I'm going to propose a PR.

so for now i believe this pr does not depend on no_std support to be upstreamed.

---

aya-obj/src/extern_types.rs line 26 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

this appears 8 times in this patch not including test code. can we centralize this a bit?

does it look better now ?

---

aya-obj/src/extern_types.rs line 92 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

odd use, there's only one reference to File in this function

it should be done now

---

aya-obj/src/extern_types.rs line 108 at r9 (raw file):

Previously, vadorovsky (Michal R) wrote…

Yeah, you're right

done

---

aya-obj/src/obj.rs line 520 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

nit: could revert this to reduce the diff

done

---

test/integration-test/src/tests/ksyms.rs line 9 at r9 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

as far as i can tell the only reason the whole extern_types module is pub is to expose this one error type. is it worth it?

i changed it to be private and only expose the error type now.

[altugbozkurt07]

I believe my pr wont need to wait for no_std support to be merged today ?

[altugbozkurt07]

done

[altugbozkurt07]

done