On Tue, Nov 12, 2024 at 11:13 AM <dev@xxxxxxxxxxx> wrote: > > On Mon, Nov 11, 2024 at 07:01:26PM -0800, Alexei Starovoitov wrote: > > On Mon, Nov 11, 2024 at 6:15 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: > > > > > > > > > > > > On 11/10/2024 7:29 PM, Florian Lehner wrote: > > > > Introduce a new flag for batch operations that allows the deletion process > > > > to continue even if certain keys are missing. This simplifies map flushing > > > > by eliminating the requirement to maintain a separate list of keys and > > > > makes sure maps can be flushed with a single batch delete operation. > > > > > > Is it expensive to close and recreate a new map instead ? If it is > > > expensive, does it make more sense to add a new command to delete all > > > elements in the map ? Because reusing the deletion logic will make each > > > deletion involve an unnecessary lookup operation. > > > > +1 to above questions. > > There is an eBPF map, that a variable number of eBPF programs use, to access > common states for a variable number of connections. On predefined events, a set > of keys is deleted from this map. This set can either be all keys or just a > subset of all keys - but it is not guaranteed that this set of keys still exists > in this eBPF map. > The current work around is to use bpf_map_lookup_and_delete_batch(), as this > operation continues on missing keys and clears all requested keys from the eBPF > map. Hmm. bpf_map_lookup_and_delete_batch() deletes all elements and returns key/values. It doesn't do any key comparisons. There is no concept of skipping keys or missing keys. It deletes all. This cannot be a workaround. > The noticeable downside of bpf_map_lookup_and_delete_batch() is the memory > requirement that comes with the lookup and allocation for the values. Sure. That's why I suggest for_each+delete that clears the map without returning key/value back to user space. > > > [..] If it is > > > expensive, does it make more sense to add a new command to delete all > > > elements in the map ? > > It felt like bpf_map_delete_batch() was introduced for this use case. So adding > a new command was not considered. > > > > > In addition: > > > > What is the use case ? > > Are you trying to erase all elements from the map ? > > > > If so you bpf_for_each_map_elem() and delete elems while iterating. > > bpf_for_each_map_elem() could be an option if the map should be flushed > completley, but in most cases only a subset of keys should be removed from the > map. > > > > > This extra flag looks too specific. > > Sure, the proposed flag is focused on the delete operation. What could be the > requirement to make it less specific? I feel that letting map_delete_batch() ignore keys make it a footgun. Sounds like in your case the key is a network connection tuple. So by ignoring unknown tuple you're saying 'delete connection info if it's there'. I assuming because bpf prog and user space may race to delete it. In such case user space can trim key list and retry generic_map_delete_batch. It's another syscall. A bit more overhead, but try to speed up this very specific use cases seems wrong. It's not going to be much faster. The set of keys is probably limited.
