Support clear() and delete() on a count()-based map without key #1639
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
mmisono
force-pushed
the
clear_count_maps
branch
2 times, most recently
from
0acedd8 to
051b6ee
Compare
|
So the difference between before and after 439a6bc is a count()-based map is an array, so it'll be printed out even if we clear() or delete() it. For example: but But I guess most people want to know the final value of the count. |
danobi
reviewed
Nov 20, 2020
src/imap.h
Outdated
| @@ -27,6 +27,11 @@ class IMap | |||
| return map_type_ == BPF_MAP_TYPE_PERCPU_HASH || | |||
| map_type_ == BPF_MAP_TYPE_PERCPU_ARRAY; | |||
| } | |||
| bool is_not_clearable() const | |||
There was a problem hiding this comment.
Can we name this is_clearable()? Would be confusing to read !is_not_clearable() later on (the double negation).
There was a problem hiding this comment.
That makes sense. will fix.
mmisono
force-pushed
the
clear_count_maps
branch
4 times, most recently
from
bbe53ce to
6efc09f
Compare
mmisono
force-pushed
the
clear_count_maps
branch
from
6efc09f to
e29a397
Compare
danobi
approved these changes
Jan 5, 2021
|
Needs a rebase |
439a6bc changes so that count()-based map with no arguments uses BPF_MAP_TYPE_PERCPU_ARRAY for the performance. The problem is crear() uses bpf_map_delete_elem(), but ARRAY-typed maps cannot be deleted. To solve this, suggested by fbs in bpftrace#1350 (comment), use zero() internally in clear() if a map type is array. Suggested-by: bas smit <[email protected]> Signed-off-by: Masanori Misono <[email protected]>
Like clear(), delete() also does not work for count()-based map (please see the previous commit message for the details.) To fix this, instead calling bpf_map_delete_elem() when delete(), call bpf_mpa_updat_elem() and update the map value with zero.
mmisono
force-pushed
the
clear_count_maps
branch
from
e29a397 to
91a71e6
Compare
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.
439a6bc changes so that count()-based map with no arguments uses
BPF_MAP_TYPE_PERCPU_ARRAY for the performance. The problem is crear()
uses bpf_map_delete_elem(), but ARRAY-typed maps cannot be deleted.
To solve this, suggested by fbs in #1350 (comment),
use zero() internally in clear() if a map type is array.
Like clear(), delete() also does not work for count()-based map (please
see the previous commit message for the details.) To fix this, instead
calling bpf_map_delete_elem() when delete(), call bpf_mpa_updat_elem()
and update the map value with zero.
Closes #1350
Checklist
docs/reference_guide.mdCHANGELOG.md