* [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
* 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
* [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
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