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

bugtool: fix IP route debug gathering commands #18059

Merged
merged 1 commit into from Nov 30, 2021

Conversation

tklauser
Copy link
Member

Commit 8bcc4e5 ("bugtool: avoid allocation on conversion of
execCommand result to string") broke the ip route show commands
because the change from []byte to string causes the %v formatting
verb to emit the raw byte slice, not the string. Fix this by using the
%s formatting verb to make sure the argument gets interpreted as a
string.

Also fix another instance in writeCmdToFile where fmt.Fprint is now
invoked with a byte slice.

Grepping for %v in bugtool sources and manually inspecting all changes
from commit 8bcc4e5 showed no other instances where a byte slice
could potentially end up being formatted in a wrong way.

Fixes: 8bcc4e5 ("bugtool: avoid allocation on conversion of execCommand result to string")
Fixes: #17546
Closes: #17896

Commit 8bcc4e5 ("bugtool: avoid allocation on conversion of
execCommand result to string") broke the `ip route show` commands
because the change from `[]byte` to `string` causes the `%v` formatting
verb to emit the raw byte slice, not the string. Fix this by using the
`%s` formatting verb to make sure the argument gets interpreted as a
string.

Also fix another instance in `writeCmdToFile` where `fmt.Fprint` is now
invoked with a byte slice.

Grepping for `%v` in bugtool sources and manually inspecting all changes
from commit 8bcc4e5 showed no other instances where a byte slice
could potentially end up being formatted in a wrong way.

Fixes: 8bcc4e5 ("bugtool: avoid allocation on conversion of execCommand result to string")

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser added release-note/bug This PR fixes an issue in a previous release of Cilium. area/bugtool Impacts gathering of data for debugging purposes. labels Nov 30, 2021
@tklauser tklauser requested a review from a team as a code owner November 30, 2021 12:09
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.0 Nov 30, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Nov 30, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.12 Nov 30, 2021
@tklauser
Copy link
Member Author

Marked for backport to 1.9, 1.10 and 1.11 because #17546 introducing the regression was backported as well.

@borkmann borkmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 30, 2021
@borkmann borkmann merged commit e38e3c4 into cilium:master Nov 30, 2021
@tklauser tklauser deleted the bugtool-ip-route-dump-fix branch November 30, 2021 13:32
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.6 Nov 30, 2021
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
3 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.12 Nov 30, 2021
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
18 tasks
@nathanjsweet nathanjsweet added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.11 in 1.11.0 Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.11 in 1.11.0 Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.12 Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.6 Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bugtool Impacts gathering of data for debugging purposes. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.10.6
Backport done to v1.10
1.11.0
Backport done to v1.11
1.9.12
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

Bugtool for dumping IPv6 routes is broken
6 participants