[v1.8] hubble: Fix reply state unknown being interpreted as false #13876
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ upstream commit ee16843 ] This replaces the existing `reply` field, which suffers from false negatives due to protobuf not being able to distinguish between a false value and an absent value. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
[ upstream commit 0507c62 ] This fixes a long-standing issue with Hubble not being able to distingiush between the reply field being false and the reply field being absent. To address the issue, the old `reply` field is replaced with a tri-state `is_reply` field which can be either nil, true or false. All uses of the old field are removed in the source tree, but the field itself is preserved for backwards-compatibility. This incidentally also fixes a bug where the `port-distribution` metric suffered from the same confusion, which caused it to report source ports as destination ports. Ref: #13248 Fixes: #13744 Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
[ upstream commit 169c517 ] This commit makes the assumption of the `decodeIsReply` functions a bit more explicit and adds unit tests to ensure they remain valid. This commit also changes the behavior for forwarded `PolicyVerdictNotify` to always have `reply=false`, as a policy verdict to allow a connection will not be taken on a reply message. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
[ upstream commit 2bdf827 ] Hubble UI always sets a reply filter when fetching flows. Before the new tri-state `is_reply` was introduced, a filter on `reply=false` would always include drops as well. With the new `is_reply` field, we however do not statically assume anymore that drops only happen for non-reply packets. Therefore, the changes introduced in 0507c62 made it such that Hubble UI (because it sets a filter on `reply=false`) would not display dropped flows anymore. This commit adds a work around, to treat `is_reply=unknown` as `is_reply=false` for dropped flows. Fixes: 0507c62 ("hubble: Fix reply state unknown being interpreted as false") Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
maintainer-s-little-helper
bot
added
backport/1.8
kind/backports
|
test-backport-1.8 |
ti-mo
approved these changes
Nov 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These were skipped in the main backport PR #13875 due conflicts in with the generated protobuf and old function signatures.
I manually regenerated the protobuf (running
PROTOC=protoc-3.11.4 make -C api/v1) and did some minor conflict resolution to match the older function signatures.#13796 is a follow-up to #13750 and therefore could not have been backported by itself.
Once this PR is merged, you can update the PR labels via: