-
Notifications
You must be signed in to change notification settings - Fork 427
aya-obj: expand bpf_map_def to 36 bytes for tc maps #1497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1009,6 +1009,9 @@ pub enum ParseError { | |
| /// No BTF parsed for object | ||
| #[error("no BTF parsed for object")] | ||
| NoBTF, | ||
|
|
||
| #[error("map `{name}` uses unsupported legacy pinning")] | ||
| UnsupportedLegacyPinning { name: String }, | ||
| } | ||
|
|
||
| /// Invalid bindings to the bpf type from the parsed/received value. | ||
|
|
@@ -1251,16 +1254,23 @@ fn parse_map_def(name: &str, data: &[u8]) -> Result<bpf_map_def, ParseError> { | |
| }); | ||
| } | ||
|
|
||
| if data.len() < size_of::<bpf_map_def>() { | ||
| let map_def = if data.len() < size_of::<bpf_map_def>() { | ||
| let mut map_def = bpf_map_def::default(); | ||
| unsafe { | ||
| let map_def_ptr = from_raw_parts_mut(ptr::from_mut(&mut map_def).cast(), data.len()); | ||
| map_def_ptr.copy_from_slice(data); | ||
| } | ||
| Ok(map_def) | ||
| map_def | ||
| } else { | ||
| Ok(unsafe { ptr::read_unaligned(data.as_ptr().cast()) }) | ||
| unsafe { ptr::read_unaligned(data.as_ptr().cast()) } | ||
| }; | ||
|
|
||
| if map_def.pinning != PinningType::None { | ||
| return Err(ParseError::UnsupportedLegacyPinning { | ||
| name: name.to_owned(), | ||
| }); | ||
| } | ||
| Ok(map_def) | ||
| } | ||
|
|
||
| fn parse_btf_map_def(btf: &Btf, info: &DataSecEntry) -> Result<(String, BtfMapDef), BtfError> { | ||
|
|
@@ -1367,6 +1377,8 @@ pub const fn parse_map_info(info: bpf_map_info, pinned: PinningType) -> Map { | |
| map_flags: info.map_flags, | ||
| pinning: pinned, | ||
| id: info.id, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }, | ||
| section_index: 0, | ||
| symbol_index: None, | ||
|
|
@@ -1566,6 +1578,8 @@ mod tests { | |
| map_flags: 5, | ||
| id: 0, | ||
| pinning: PinningType::None, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }; | ||
|
|
||
| assert_eq!( | ||
|
|
@@ -1574,6 +1588,25 @@ mod tests { | |
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_map_def_unsupported_pinning() { | ||
| let def = bpf_map_def { | ||
| map_type: 1, | ||
| key_size: 2, | ||
| value_size: 3, | ||
| max_entries: 4, | ||
| map_flags: 5, | ||
| id: 0, | ||
| pinning: PinningType::ByName, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }; | ||
| assert_matches!( | ||
| parse_map_def("foo", bytes_of(&def)), | ||
| Err(ParseError::UnsupportedLegacyPinning { .. }) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_map_def() { | ||
| let def = bpf_map_def { | ||
|
|
@@ -1583,7 +1616,9 @@ mod tests { | |
| max_entries: 4, | ||
| map_flags: 5, | ||
| id: 6, | ||
| pinning: PinningType::ByName, | ||
| pinning: PinningType::None, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }; | ||
|
|
||
| assert_eq!(parse_map_def("foo", bytes_of(&def)).unwrap(), def); | ||
|
|
@@ -1598,7 +1633,9 @@ mod tests { | |
| max_entries: 4, | ||
| map_flags: 5, | ||
| id: 6, | ||
| pinning: PinningType::ByName, | ||
| pinning: PinningType::None, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }; | ||
| let mut buf = [0u8; 128]; | ||
| unsafe { ptr::write_unaligned(buf.as_mut_ptr().cast(), def) } | ||
|
|
@@ -1630,6 +1667,8 @@ mod tests { | |
| map_flags: 0, | ||
| id: 0, | ||
| pinning: PinningType::None, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }, | ||
| data, | ||
| }) if data == map_data && value_size == map_data.len() as u32 | ||
|
|
@@ -1809,17 +1848,59 @@ mod tests { | |
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_section_multiple_maps() { | ||
| fn test_parse_section_multiple_maps_v1_legacy() { | ||
| let mut obj = fake_obj(); | ||
| fake_sym(&mut obj, 0, 0, "foo", 28); | ||
| fake_sym(&mut obj, 0, 28, "bar", 28); | ||
| fake_sym(&mut obj, 0, 60, "baz", 28); | ||
| let def = &bpf_map_def { | ||
| map_type: 1, | ||
| key_size: 2, | ||
| value_size: 3, | ||
| max_entries: 4, | ||
| map_flags: 5, | ||
| ..Default::default() | ||
| }; | ||
| let map_data = bytes_of(def)[..28].to_vec(); | ||
| let mut buf = vec![]; | ||
| buf.extend(&map_data); | ||
| buf.extend(&map_data); | ||
| // throw in some padding | ||
| buf.extend([0, 0, 0, 0]); | ||
| buf.extend(&map_data); | ||
| assert_matches!( | ||
| obj.parse_section(fake_section( | ||
| EbpfSectionKind::Maps, | ||
| "maps", | ||
| buf.as_slice(), | ||
| None | ||
| )), | ||
| Ok(()) | ||
| ); | ||
| assert!(obj.maps.contains_key("foo")); | ||
| assert!(obj.maps.contains_key("bar")); | ||
| assert!(obj.maps.contains_key("baz")); | ||
| for map in obj.maps.values() { | ||
| assert_matches!(map, Map::Legacy(m) => { | ||
| assert_eq!(&m.def, def); | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_section_multiple_maps_v2_legacy_tc() { | ||
| let mut obj = fake_obj(); | ||
| fake_sym(&mut obj, 0, 0, "foo", size_of::<bpf_map_def>() as u64); | ||
| fake_sym(&mut obj, 0, 28, "bar", size_of::<bpf_map_def>() as u64); | ||
| fake_sym(&mut obj, 0, 60, "baz", size_of::<bpf_map_def>() as u64); | ||
| fake_sym(&mut obj, 0, 36, "bar", size_of::<bpf_map_def>() as u64); | ||
| fake_sym(&mut obj, 0, 76, "baz", size_of::<bpf_map_def>() as u64); | ||
|
Comment on lines
1893
to
+1895
|
||
| let def = &bpf_map_def { | ||
| map_type: 1, | ||
| key_size: 2, | ||
| value_size: 3, | ||
| max_entries: 4, | ||
| map_flags: 5, | ||
| inner_id: 6, | ||
| inner_idx: 7, | ||
| ..Default::default() | ||
| }; | ||
| let map_data = bytes_of(def).to_vec(); | ||
|
|
@@ -2628,6 +2709,8 @@ mod tests { | |
| map_flags: BPF_F_RDONLY_PROG, | ||
| id: 1, | ||
| pinning: PinningType::None, | ||
| inner_id: 0, | ||
| inner_idx: 0, | ||
| }, | ||
| section_index: 1, | ||
| section_kind: EbpfSectionKind::Rodata, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // clang-format off | ||
| #include <vmlinux.h> | ||
| #include <bpf/bpf_helpers.h> | ||
| // clang-format on | ||
|
|
||
| struct bpf_elf_map { | ||
| __u32 type; | ||
| __u32 size_key; | ||
| __u32 size_value; | ||
| __u32 max_elem; | ||
| __u32 flags; | ||
| __u32 id; | ||
| __u32 pinning; | ||
| __u32 inner_id; | ||
| __u32 inner_idx; | ||
| }; | ||
|
|
||
| struct bpf_elf_map SEC("maps") tc_map_1 = { | ||
| .type = BPF_MAP_TYPE_ARRAY, | ||
| .size_key = sizeof(__u32), | ||
| .size_value = sizeof(__u32), | ||
| .max_elem = 1, | ||
| .id = 1, | ||
| .inner_id = 1, | ||
| .inner_idx = 1, | ||
| }; | ||
|
|
||
| struct bpf_elf_map SEC("maps") tc_map_2 = { | ||
| .type = BPF_MAP_TYPE_ARRAY, | ||
| .size_key = sizeof(__u32), | ||
| .size_value = sizeof(__u32), | ||
| .max_elem = 2, | ||
| .id = 2, | ||
| .inner_id = 2, | ||
| .inner_idx = 2, | ||
| }; | ||
|
|
||
| SEC("classifier") | ||
| int tc_pass(void *ctx) { | ||
| __u32 key = 0; | ||
| __u32 val = 42; | ||
|
|
||
| bpf_map_update_elem(&tc_map_1, &key, &val, BPF_ANY); | ||
| bpf_map_update_elem(&tc_map_2, &key, &val, BPF_ANY); | ||
| return 0; | ||
| } | ||
|
|
||
| char _license[] SEC("license") = "GPL"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -675,3 +675,14 @@ fn pin_lifecycle_uprobe() { | |
| // Make sure the function isn't optimized out. | ||
| uprobe_function(); | ||
| } | ||
|
|
||
| #[test_log::test] | ||
| fn legacy_36_byte_maps() { | ||
| let mut bpf = Ebpf::load(crate::TC_LEGACY_MAP).unwrap(); | ||
|
|
||
| let map_1: Array<_, u32> = bpf.take_map("tc_map_1").unwrap().try_into().unwrap(); | ||
| let map_2: Array<_, u32> = bpf.take_map("tc_map_2").unwrap().try_into().unwrap(); | ||
|
|
||
| assert_eq!(map_1.len(), 1); | ||
| assert_eq!(map_2.len(), 2); | ||
| } | ||
|
Comment on lines
+679
to
+688
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is being tested here? reading the change description and this test name i am not understanding what i should be checking here.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. each map has a distinct max_elem (1 and 2) to verify both are parsed correctly from the 36-byte struct. the parser is backwards compatible, so it also handles the 28-byte variant without needing separate test cases |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_parse_section_multiple_maps_v1_legacyuses several magic numbers (28,60) for legacy map-def sizes/offsets. Since the intent is to validate the legacy 28-byte format, consider introducing a named constant (e.g., legacy map-def size and padding) and computing the offsets from it to make the test intent clearer and reduce the chance of accidental inconsistency.