On Wed, Aug 19, 2020 at 02:41:50PM +0200, KP Singh wrote: > On 8/18/20 3:05 AM, Martin KaFai Lau wrote: > > On Mon, Aug 03, 2020 at 06:46:51PM +0200, KP Singh wrote: > >> From: KP Singh <kpsingh@xxxxxxxxxx> > >> > >> Refactor the functionality in bpf_sk_storage.c so that concept of > >> storage linked to kernel objects can be extended to other objects like > >> inode, task_struct etc. > >> > >> Each new local storage will still be a separate map and provide its own > >> set of helpers. This allows for future object specific extensions and > >> still share a lot of the underlying implementation. > >> > >> This includes the changes suggested by Martin in: > >> > >> https://lore.kernel.org/bpf/20200725013047.4006241-1-kafai@xxxxxx/ > >> > >> which adds map_local_storage_charge, map_local_storage_uncharge, > >> and map_owner_storage_ptr. > > A description will still be useful in the commit message to talk > > about the new map_ops, e.g. > > they allow kernel object to optionally have different mem-charge strategy. > > > >> > >> Co-developed-by: Martin KaFai Lau <kafai@xxxxxx> > >> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx> > >> --- > >> include/linux/bpf.h | 9 ++ > >> include/net/bpf_sk_storage.h | 51 +++++++ > >> include/uapi/linux/bpf.h | 8 +- > >> net/core/bpf_sk_storage.c | 246 +++++++++++++++++++++------------ > >> tools/include/uapi/linux/bpf.h | 8 +- > >> 5 files changed, 233 insertions(+), 89 deletions(-) > >> > > >> + struct bpf_local_storage_map *smap, > >> + struct bpf_local_storage_elem *first_selem); > >> + > >> +struct bpf_local_storage_data * > >> +bpf_local_storage_update(void *owner, struct bpf_map *map, void *value, > > Nit. It may be more consistent to take "struct bpf_local_storage_map *smap" > > instead of "struct bpf_map *map" here. > > > > bpf_local_storage_map_check_btf() will be the only one taking > > "struct bpf_map *map". > > That's because it is used in map operations as map_check_btf which expects > a bpf_map *map pointer. We can wrap it in another function but is that > worth doing? Agree. bpf_local_storage_map_check_btf() should stay as is. I meant to only change the "bpf_local_storage_update()" to take "struct bpf_local_storage_map *smap". > > > >> * > >> * The elem of this map can be cleaned up here > >> * or > >> - * by bpf_sk_storage_free() during __sk_destruct(). > >> + * by bpf_local_storage_free() during the destruction of the > >> + * owner object. eg. __sk_destruct. > > This belongs to patch 1 also. > > > In patch, 1, changed it to: > > * The elem of this map can be cleaned up here > * or when the storage is freed e.g. > * by bpf_sk_storage_free() during __sk_destruct(). > +1
