-
Notifications
You must be signed in to change notification settings - Fork 49
NETOBSERV-2455 - Get DNS names #820
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
|
[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 |
|
New images: These will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=34362e6 make set-agent-image |
bpf/dns_tracker.h
Outdated
| __uint(max_entries, 1); | ||
| __type(key, u32); | ||
| __type(value, struct dns_parse_buffer); | ||
| } dns_parse_buffer_map SEC(".maps"); |
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.
we don't need this additional map, u can directly read dns name from dns record and added to metric
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.
rebased to remove the map: e7fe3e5
bpf/dns_tracker.h
Outdated
| if (buf) { | ||
| // Copy raw QNAME bytes (label-encoded) and let userspace decode to dotted form | ||
| __builtin_memset(buf->name, 0, DNS_NAME_MAX_LEN); | ||
| u32 qname_off = dns_offset + sizeof(struct dns_header); |
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.
pls make sure this math aligns for both DNS over UDP and DNS over TCP , IIRC the headers are of different format
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.
| type Protobuf struct { | ||
| } | ||
|
|
||
| // dnsRawNameToDotted parses a label-encoded DNS QNAME (raw bytes copied from kernel) |
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.
this is a good candidate for unit-test as it looks so convoluted
| break | ||
| } | ||
| // Stop on compression pointer (0xC0xx) since we didn't follow it in kernel | ||
| if (l & 0xC0) == 0xC0 { |
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.
what is this 0xC0 magic number ?
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.
That's a skip when a DNS messageg use compression to reduce its size (repeated domain name parts are using pointers).
l is the length byte of a DNS label.
0xC0 in binary is 11000000.
The bitwise AND l & 0xC0 isolates the top two bits of l.
If the result equals 0xC0, it indicates a compression pointer.
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.
pls add comments with all the above details
| uint8(mac), | ||
| } | ||
| } | ||
|
|
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.
another UT is needed for this
|
@jpinsonneau: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| "additional_flow_metrics":"per_cpu_hash", | ||
| "packet_record":"ringbuf", | ||
| "dns_flows":"hash", | ||
| "dns_parse_buffer_map":"percpu_array", |
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.
not needed anymore




Description
Parse truncated DNS QNAME using per-CPU buffer and append it to the flows when available
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.
To run a perfscale test, comment with:
/test ebpf-node-density-heavy-25nodes