-
Notifications
You must be signed in to change notification settings - Fork 6
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
NETOBSERV-1850 fix cli enrichment issue #91
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
==========================================
- Coverage 25.10% 25.08% -0.03%
==========================================
Files 8 8
Lines 1147 1148 +1
==========================================
Hits 288 288
- Misses 838 839 +1
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
|
// then append packet to file | ||
err = pw.WriteEnhancedPacketBlock(0, ts, b, types.EnhancedPacketOptions{}) | ||
// then append packet to file using totalPackets as unique id | ||
err = pw.WriteEnhancedPacketBlock(totalPackets, ts, b, types.EnhancedPacketOptions{}) |
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.
@jpinsonneau how the total packet can be unique id ? why not startupTime ?
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 increase the totalPackets
value each time we add a packet just below this call:
totalPackets = totalPackets + 1
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.
yeah I saw that isn't that will be the case with other pkts u capture so if I received 2 udp pkts then 2 tcp pkts in two different runs won't that collide ?
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.
do you open the 2 pcapng files in one ? 🤔
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 tried the following code to see what happens:
err = pw.WriteEnhancedPacketBlock(uint32(startupTime.UnixMilli())+totalPackets, ts, b, types.EnhancedPacketOptions{})
And it seems wireshark is not expecting that. Here the id is an index related to the previously created interface description using WriteInterfaceDescription
call:
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.
ok I guess lets proceed as is thanks for confirming
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.
thank you for reviewing !
/lgtm |
To be tested post merge |
Description
Fix CLI packet enrichment issue where all packets were pointing the same info
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.