-
Notifications
You must be signed in to change notification settings - Fork 31
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-1703 Add enrichment in packet capture #364
Conversation
Skipping CI for Draft Pull Request. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #364 +/- ##
=======================================
Coverage ? 32.16%
=======================================
Files ? 48
Lines ? 3666
Branches ? 0
=======================================
Hits ? 1179
Misses ? 2386
Partials ? 101
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@eranra & @KalmanMeth you might be interested by this PR 👀 |
} else if udpLayer := packet.Layer(layers.LayerTypeUDP); udpLayer != nil { | ||
udp, _ := udpLayer.(*layers.UDP) | ||
out["SrcPort"] = udp.SrcPort.String() | ||
out["DstPort"] = udp.DstPort.String() |
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.
you are missing SCTP
protocol support
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.
sure I can add it !
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.
out["IcmpCode"] = icmpv6.TypeCode.Code() | ||
} | ||
|
||
if dnsLayer := packet.Layer(layers.LayerTypeDNS); dnsLayer != nil { |
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 we allow DNS in packet capture mode ? pca code doesn't enable any feature including DNS
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.
|
||
out["Bytes"] = len(pr.Stream) | ||
// Data is base64 encoded to avoid marshal / unmarshal issues | ||
out["Data"] = base64.StdEncoding.EncodeToString(packet.Data()) |
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.
is this efficient I didn't look for any better alternative did u get a chance to explore other options ?
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'm considering to update the gRPC message to send flows and packets as two separate byte array but during the pipeline lifetime I feel that's the easiest solution.
On my local kind cluster it's super fast so for now I'm not looking deeper in this.
if dnsLayer := packet.Layer(layers.LayerTypeDNS); dnsLayer != nil { | ||
dns, _ := dnsLayer.(*layers.DNS) | ||
out["DnsId"] = dns.ID | ||
out["DnsFlagsResponseCode"] = dns.ResponseCode.String() |
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.
DNS enrichement not enabled with pca
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.
see #364 (comment)
/lgtm |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=30c0f75 make set-agent-image |
/ok-to-test |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=95041fd make set-agent-image |
/label qe-approved |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
This PR allows pcap parsing to generic map. Check CLI implementation for usage.
Dependencies
netobserv/network-observability-cli#61
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.