| Message ID | 20190905211528.97828-1-samitolvanen@google.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | kcm: use BPF_PROG_RUN | expand |
On 9/5/19 2:15 PM, Sami Tolvanen wrote: > Instead of invoking struct bpf_prog::bpf_func directly, use the > BPF_PROG_RUN macro. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Acked-by: Yonghong Song <yhs@fb.com> > --- > net/kcm/kcmsock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c > index 5dbc0c48f8cb..f350c613bd7d 100644 > --- a/net/kcm/kcmsock.c > +++ b/net/kcm/kcmsock.c > @@ -379,7 +379,7 @@ static int kcm_parse_func_strparser(struct strparser *strp, struct sk_buff *skb) > struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp); > struct bpf_prog *prog = psock->bpf_prog; > > - return (*prog->bpf_func)(skb, prog->insnsi); > + return BPF_PROG_RUN(prog, skb); > } > > static int kcm_read_sock_done(struct strparser *strp, int err) >
On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 9/5/19 2:15 PM, Sami Tolvanen wrote: > > Instead of invoking struct bpf_prog::bpf_func directly, use the > > BPF_PROG_RUN macro. > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > Acked-by: Yonghong Song <yhs@fb.com> Applied. Thanks
On 9/6/19 10:06 AM, Alexei Starovoitov wrote: > On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <yhs@fb.com> wrote: >> >> >> >> On 9/5/19 2:15 PM, Sami Tolvanen wrote: >>> Instead of invoking struct bpf_prog::bpf_func directly, use the >>> BPF_PROG_RUN macro. >>> >>> Signed-off-by: Sami Tolvanen <samitolvanen@google.com> >> >> Acked-by: Yonghong Song <yhs@fb.com> > > Applied. Thanks > Then we probably need this as well, what do you think ? diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index 8f12f5c6ab875ebaa6c59c6268c337919fb43bb9..6508e88efdaf57f206b84307f5ad5915a2ed21f7 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -378,8 +378,13 @@ static int kcm_parse_func_strparser(struct strparser *strp, struct sk_buff *skb) { struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp); struct bpf_prog *prog = psock->bpf_prog; + int res; - return BPF_PROG_RUN(prog, skb); + preempt_disable(); + res = BPF_PROG_RUN(prog, skb); + preempt_enable(); + + return res; } static int kcm_read_sock_done(struct strparser *strp, int err)
On Mon, Sep 23, 2019 at 02:31:04PM -0700, Eric Dumazet wrote: > On 9/6/19 10:06 AM, Alexei Starovoitov wrote: > > On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <yhs@fb.com> wrote: > >> On 9/5/19 2:15 PM, Sami Tolvanen wrote: > >>> Instead of invoking struct bpf_prog::bpf_func directly, use the > >>> BPF_PROG_RUN macro. > >>> > >>> Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > >> > >> Acked-by: Yonghong Song <yhs@fb.com> > > > > Applied. Thanks > > Then we probably need this as well, what do you think ? Yep, it's broken. 6cab5e90ab2b ("bpf: run bpf programs with preemption disabled") probably forgot about it since it wasn't using BPF_PROG_RUN() in the first place. If you get a chance, please send a proper patch, thanks! > diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c > index 8f12f5c6ab875ebaa6c59c6268c337919fb43bb9..6508e88efdaf57f206b84307f5ad5915a2ed21f7 100644 > --- a/net/kcm/kcmsock.c > +++ b/net/kcm/kcmsock.c > @@ -378,8 +378,13 @@ static int kcm_parse_func_strparser(struct strparser *strp, struct sk_buff *skb) > { > struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp); > struct bpf_prog *prog = psock->bpf_prog; > + int res; > > - return BPF_PROG_RUN(prog, skb); > + preempt_disable(); > + res = BPF_PROG_RUN(prog, skb); > + preempt_enable(); > + > + return res; > } > > static int kcm_read_sock_done(struct strparser *strp, int err)
On 9/24/19 11:59 AM, Daniel Borkmann wrote: > On Mon, Sep 23, 2019 at 02:31:04PM -0700, Eric Dumazet wrote: >> On 9/6/19 10:06 AM, Alexei Starovoitov wrote: >>> On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <yhs@fb.com> wrote: >>>> On 9/5/19 2:15 PM, Sami Tolvanen wrote: >>>>> Instead of invoking struct bpf_prog::bpf_func directly, use the >>>>> BPF_PROG_RUN macro. >>>>> >>>>> Signed-off-by: Sami Tolvanen <samitolvanen@google.com> >>>> >>>> Acked-by: Yonghong Song <yhs@fb.com> >>> >>> Applied. Thanks >> >> Then we probably need this as well, what do you think ? > > Yep, it's broken. 6cab5e90ab2b ("bpf: run bpf programs with preemption > disabled") probably forgot about it since it wasn't using BPF_PROG_RUN() > in the first place. If you get a chance, please send a proper patch, > thanks! Sure, I will send this today.
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index 5dbc0c48f8cb..f350c613bd7d 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -379,7 +379,7 @@ static int kcm_parse_func_strparser(struct strparser *strp, struct sk_buff *skb) struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp); struct bpf_prog *prog = psock->bpf_prog; - return (*prog->bpf_func)(skb, prog->insnsi); + return BPF_PROG_RUN(prog, skb); } static int kcm_read_sock_done(struct strparser *strp, int err)
Instead of invoking struct bpf_prog::bpf_func directly, use the BPF_PROG_RUN macro. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- net/kcm/kcmsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)