Skip to content

Commit

Permalink
aya-log: Unify IP format hints into one, repsesent it by :i token
Browse files Browse the repository at this point in the history 
Having separate format hints and tokens per IP address family is
unnecessary, since they are represented by different types and we handle
format hints for each type separately. So we can just have one format
hint.

Also, we should be consistent with the format strings grammar in
Rust[0]. The `type` token, which is mapped to formatting traits, usually
consists of one letter[1] (and optional `?` for `Debug` trait, but that
doesn't matter for us). It shouldn't consist of multiple letters. Our
`:ipv4` and `:ipv6` tokens were clearly breaking that convention, so we
should rather switch to something with one letter - hence `:i`.

[0] https://doc.rust-lang.org/std/fmt/#syntax
[1] https://doc.rust-lang.org/std/fmt/#formatting-traits
  • Loading branch information
@vadorovsky @alessandrod
vadorovsky authored and alessandrod committed May 27, 2023
1 parent 5894c4c commit 84e5e28
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 29 deletions.
6 changes: 2 additions & 4 deletions aya-log-common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,8 @@ pub enum DisplayHint {
LowerHex,
/// `:X`
UpperHex,
/// `:ipv4`
Ipv4,
/// `:ipv6`
Ipv6,
/// `:i`
Ip,
/// `:mac`
LowerMac,
/// `:MAC`
Expand Down
3 changes: 1 addition & 2 deletions aya-log-ebpf-macros/src/expand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ fn hint_to_expr(hint: DisplayHint) -> Result<Expr> {
DisplayHint::Default => parse_str("::aya_log_ebpf::macro_support::DisplayHint::Default"),
DisplayHint::LowerHex => parse_str("::aya_log_ebpf::macro_support::DisplayHint::LowerHex"),
DisplayHint::UpperHex => parse_str("::aya_log_ebpf::macro_support::DisplayHint::UpperHex"),
DisplayHint::Ipv4 => parse_str("::aya_log_ebpf::macro_support::DisplayHint::Ipv4"),
DisplayHint::Ipv6 => parse_str("::aya_log_ebpf::macro_support::DisplayHint::Ipv6"),
DisplayHint::Ip => parse_str("::aya_log_ebpf::macro_support::DisplayHint::Ip"),
DisplayHint::LowerMac => parse_str("::aya_log_ebpf::macro_support::DisplayHint::LowerMac"),
DisplayHint::UpperMac => parse_str("::aya_log_ebpf::macro_support::DisplayHint::UpperMac"),
}
Expand Down
10 changes: 3 additions & 7 deletions aya-log-parser/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ fn parse_display_hint(s: &str) -> Result<DisplayHint, String> {
Ok(match s {
"x" => DisplayHint::LowerHex,
"X" => DisplayHint::UpperHex,
"ipv4" => DisplayHint::Ipv4,
"ipv6" => DisplayHint::Ipv6,
"i" => DisplayHint::Ip,
"mac" => DisplayHint::LowerMac,
"MAC" => DisplayHint::UpperMac,
_ => return Err(format!("unknown display hint: {s:?}")),
Expand Down Expand Up @@ -146,7 +145,7 @@ mod test {
#[test]
fn test_parse() {
assert_eq!(
parse("foo {} bar {:x} test {:X} ayy {:ipv4} lmao {:ipv6} {{}} {{something}}"),
parse("foo {} bar {:x} test {:X} ayy {:i} lmao {{}} {{something}}"),
Ok(vec![
Fragment::Literal("foo ".into()),
Fragment::Parameter(Parameter {
Expand All @@ -162,12 +161,9 @@ mod test {
}),
Fragment::Literal(" ayy ".into()),
Fragment::Parameter(Parameter {
hint: DisplayHint::Ipv4
hint: DisplayHint::Ip
}),
Fragment::Literal(" lmao ".into()),
Fragment::Parameter(Parameter {
hint: DisplayHint::Ipv6
}),
Fragment::Literal(" {{}} {{something}}".into()),
])
);
Expand Down
24 changes: 9 additions & 15 deletions aya-log/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ impl Format for u32 {
Some(DisplayHint::Default) => Ok(DefaultFormatter::format(self)),
Some(DisplayHint::LowerHex) => Ok(LowerHexFormatter::format(self)),
Some(DisplayHint::UpperHex) => Ok(UpperHexFormatter::format(self)),
Some(DisplayHint::Ipv4) => Ok(Ipv4Formatter::format(*self)),
Some(DisplayHint::Ipv6) => Err(()),
Some(DisplayHint::Ip) => Ok(Ipv4Formatter::format(*self)),
Some(DisplayHint::LowerMac) => Err(()),
Some(DisplayHint::UpperMac) => Err(()),
_ => Ok(DefaultFormatter::format(self)),
Expand All @@ -273,8 +272,7 @@ impl Format for [u8; 6] {
Some(DisplayHint::Default) => Err(()),
Some(DisplayHint::LowerHex) => Err(()),
Some(DisplayHint::UpperHex) => Err(()),
Some(DisplayHint::Ipv4) => Err(()),
Some(DisplayHint::Ipv6) => Err(()),
Some(DisplayHint::Ip) => Err(()),
Some(DisplayHint::LowerMac) => Ok(LowerMacFormatter::format(*self)),
Some(DisplayHint::UpperMac) => Ok(UpperMacFormatter::format(*self)),
_ => Err(()),
Expand All @@ -288,8 +286,7 @@ impl Format for [u8; 16] {
Some(DisplayHint::Default) => Err(()),
Some(DisplayHint::LowerHex) => Err(()),
Some(DisplayHint::UpperHex) => Err(()),
Some(DisplayHint::Ipv4) => Err(()),
Some(DisplayHint::Ipv6) => Ok(Ipv6Formatter::format(*self)),
Some(DisplayHint::Ip) => Ok(Ipv6Formatter::format(*self)),
Some(DisplayHint::LowerMac) => Err(()),
Some(DisplayHint::UpperMac) => Err(()),
_ => Err(()),
Expand All @@ -303,8 +300,7 @@ impl Format for [u16; 8] {
Some(DisplayHint::Default) => Err(()),
Some(DisplayHint::LowerHex) => Err(()),
Some(DisplayHint::UpperHex) => Err(()),
Some(DisplayHint::Ipv4) => Err(()),
Some(DisplayHint::Ipv6) => Ok(Ipv6Formatter::format(*self)),
Some(DisplayHint::Ip) => Ok(Ipv6Formatter::format(*self)),
Some(DisplayHint::LowerMac) => Err(()),
Some(DisplayHint::UpperMac) => Err(()),
_ => Err(()),
Expand All @@ -320,8 +316,7 @@ macro_rules! impl_format {
Some(DisplayHint::Default) => Ok(DefaultFormatter::format(self)),
Some(DisplayHint::LowerHex) => Ok(LowerHexFormatter::format(self)),
Some(DisplayHint::UpperHex) => Ok(UpperHexFormatter::format(self)),
Some(DisplayHint::Ipv4) => Err(()),
Some(DisplayHint::Ipv6) => Err(()),
Some(DisplayHint::Ip) => Err(()),
Some(DisplayHint::LowerMac) => Err(()),
Some(DisplayHint::UpperMac) => Err(()),
_ => Ok(DefaultFormatter::format(self)),
Expand Down Expand Up @@ -350,8 +345,7 @@ macro_rules! impl_format_float {
Some(DisplayHint::Default) => Ok(DefaultFormatter::format(self)),
Some(DisplayHint::LowerHex) => Err(()),
Some(DisplayHint::UpperHex) => Err(()),
Some(DisplayHint::Ipv4) => Err(()),
Some(DisplayHint::Ipv6) => Err(()),
Some(DisplayHint::Ip) => Err(()),
Some(DisplayHint::LowerMac) => Err(()),
Some(DisplayHint::UpperMac) => Err(()),
_ => Ok(DefaultFormatter::format(self)),
Expand Down Expand Up @@ -752,7 +746,7 @@ mod test {
let (mut len, mut input) = new_log(3).unwrap();

len += "ipv4: ".write(&mut input[len..]).unwrap();
len += DisplayHint::Ipv4.write(&mut input[len..]).unwrap();
len += DisplayHint::Ip.write(&mut input[len..]).unwrap();
// 10.0.0.1 as u32
len += 167772161u32.write(&mut input[len..]).unwrap();

Expand All @@ -773,7 +767,7 @@ mod test {
let (mut len, mut input) = new_log(3).unwrap();

len += "ipv6: ".write(&mut input[len..]).unwrap();
len += DisplayHint::Ipv6.write(&mut input[len..]).unwrap();
len += DisplayHint::Ip.write(&mut input[len..]).unwrap();
// 2001:db8::1:1 as byte array
let ipv6_arr: [u8; 16] = [
0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
Expand All @@ -798,7 +792,7 @@ mod test {
let (mut len, mut input) = new_log(3).unwrap();

len += "ipv6: ".write(&mut input[len..]).unwrap();
len += DisplayHint::Ipv6.write(&mut input[len..]).unwrap();
len += DisplayHint::Ip.write(&mut input[len..]).unwrap();
// 2001:db8::1:1 as u16 array
let ipv6_arr: [u16; 8] = [
0x2001, 0x0db8, 0x0000, 0x0000, 0x0000, 0x0000, 0x0001, 0x0001,
Expand Down
2 changes: 1 addition & 1 deletion test/integration-ebpf/src/log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn test_log(ctx: ProbeContext) {
let ipv6 = [
32u8, 1u8, 13u8, 184u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 1u8,
]; // 2001:db8::1
info!(&ctx, "ipv4: {:ipv4}, ipv6: {:ipv6}", ipv4, ipv6);
info!(&ctx, "ipv4: {:i}, ipv6: {:i}", ipv4, ipv6);
let mac = [4u8, 32u8, 6u8, 9u8, 0u8, 64u8];
trace!(&ctx, "mac lc: {:mac}, mac uc: {:MAC}", mac, mac);
let hex = 0x2f;
Expand Down

0 comments on commit 84e5e28

Please sign in to comment.