Skip to content
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

Aspsk/pr/make cilium monitor great again #24143

Merged

Conversation

aspsk
Copy link
Contributor

@aspsk aspsk commented Mar 2, 2023

Commit bb5e3fe "improved" the output of 'cilium monitor -t drop'
so that file ids were used since in the report:

    xx drop (Invalid packet) flow 0x0 to endpoint 0, ifindex 12, file 101:80, ...
    xx drop (Policy denied) flow 0xeea42740 to endpoint 0, ifindex 14, file 2:1167, ...

This turns out to be really inconvenient, so switch back to file names. Now
output looks more readable, like this:

    xx drop (Invalid source ip) flow 0x0 to endpoint 0, ifindex 8, file bpf_lxc.c:734, ...
    xx drop (Unknown L3 target address) flow 0x0 to endpoint 0, ifindex 8, file lib/icmp6.h:413, ...
    xx drop (Invalid source ip) flow 0x0 to endpoint 0, ifindex 10, file bpf_lxc.c:734, ...

The pkg/datapath/loader/check-sources.sh script was adjusted to check that file
names are synced between C and Go.

Improve cilium monitor output for dropped packets: display source file names instead of numerical ids

Commit bb5e3fe "improved" the output of 'cilium monitor -t drop'
so that file ids were used since in the report:

    xx drop (Invalid packet) flow 0x0 to endpoint 0, ifindex 12, file 101:80, ...
    xx drop (Policy denied) flow 0xeea42740 to endpoint 0, ifindex 14, file 2:1167, ...

This turns out to be really inconvenient, so switch back to file names. Now
output looks more readable, like this:

    xx drop (Invalid source ip) flow 0x0 to endpoint 0, ifindex 8, file bpf_lxc.c:734, ...
    xx drop (Unknown L3 target address) flow 0x0 to endpoint 0, ifindex 8, file lib/icmp6.h:413, ...
    xx drop (Invalid source ip) flow 0x0 to endpoint 0, ifindex 10, file bpf_lxc.c:734, ...

The pkg/datapath/loader/check-sources.sh script was adjusted to check that file
names are synced between C and Go.

Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
Script ignored the error code in some cases, only printed the error message.
Fix this.

Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
@aspsk aspsk added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 2, 2023
@aspsk aspsk requested review from a team as code owners March 2, 2023 17:18
@aspsk aspsk added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 3, 2023
@aspsk
Copy link
Contributor Author

aspsk commented Mar 3, 2023

Marked as ready-to-merge, as this change doesn't require the full test suite.

@borkmann borkmann merged commit 6ba5716 into cilium:master Mar 3, 2023
@aspsk aspsk deleted the aspsk/pr/make-cilium-monitor-great-again branch March 30, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants