* [PATCH bpf 0/2] fix rmem incorrect accounting in bpf_tcp_ingress()
@ 2023-07-26 14:20 Liu Jian
2023-07-26 14:20 ` [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper Liu Jian
2023-07-26 14:20 ` [PATCH bpf 2/2] bpf, sockmap: fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
0 siblings, 2 replies; 6+ messages in thread
From: Liu Jian @ 2023-07-26 14:20 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, john.fastabend, jakub, dsahern,
ast, daniel, netdev, bpf
Cc: liujian56
fix rmem incorrect accounting in bpf_tcp_ingress()
Liu Jian (2):
net: introduce __sk_rmem_schedule() helper
bpf, sockmap: fix rmem incorrect accounting in bpf_tcp_ingress()
include/net/sock.h | 12 ++++++++----
net/ipv4/tcp_bpf.c | 2 +-
2 files changed, 9 insertions(+), 5 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper
2023-07-26 14:20 [PATCH bpf 0/2] fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
@ 2023-07-26 14:20 ` Liu Jian
2023-07-26 14:25 ` Eric Dumazet
2023-07-26 14:20 ` [PATCH bpf 2/2] bpf, sockmap: fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
1 sibling, 1 reply; 6+ messages in thread
From: Liu Jian @ 2023-07-26 14:20 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, john.fastabend, jakub, dsahern,
ast, daniel, netdev, bpf
Cc: liujian56
Compared with sk_wmem_schedule(), sk_rmem_schedule() not only performs
rmem accounting, but also checks skb_pfmemalloc. The __sk_rmem_schedule()
helper function is introduced here to perform only rmem accounting related
activities.
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
include/net/sock.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 2eb916d1ff64..58bf26c5c041 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1617,16 +1617,20 @@ static inline bool sk_wmem_schedule(struct sock *sk, int size)
return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_SEND);
}
-static inline bool
-sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
+static inline bool __sk_rmem_schedule(struct sock *sk, int size)
{
int delta;
if (!sk_has_account(sk))
return true;
delta = size - sk->sk_forward_alloc;
- return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV) ||
- skb_pfmemalloc(skb);
+ return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV);
+}
+
+static inline bool
+sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
+{
+ return __sk_rmem_schedule(sk, size) || skb_pfmemalloc(skb);
}
static inline int sk_unused_reserved_mem(const struct sock *sk)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf 2/2] bpf, sockmap: fix rmem incorrect accounting in bpf_tcp_ingress()
2023-07-26 14:20 [PATCH bpf 0/2] fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
2023-07-26 14:20 ` [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper Liu Jian
@ 2023-07-26 14:20 ` Liu Jian
1 sibling, 0 replies; 6+ messages in thread
From: Liu Jian @ 2023-07-26 14:20 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, john.fastabend, jakub, dsahern,
ast, daniel, netdev, bpf
Cc: liujian56
In the bpf_tcp_ingress function, the operation on parameter @sk is in the
recv direction. We should use __sk_rmem_schedule to calculate accounting.
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
net/ipv4/tcp_bpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 81f0dff69e0b..6eb9b320eecc 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -49,7 +49,7 @@ static int bpf_tcp_ingress(struct sock *sk, struct sk_psock *psock,
sge = sk_msg_elem(msg, i);
size = (apply && apply_bytes < sge->length) ?
apply_bytes : sge->length;
- if (!sk_wmem_schedule(sk, size)) {
+ if (!__sk_rmem_schedule(sk, size)) {
if (!copied)
ret = -ENOMEM;
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper
2023-07-26 14:20 ` [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper Liu Jian
@ 2023-07-26 14:25 ` Eric Dumazet
2023-07-27 19:16 ` John Fastabend
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2023-07-26 14:25 UTC (permalink / raw)
To: Liu Jian
Cc: davem, kuba, pabeni, john.fastabend, jakub, dsahern, ast, daniel,
netdev, bpf
On Wed, Jul 26, 2023 at 4:15 PM Liu Jian <liujian56@huawei.com> wrote:
>
> Compared with sk_wmem_schedule(), sk_rmem_schedule() not only performs
> rmem accounting, but also checks skb_pfmemalloc. The __sk_rmem_schedule()
> helper function is introduced here to perform only rmem accounting related
> activities.
>
Why not care about pfmemalloc ? Why is it safe ?
You need to give more details, or simply reuse the existing helper.
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
> include/net/sock.h | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 2eb916d1ff64..58bf26c5c041 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1617,16 +1617,20 @@ static inline bool sk_wmem_schedule(struct sock *sk, int size)
> return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_SEND);
> }
>
> -static inline bool
> -sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
> +static inline bool __sk_rmem_schedule(struct sock *sk, int size)
> {
> int delta;
>
> if (!sk_has_account(sk))
> return true;
> delta = size - sk->sk_forward_alloc;
> - return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV) ||
> - skb_pfmemalloc(skb);
> + return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV);
> +}
> +
> +static inline bool
> +sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
> +{
> + return __sk_rmem_schedule(sk, size) || skb_pfmemalloc(skb);
> }
>
> static inline int sk_unused_reserved_mem(const struct sock *sk)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper
2023-07-26 14:25 ` Eric Dumazet
@ 2023-07-27 19:16 ` John Fastabend
2023-07-28 12:36 ` liujian (CE)
0 siblings, 1 reply; 6+ messages in thread
From: John Fastabend @ 2023-07-27 19:16 UTC (permalink / raw)
To: Eric Dumazet, Liu Jian
Cc: davem, kuba, pabeni, john.fastabend, jakub, dsahern, ast, daniel,
netdev, bpf
Eric Dumazet wrote:
> On Wed, Jul 26, 2023 at 4:15 PM Liu Jian <liujian56@huawei.com> wrote:
> >
> > Compared with sk_wmem_schedule(), sk_rmem_schedule() not only performs
> > rmem accounting, but also checks skb_pfmemalloc. The __sk_rmem_schedule()
> > helper function is introduced here to perform only rmem accounting related
> > activities.
> >
>
> Why not care about pfmemalloc ? Why is it safe ?
>
> You need to give more details, or simply reuse the existing helper.
I would just use the existing helper. Seems it should be fine.
>
> > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > ---
> > include/net/sock.h | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 2eb916d1ff64..58bf26c5c041 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1617,16 +1617,20 @@ static inline bool sk_wmem_schedule(struct sock *sk, int size)
> > return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_SEND);
> > }
> >
> > -static inline bool
> > -sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
> > +static inline bool __sk_rmem_schedule(struct sock *sk, int size)
> > {
> > int delta;
> >
> > if (!sk_has_account(sk))
> > return true;
> > delta = size - sk->sk_forward_alloc;
> > - return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV) ||
> > - skb_pfmemalloc(skb);
> > + return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV);
> > +}
> > +
> > +static inline bool
> > +sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
> > +{
> > + return __sk_rmem_schedule(sk, size) || skb_pfmemalloc(skb);
> > }
> >
> > static inline int sk_unused_reserved_mem(const struct sock *sk)
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper
2023-07-27 19:16 ` John Fastabend
@ 2023-07-28 12:36 ` liujian (CE)
0 siblings, 0 replies; 6+ messages in thread
From: liujian (CE) @ 2023-07-28 12:36 UTC (permalink / raw)
To: John Fastabend, Eric Dumazet
Cc: davem, kuba, pabeni, jakub, dsahern, ast, daniel, netdev, bpf
On 2023/7/28 3:16, John Fastabend wrote:
> Eric Dumazet wrote:
>> On Wed, Jul 26, 2023 at 4:15 PM Liu Jian <liujian56@huawei.com> wrote:
>>>
>>> Compared with sk_wmem_schedule(), sk_rmem_schedule() not only performs
>>> rmem accounting, but also checks skb_pfmemalloc. The __sk_rmem_schedule()
>>> helper function is introduced here to perform only rmem accounting related
>>> activities.
>>>
>>
>> Why not care about pfmemalloc ? Why is it safe ?
>>
>> You need to give more details, or simply reuse the existing helper.
I didn't think so much. I extracted this helper just to do rmem
accounting in bpf_tcp_ingress(), because I didn't see the pfmemalloc
flag set when alloc sk_msg in sk_msg_alloc(). And thanks for your review.
>
> I would just use the existing helper. Seems it should be fine.
ok, let's just leave it as it is. Thanks John.
>
>>
>>> Signed-off-by: Liu Jian <liujian56@huawei.com>
>>> ---
>>> include/net/sock.h | 12 ++++++++----
>>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/net/sock.h b/include/net/sock.h
>>> index 2eb916d1ff64..58bf26c5c041 100644
>>> --- a/include/net/sock.h
>>> +++ b/include/net/sock.h
>>> @@ -1617,16 +1617,20 @@ static inline bool sk_wmem_schedule(struct sock *sk, int size)
>>> return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_SEND);
>>> }
>>>
>>> -static inline bool
>>> -sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
>>> +static inline bool __sk_rmem_schedule(struct sock *sk, int size)
>>> {
>>> int delta;
>>>
>>> if (!sk_has_account(sk))
>>> return true;
>>> delta = size - sk->sk_forward_alloc;
>>> - return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV) ||
>>> - skb_pfmemalloc(skb);
>>> + return delta <= 0 || __sk_mem_schedule(sk, delta, SK_MEM_RECV);
>>> +}
>>> +
>>> +static inline bool
>>> +sk_rmem_schedule(struct sock *sk, struct sk_buff *skb, int size)
>>> +{
>>> + return __sk_rmem_schedule(sk, size) || skb_pfmemalloc(skb);
>>> }
>>>
>>> static inline int sk_unused_reserved_mem(const struct sock *sk)
>>> --
>>> 2.34.1
>>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-28 12:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-26 14:20 [PATCH bpf 0/2] fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
2023-07-26 14:20 ` [PATCH bpf 1/2] net: introduce __sk_rmem_schedule() helper Liu Jian
2023-07-26 14:25 ` Eric Dumazet
2023-07-27 19:16 ` John Fastabend
2023-07-28 12:36 ` liujian (CE)
2023-07-26 14:20 ` [PATCH bpf 2/2] bpf, sockmap: fix rmem incorrect accounting in bpf_tcp_ingress() Liu Jian
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).