lib,daemon,cli: use float for meta.probability matcher#454
Merged
qdeslandes merged 1 commit intofacebook:mainfrom Mar 4, 2026
Merged
lib,daemon,cli: use float for meta.probability matcher#454qdeslandes merged 1 commit intofacebook:mainfrom
qdeslandes merged 1 commit intofacebook:mainfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/bpfilter/cgen/matcher/meta.c
Outdated
| program, | ||
| BPF_JMP_IMM(BPF_JGT, BPF_REG_0, | ||
| (uint32_t)((uint64_t)UINT32_MAX * (proba / 100.0)), 0)); | ||
| (uint32_t)((double)UINT32_MAX / 100.0 * proba), 0)); |
There was a problem hiding this comment.
Claude: nit: The threshold expression here uses (double)UINT32_MAX / 100.0 * proba, while _bf_matcher_generate_meta_flow_probability uses (double)UINT32_MAX * (proba / 100.0). Since the two matchers now share the same payload type and parse/print functions, using the same threshold expression would make the consolidation more complete.
Suggested change
| (uint32_t)((double)UINT32_MAX / 100.0 * proba), 0)); | |
| (uint32_t)((double)UINT32_MAX * (proba / 100.0)), 0)); |
|
Claude: review of facebook/bpfilter #454 (9908fea) Clean PR — good consolidation of the two probability matchers. Both previous nits and the maintainer's lexer state feedback have been addressed. Nits
|
agelun
force-pushed
the
agelun/probability-float
branch
from
f384813 to
e120a75
Compare
qdeslandes
requested changes
Mar 4, 2026
Contributor
qdeslandes
left a comment
qdeslandes
left a comment
There was a problem hiding this comment.
Small change, otherwise LGTM.
Change the meta.probability matcher payload from uint8_t to float, enabling fractional percentage values (e.g., "33.33%", "0.1%"). This aligns meta.probability with meta.flow_probability which already used float. Consolidate the now-identical parse and print functions between the two matchers.
agelun
force-pushed
the
agelun/probability-float
branch
from
e120a75 to
9908fea
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Change the meta.probability matcher payload from uint8_t to float, enabling fractional percentage values (e.g., "33.33%", "0.1%"). This aligns meta.probability with meta.flow_probability which already used float. Consolidate the now-identical parse and print functions between the two matchers.