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

Rounding behavor of --max-flows is confusing #11650

Closed
gandro opened this issue Feb 27, 2020 · 3 comments · Fixed by #13894
Closed

Rounding behavor of --max-flows is confusing #11650

gandro opened this issue Feb 27, 2020 · 3 comments · Fixed by #13894
Assignees
Labels
kind/enhancement This would improve or streamline existing functionality. pinned These issues are not marked stale by our issue bot. sig/hubble Impacts hubble server or relay

Comments

@gandro
Copy link
Member

gandro commented Feb 27, 2020

Currently, --max-flows bumps up to the next (2^n)-1.

--max-flows uint32   Max number of flows to store in memory (gets rounded up to closest (2^n)-1 (default 131071)

Examples:

1000 -> 1024
1023 -> 1024
1024 -> 2048

As a user, I find this behavior confusing. I would expect --max-flows 1024 to be rounded as 1024, not bumped up to 2048.

@rolinh
Copy link
Member

rolinh commented Feb 27, 2020

Note that this is due to ring buffer implementation details: even if max flows number is changed to not appear rounded to the next power of 2 to the user, unless the ring implementation is changed, the ring buffer will still be allocated with a size rounded to the next power of 2.
Ultimately, I don't think that this is so trivial to fix so I'd remove the good-first-issue label.

@glibsm
Copy link
Member

glibsm commented Feb 27, 2020

I think there is an easy way out of here where we treat 2n-1 and 2n as actual 2*n-1 capacity. I certainly wouldn't expect 1024 to be upgraded to 2047 in reality, if that makes any sense.

We don't need to modify the actual internals that much, just how we interpret this one-off thing (one off is due to us needing a mask of 1s that covers the entire capacity)

@rolinh rolinh transferred this issue from cilium/hubble May 22, 2020
@rolinh rolinh added sig/hubble Impacts hubble server or relay kind/enhancement This would improve or streamline existing functionality. labels May 22, 2020
@stale
Copy link

stale bot commented Jul 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 21, 2020
@rolinh rolinh added pinned These issues are not marked stale by our issue bot. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Jul 21, 2020
@rolinh rolinh self-assigned this Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This would improve or streamline existing functionality. pinned These issues are not marked stale by our issue bot. sig/hubble Impacts hubble server or relay
Projects
None yet
3 participants