2019-09-06 08:19:17

by Sami Tolvanen

Subject: [PATCH] kcm: use BPF_PROG_RUN

Instead of invoking struct bpf_prog::bpf_func directly, use the
BPF_PROG_RUN macro.

Signed-off-by: Sami Tolvanen <[email protected]>
---
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)
--
2.23.0.187.g17f5b7556c-goog

2019-09-06 10:04:55

by Yonghong Song

Subject: Re: [PATCH] kcm: use BPF_PROG_RUN

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 <[email protected]>

Acked-by: Yonghong Song <[email protected]>

> ---
> 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)
>

2019-09-07 09:30:26

by Alexei Starovoitov

Subject: Re: [PATCH] kcm: use BPF_PROG_RUN

On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <[email protected]> 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 <[email protected]>
>
> Acked-by: Yonghong Song <[email protected]>

Applied. Thanks

2019-09-25 23:58:57

by Eric Dumazet

Subject: Re: [PATCH] kcm: use BPF_PROG_RUN

On 9/6/19 10:06 AM, Alexei Starovoitov wrote:
> On Fri, Sep 6, 2019 at 3:03 AM Yonghong Song <[email protected]> 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 <[email protected]>
>>
>> Acked-by: Yonghong Song <[email protected]>
>
> 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)

2019-09-26 08:42:00

by Eric Dumazet

Subject: Re: [PATCH] kcm: use BPF_PROG_RUN

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 <[email protected]> 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 <[email protected]>
>>>>
>>>> Acked-by: Yonghong Song <[email protected]>
>>>
>>> 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.

2019-09-26 08:42:35

by Daniel Borkmann

Subject: Re: [PATCH] kcm: use BPF_PROG_RUN

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 <[email protected]> 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 <[email protected]>
> >>
> >> Acked-by: Yonghong Song <[email protected]>
> >
> > 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)