-
-
Notifications
You must be signed in to change notification settings - Fork 25
Add frame information to packet info popup #47
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?
Conversation
54fbe55
to
5a79a1f
Compare
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.
Great PR 👏
just couple of comments :)
oryx-ebpf/src/main.rs
Outdated
fn submit(packet: RawFrame) { | ||
if let Some(mut buf) = DATA.reserve::<RawFrame>(0) { |
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.
maybe change the param name from packet
to frame
fn submit(frame: RawFrame) {
...
}
#[repr(C)] | ||
#[derive(Clone)] | ||
pub struct RawFrame { | ||
pub header: EthHdr, |
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.
maybe we should define the header here with the fields we are interested in to be more explicit, what do you think ?
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.
Eh it would be easy enough to do this, I dont have a strong opinion either way. My thinking in using the already defined type was mostly for convenience since it is already defined and is returned from the TcContext
so app layer dosnt need to do extra work to convert it (not a big deal to implement a From
impl though). But also it includes additional information that could be interesting to render later, mainly in the eth type. But this assumes network_types
expands the eth type enum to cover other interesting packet types like Wake on LAN, MPLS, or VLAN etc., but that would of course need changes in aya
oryx-tui/src/app.rs
Outdated
let packets = Arc::new(RwLock::new(Vec::with_capacity( | ||
RawPacket::LEN * 1024 * 1024, | ||
))); | ||
let packets = Arc::new(RwLock::new(Vec::with_capacity(RawFrame::LEN * 1024 * 1024))); |
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.
packets
should be renamed frames
also, in the App
structs, maybe change the fields packets
to frames
for consistency, what do you think ?
oryx-tui/src/packet/eth_frame.rs
Outdated
|
||
let infos = [ | ||
Row::new(vec![ | ||
Span::styled("Dest. MAC Address", Style::new().bold()), |
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.
I guess you can remove the dot
oryx-tui/src/section/inspection.rs
Outdated
NetworkPacket::Ip(ip_packet) => { | ||
let (network_packet_block, eth_frame_block) = { | ||
let chunks = Layout::default() | ||
.direction(Direction::Vertical) | ||
.constraints(Constraint::from_percentages([70, 30])) | ||
.flex(ratatui::layout::Flex::SpaceAround) | ||
.margin(2) | ||
.split(block); | ||
|
||
(chunks[0], chunks[1]) | ||
}; | ||
|
||
// split to apply extra margin to line up with network packet rendering | ||
let eth_frame_block = Layout::default() | ||
.direction(Direction::Horizontal) | ||
.constraints(Constraint::from_percentages([95, 5])) | ||
.margin(2) | ||
.split(eth_frame_block)[0]; | ||
|
||
EthFrameHeader::from(app_packet.eth_header).render(eth_frame_block, frame); | ||
ip_packet.render(network_packet_block, frame) | ||
} | ||
NetworkPacket::Arp(arp_packet) => { | ||
let (arp_block, eth_frame_block) = { | ||
let chunks = Layout::default() | ||
.direction(Direction::Vertical) | ||
.constraints([ | ||
Constraint::Fill(1), | ||
Constraint::Percentage(50), | ||
Constraint::Length(6), | ||
Constraint::Fill(1), | ||
]) | ||
.flex(ratatui::layout::Flex::Center) | ||
.margin(2) | ||
.split(block); | ||
|
||
(chunks[1], chunks[2]) | ||
}; | ||
|
||
arp_packet.render(arp_block, frame); | ||
EthFrameHeader::from(app_packet.eth_header).render(eth_frame_block, frame); | ||
} |
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.
there is some extra space between the l2 layer and the layers on top that I think this needs some tweaks. I fixed it locally, just let me know if you want me to share them
Closes #45
Replaces the
RawPacket
type withRawFrame
, ultimately passingnetwork_types::eth::EthHdr
info to the tui rendering layer. This allows to show basic frame information in the packet info popup, along with other packet details.