* [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> @ 2021-01-11 15:31 giladreti 2021-01-11 15:51 ` Marek Behún 2021-01-11 15:55 ` Daniel Borkmann 0 siblings, 2 replies; 5+ messages in thread From: giladreti @ 2021-01-11 15:31 UTC (permalink / raw) To: bpf, Yonghong Song Cc: Alexei Starovoitov, Daniel Borkmann, netdev, linux-kernel Added support for pointer to mem register spilling, to allow the verifier to track pointer to valid memory addresses. Such pointers are returned for example by a successful call of the bpf_ringbuf_reserve helper. This patch was suggested as a solution by Yonghong Song. --- kernel/bpf/verifier.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 17270b8404f1..36af69fac591 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) case PTR_TO_RDWR_BUF: case PTR_TO_RDWR_BUF_OR_NULL: case PTR_TO_PERCPU_BTF_ID: + case PTR_TO_MEM: + case PTR_TO_MEM_OR_NULL: return true; default: return false; -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> 2021-01-11 15:31 [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> giladreti @ 2021-01-11 15:51 ` Marek Behún 2021-01-11 15:55 ` Daniel Borkmann 1 sibling, 0 replies; 5+ messages in thread From: Marek Behún @ 2021-01-11 15:51 UTC (permalink / raw) To: giladreti Cc: bpf, Yonghong Song, Alexei Starovoitov, Daniel Borkmann, netdev, linux-kernel The Signed-off-by line should be last in the commit message, not first. First line (which becomes e-mail subject) should describe what the commit does (in a short one liner) and where it does it. So for your patch it could be something like bpf: support pointer to mem register spilling in verifier The commit message should be written in present simple tense, not past simple, ie. not "Added support" but "Add support". Also we need your name and surname in From: header and Signed-off-by: tag. So instead of giladreti <gilad.reti@gmail.com> it should be Gilad Reti <gilad.reti@gmail.com> if that is your real time. If it is not, please provide your real name. On Mon, 11 Jan 2021 17:31:23 +0200 giladreti <gilad.reti@gmail.com> wrote: > Added support for pointer to mem register spilling, to allow the verifier > to track pointer to valid memory addresses. Such pointers are returned > for example by a successful call of the bpf_ringbuf_reserve helper. > > This patch was suggested as a solution by Yonghong Song. > --- > kernel/bpf/verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 17270b8404f1..36af69fac591 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > case PTR_TO_RDWR_BUF: > case PTR_TO_RDWR_BUF_OR_NULL: > case PTR_TO_PERCPU_BTF_ID: > + case PTR_TO_MEM: > + case PTR_TO_MEM_OR_NULL: > return true; > default: > return false; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> 2021-01-11 15:31 [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> giladreti 2021-01-11 15:51 ` Marek Behún @ 2021-01-11 15:55 ` Daniel Borkmann 2021-01-11 16:03 ` Gilad Reti 1 sibling, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2021-01-11 15:55 UTC (permalink / raw) To: giladreti, bpf, Yonghong Song; +Cc: Alexei Starovoitov, netdev, linux-kernel Hello Gilad, On 1/11/21 4:31 PM, giladreti wrote: > Added support for pointer to mem register spilling, to allow the verifier > to track pointer to valid memory addresses. Such pointers are returned > for example by a successful call of the bpf_ringbuf_reserve helper. > > This patch was suggested as a solution by Yonghong Song. The SoB should not be in subject line but as part of the commit message instead and with proper name, e.g. Signed-off-by: Gilad Reti <gilad.reti@gmail.com> For subject line, please use a short summary that fits the patch prefixed with the subsystem "bpf: [...]", see also [0] as an example. Thanks. It would be good if you could also add a BPF selftest for this [1]. [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/ https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/verifier/spill_fill.c > --- > kernel/bpf/verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 17270b8404f1..36af69fac591 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > case PTR_TO_RDWR_BUF: > case PTR_TO_RDWR_BUF_OR_NULL: > case PTR_TO_PERCPU_BTF_ID: > + case PTR_TO_MEM: > + case PTR_TO_MEM_OR_NULL: > return true; > default: > return false; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> 2021-01-11 15:55 ` Daniel Borkmann @ 2021-01-11 16:03 ` Gilad Reti 2021-01-11 20:39 ` Andrii Nakryiko 0 siblings, 1 reply; 5+ messages in thread From: Gilad Reti @ 2021-01-11 16:03 UTC (permalink / raw) To: Daniel Borkmann Cc: bpf, Yonghong Song, Alexei Starovoitov, Networking, linux-kernel On Mon, Jan 11, 2021, 17:55 Daniel Borkmann <daniel@iogearbox.net> wrote: > > Hello Gilad, > > On 1/11/21 4:31 PM, giladreti wrote: > > Added support for pointer to mem register spilling, to allow the verifier > > to track pointer to valid memory addresses. Such pointers are returned > > for example by a successful call of the bpf_ringbuf_reserve helper. > > > > This patch was suggested as a solution by Yonghong Song. > > The SoB should not be in subject line but as part of the commit message instead > and with proper name, e.g. > > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > > For subject line, please use a short summary that fits the patch prefixed with > the subsystem "bpf: [...]", see also [0] as an example. Thanks. > > It would be good if you could also add a BPF selftest for this [1]. > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc > [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/ > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/verifier/spill_fill.c > Sure. Thanks for your guidance. As you can probably tell, I am new to kernel code contribution (in fact this is a first time for me). Should I try to submit this patch again? Sorry in advance for all the overhead I may be causing to you... > > --- > > kernel/bpf/verifier.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index 17270b8404f1..36af69fac591 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > > case PTR_TO_RDWR_BUF: > > case PTR_TO_RDWR_BUF_OR_NULL: > > case PTR_TO_PERCPU_BTF_ID: > > + case PTR_TO_MEM: > > + case PTR_TO_MEM_OR_NULL: > > return true; > > default: > > return false; > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> 2021-01-11 16:03 ` Gilad Reti @ 2021-01-11 20:39 ` Andrii Nakryiko 0 siblings, 0 replies; 5+ messages in thread From: Andrii Nakryiko @ 2021-01-11 20:39 UTC (permalink / raw) To: Gilad Reti Cc: Daniel Borkmann, bpf, Yonghong Song, Alexei Starovoitov, Networking, open list On Mon, Jan 11, 2021 at 8:06 AM Gilad Reti <gilad.reti@gmail.com> wrote: > > On Mon, Jan 11, 2021, 17:55 Daniel Borkmann <daniel@iogearbox.net> wrote: > > > > Hello Gilad, > > > > On 1/11/21 4:31 PM, giladreti wrote: > > > Added support for pointer to mem register spilling, to allow the verifier > > > to track pointer to valid memory addresses. Such pointers are returned > > > for example by a successful call of the bpf_ringbuf_reserve helper. > > > > > > This patch was suggested as a solution by Yonghong Song. > > > > The SoB should not be in subject line but as part of the commit message instead > > and with proper name, e.g. > > > > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > > > > For subject line, please use a short summary that fits the patch prefixed with > > the subsystem "bpf: [...]", see also [0] as an example. Thanks. > > > > It would be good if you could also add a BPF selftest for this [1]. > > > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/ > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/verifier/spill_fill.c > > > > Sure. Thanks for your guidance. As you can probably tell, I am new to > kernel code contribution (in fact this is a first time for me). > Should I try to submit this patch again? In addition to all already mentioned things, also make sure you have [PATCH bpf] prefix in the subject, to identify that this is a bug fix for the bpf tree. Also you missed adding Fixes tag, please add this: Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it") And yes, please re-submit with all the feedback incorporated (including the selftest). > > Sorry in advance for all the overhead I may be causing to you... > > > > --- > > > kernel/bpf/verifier.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > index 17270b8404f1..36af69fac591 100644 > > > --- a/kernel/bpf/verifier.c > > > +++ b/kernel/bpf/verifier.c > > > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > > > case PTR_TO_RDWR_BUF: > > > case PTR_TO_RDWR_BUF_OR_NULL: > > > case PTR_TO_PERCPU_BTF_ID: > > > + case PTR_TO_MEM: > > > + case PTR_TO_MEM_OR_NULL: > > > return true; > > > default: > > > return false; > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-11 20:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-11 15:31 [PATCH] Signed-off-by: giladreti <gilad.reti@gmail.com> giladreti 2021-01-11 15:51 ` Marek Behún 2021-01-11 15:55 ` Daniel Borkmann 2021-01-11 16:03 ` Gilad Reti 2021-01-11 20:39 ` Andrii Nakryiko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.