aya: Implement .kconfig support #1017
Conversation
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
mergify
bot
added
aya
davibe
force-pushed
the
feature/kconfig-support
branch
4 times, most recently
from
94aae7c to
da75812
Compare
|
@davibe, this pull request is now in conflict and requires a rebase. |
davibe
force-pushed
the
feature/kconfig-support
branch
2 times, most recently
from
3dd4a80 to
00d6b57
Compare
|
Hey @alessandrod, this pull request changes the Aya Public API and requires your review. |
davibe
force-pushed
the
feature/kconfig-support
branch
from
6cba89d to
fbb0930
Compare
davibe
force-pushed
the
feature/kconfig-support
branch
from
4b1542f to
a1fb970
Compare
|
|
davibe
force-pushed
the
feature/kconfig-support
branch
7 times, most recently
from
1d6718b to
c60557e
Compare
davibe
force-pushed
the
feature/kconfig-support
branch
2 times, most recently
from
9881292 to
f3d8b7d
Compare
davibe
force-pushed
the
feature/kconfig-support
branch
4 times, most recently
from
8859344 to
eb618c2
Compare
davibe
force-pushed
the
feature/kconfig-support
branch
4 times, most recently
from
b66fd84 to
67d8937
Compare
There was a problem hiding this comment.
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 👍
aya-obj/src/btf/btf.rs
Outdated
| 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(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Just a nit but I would squash that to the first commit (the same apply for next commit)
davibe
force-pushed
the
feature/kconfig-support
branch
2 times, most recently
from
913f6f3 to
12b3727
Compare
|
|
| return Ok(None); | ||
| } | ||
|
|
||
| let mut kconfig_map_index = 0; |
There was a problem hiding this comment.
What is going on here? This is overridden at line 856?
| &self, | ||
| target_var_name: &str, | ||
| ) -> Result<(String, Var), BtfError> { | ||
| for t in &self.types.types { |
There was a problem hiding this comment.
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?
| // clang-format on | ||
|
|
||
| // CONFIG_BPF=y => 1 | ||
| extern unsigned int CONFIG_BPF __kconfig; |
There was a problem hiding this comment.
We should test char, short, int and long here, to test different alignments
| @@ -109,6 +114,133 @@ pub fn features() -> &'static Features { | |||
| &FEATURES | |||
| } | |||
|
|
|||
| lazy_static! { | |||
| static ref KCONFIG_DEFINITION: HashMap<String, Vec<u8>> = compute_kconfig_definition(&FEATURES); | |||
There was a problem hiding this comment.
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.
|
|
Implement support for external data symbols (kconfig) following libbpf logic. Co-authored-by: Mary <[email protected]>
davibe
force-pushed
the
feature/kconfig-support
branch
from
12b3727 to
370e4ff
Compare
|
@davibe, this pull request is now in conflict and requires a rebase. |
davibe
force-pushed
the
feature/kconfig-support
branch
from
370e4ff to
8179853
Compare
|
@davibe, this pull request is now in conflict and requires a rebase. |
Implement support for external data symbols (kconfig) following libbpf
logic.
This change is