From: Martin KaFai Lau <kafai@fb.com>
To: KP Singh <kpsingh@chromium.org>
Cc: bpf <bpf@vger.kernel.org>,
Linux Security Module list
<linux-security-module@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Paul Turner <pjt@google.com>, Jann Horn <jannh@google.com>
Subject: Re: [PATCH bpf-next v2 1/4] bpf: Generalize bpf_sk_storage
Date: Mon, 6 Jul 2020 11:56:21 -0700 [thread overview]
Message-ID: <20200706185606.j3asqqsxujomaq7z@kafai-mbp> (raw)
In-Reply-To: <CACYkzJ6Vr3TtKQnTrJyB0L47goAMTC0uHoLpsNF8Vo2QySWECw@mail.gmail.com>
On Tue, Jun 30, 2020 at 10:00:18PM +0000, KP Singh wrote:
>
>
> On Tue, Jun 30, 2020 at 9:35 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > On Mon, Jun 29, 2020 at 06:01:00PM +0200, KP Singh wrote:
> > > > >
>
> [...]
>
> > > > > static atomic_t cache_idx;
> > > > inode local storage and sk local storage probably need a separate
> > > > cache_idx. An improvement on picking cache_idx has just been
> > > > landed also.
> > >
> > > I see, thanks! I rebased and I now see that cache_idx is now a:
> > >
> > > static u64 cache_idx_usage_counts[BPF_STORAGE_CACHE_SIZE];
> > >
> > > which tracks the free cache slots rather than using a single atomic
> > > cache_idx. I guess all types of local storage can share this now
> > > right?
> > I believe they have to be separated. A sk-storage will not be cached/stored
> > in inode. Caching a sk-storage at idx=0 of a sk should not stop
> > an inode-storage to be cached at the same idx of a inode.
>
> Ah yes, I see.
>
> I came up with some macros to solve this. Let me know what you think:
> (this is on top of the refactoring I did, so some function names may seem new,
> but it should, hopefully, convey the general idea).
>
> diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
> index 3067774cc640..1dc2e6d72091 100644
> --- a/include/linux/bpf_local_storage.h
> +++ b/include/linux/bpf_local_storage.h
> @@ -79,6 +79,26 @@ struct bpf_local_storage_elem {
> #define SDATA(_SELEM) (&(_SELEM)->sdata)
> #define BPF_STORAGE_CACHE_SIZE 16
>
> +u16 bpf_ls_cache_idx_get(spinlock_t *cache_idx_lock,
> + u64 *cache_idx_usage_count);
> +
> +void bpf_ls_cache_idx_free(spinlock_t *cache_idx_lock,
> + u64 *cache_idx_usage_counts, u16 idx);
> +
> +#define DEFINE_BPF_STORAGE_CACHE(type) \
> +static DEFINE_SPINLOCK(cache_idx_lock_##type); \
> +static u64 cache_idx_usage_counts_##type[BPF_STORAGE_CACHE_SIZE]; \
> +static u16 cache_idx_get_##type(void) \
> +{ \
> + return bpf_ls_cache_idx_get(&cache_idx_lock_##type, \
> + cache_idx_usage_counts_##type); \
> +} \
> +static void cache_idx_free_##type(u16 idx) \
> +{ \
> + return bpf_ls_cache_idx_free(&cache_idx_lock_##type, \
> + cache_idx_usage_counts_##type, \
> + idx); \
> +}
Sorry for the late reply. I missed this email.
The above looks reasonable.
next prev parent reply other threads:[~2020-07-06 18:56 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-17 20:29 [PATCH bpf-next v2 0/4] Generalizing bpf_local_storage KP Singh
2020-06-17 20:29 ` [PATCH bpf-next v2 1/4] bpf: Generalize bpf_sk_storage KP Singh
2020-06-19 6:43 ` Martin KaFai Lau
2020-06-29 16:01 ` KP Singh
2020-06-30 19:34 ` Martin KaFai Lau
2020-06-30 22:00 ` KP Singh
2020-07-06 18:56 ` Martin KaFai Lau [this message]
2020-06-17 20:29 ` [PATCH bpf-next v2 2/4] bpf: Implement bpf_local_storage for inodes KP Singh
2020-06-19 6:52 ` Martin KaFai Lau
2020-06-30 11:49 ` KP Singh
2020-06-22 9:40 ` Quentin Monnet
2020-06-17 20:29 ` [PATCH bpf-next v2 3/4] bpf: Allow local storage to be used from LSM programs KP Singh
2020-06-17 20:29 ` [PATCH bpf-next v2 4/4] bpf: Add selftests for local_storage KP Singh
2020-06-18 18:16 ` Andrii Nakryiko
2020-06-30 11:50 ` KP Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200706185606.j3asqqsxujomaq7z@kafai-mbp \
--to=kafai@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jannh@google.com \
--cc=kpsingh@chromium.org \
--cc=linux-security-module@vger.kernel.org \
--cc=pjt@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).