* [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated.
@ 2025-08-29 1:00 Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept() Kuniyuki Iwashima
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
Some protocols (e.g., TCP, UDP) have their own memory accounting for
socket buffers and charge memory to global per-protocol counters such
as /proc/net/ipv4/tcp_mem.
When running under a non-root cgroup, this memory is also charged to
the memcg as sock in memory.stat.
Sockets of such protocols are still subject to the global limits,
thus affected by a noisy neighbour outside cgroup.
This makes it difficult to accurately estimate and configure appropriate
global limits.
This series allows decoupling memcg from the global memory accounting
if socket is configured as such by BPF prog.
This simplifies the memcg configuration while keeping the global limits
within a reasonable range, which is only 10% of the physical memory by
default.
Overview of the series:
patch 1 & 2 are prep
patch 3 intorduces SK_BPF_MEMCG_SOCK_ISOLATED for bpf_setsockopt()
patch 4 decouples memcg from sk_prot->memory_allocated based on the flag
patch 5 is selftest
Changes:
v4:
* Patch 2
* Use __bpf_setsockopt() instead of _bpf_setsockopt()
* Add getsockopt() for a cgroup with multiple bpf progs running
* Patch 3
* Only allow inet_create() to set flags
* Inherit flags from listener to child in sk_clone_lock()
* Support clearing flags
* Patch 5
* Only use inet_create() hook
* Test bpf_getsockopt()
* Add serial_ prefix
* Reduce sleep() and the amount of sent data
v3: https://lore.kernel.org/netdev/20250826183940.3310118-1-kuniyu@google.com/
* Drop patches for accept() hook
* Patch 1
* Merge if blocks
* Patch2
* Drop bpf_func_proto for accept()
* Patch 3
* Allow flagging without sk->sk_memcg
* Inherit SK_BPF_MEMCG_SOCK_ISOLATED in __inet_accept()
v2: https://lore.kernel.org/bpf/20250825204158.2414402-1-kuniyu@google.com/
* Patch 2
* Define BPF_CGROUP_RUN_PROG_INET_SOCK_ACCEPT() when CONFIG_CGROUP_BPF=n
* Patch 5
* Make 2 new bpf_func_proto static
* Patch 6
* s/mem_cgroup_sk_set_flag/mem_cgroup_sk_set_flags/ when CONFIG_MEMCG=n
* Use finer CONFIG_CGROUP_BPF instead of CONFIG_BPF_SYSCALL for ifdef
v1: https://lore.kernel.org/netdev/20250822221846.744252-1-kuniyu@google.com/
Kuniyuki Iwashima (5):
tcp: Save lock_sock() for memcg in inet_csk_accept().
bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE.
bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED.
net-memcg: Allow decoupling memcg from global protocol memory
accounting.
selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED.
include/net/proto_memory.h | 15 +-
include/net/sock.h | 50 ++++
include/net/tcp.h | 10 +-
include/uapi/linux/bpf.h | 6 +
net/core/filter.c | 91 +++++++-
net/core/sock.c | 65 ++++--
net/ipv4/af_inet.c | 37 +++
net/ipv4/inet_connection_sock.c | 26 +--
net/ipv4/tcp.c | 3 +-
net/ipv4/tcp_output.c | 10 +-
net/mptcp/protocol.c | 3 +-
net/tls/tls_device.c | 4 +-
tools/include/uapi/linux/bpf.h | 6 +
.../selftests/bpf/prog_tests/sk_memcg.c | 218 ++++++++++++++++++
tools/testing/selftests/bpf/progs/sk_memcg.c | 38 +++
15 files changed, 525 insertions(+), 57 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/sk_memcg.c
create mode 100644 tools/testing/selftests/bpf/progs/sk_memcg.c
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept().
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
@ 2025-08-29 1:00 ` Kuniyuki Iwashima
2025-09-02 18:55 ` Martin KaFai Lau
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE Kuniyuki Iwashima
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
If memcg is enabled, accept() acquires lock_sock() twice for each new
TCP/MPTCP socket in inet_csk_accept() and __inet_accept().
Let's move memcg operations from inet_csk_accept() to __inet_accept().
Note that SCTP somehow allocates a new socket by sk_alloc() in
sk->sk_prot->accept() and clones fields manually, instead of using
sk_clone_lock().
mem_cgroup_sk_alloc() is called for SCTP before __inet_accept(),
so I added the protocol check in __inet_accept(), but this can be
removed once SCTP uses sk_clone_lock().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
---
v3: Don't split if blocks
---
net/ipv4/af_inet.c | 23 +++++++++++++++++++++++
net/ipv4/inet_connection_sock.c | 25 -------------------------
2 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 76e38092cd8a..d42757f74c6e 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -753,6 +753,29 @@ EXPORT_SYMBOL(inet_stream_connect);
void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *newsk)
{
+ /* TODO: use sk_clone_lock() in SCTP and remove protocol checks */
+ if (mem_cgroup_sockets_enabled &&
+ (!IS_ENABLED(CONFIG_IP_SCTP) ||
+ sk_is_tcp(newsk) || sk_is_mptcp(newsk))) {
+ gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
+
+ mem_cgroup_sk_alloc(newsk);
+
+ if (mem_cgroup_from_sk(newsk)) {
+ int amt;
+
+ /* The socket has not been accepted yet, no need
+ * to look at newsk->sk_wmem_queued.
+ */
+ amt = sk_mem_pages(newsk->sk_forward_alloc +
+ atomic_read(&newsk->sk_rmem_alloc));
+ if (amt)
+ mem_cgroup_sk_charge(newsk, amt, gfp);
+ }
+
+ kmem_cache_charge(newsk, gfp);
+ }
+
sock_rps_record_flow(newsk);
WARN_ON(!((1 << newsk->sk_state) &
(TCPF_ESTABLISHED | TCPF_SYN_RECV |
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 0ef1eacd539d..ed10b959a906 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -708,31 +708,6 @@ struct sock *inet_csk_accept(struct sock *sk, struct proto_accept_arg *arg)
release_sock(sk);
- if (mem_cgroup_sockets_enabled) {
- gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
- int amt = 0;
-
- /* atomically get the memory usage, set and charge the
- * newsk->sk_memcg.
- */
- lock_sock(newsk);
-
- mem_cgroup_sk_alloc(newsk);
- if (mem_cgroup_from_sk(newsk)) {
- /* The socket has not been accepted yet, no need
- * to look at newsk->sk_wmem_queued.
- */
- amt = sk_mem_pages(newsk->sk_forward_alloc +
- atomic_read(&newsk->sk_rmem_alloc));
- }
-
- if (amt)
- mem_cgroup_sk_charge(newsk, amt, gfp);
- kmem_cache_charge(newsk, gfp);
-
- release_sock(newsk);
- }
-
if (req)
reqsk_put(req);
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE.
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept() Kuniyuki Iwashima
@ 2025-08-29 1:00 ` Kuniyuki Iwashima
2025-09-02 19:10 ` Martin KaFai Lau
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
We will store a flag in sk->sk_memcg by bpf_setsockopt() at the
BPF_CGROUP_INET_SOCK_CREATE hook.
BPF_CGROUP_INET_SOCK_CREATE is invoked by __cgroup_bpf_run_filter_sk()
that passes a pointer to struct sock to the bpf prog as void *ctx.
But there are no bpf_func_proto for bpf_setsockopt() that receives
the ctx as a pointer to struct sock.
Also, bpf_getsockopt() will be necessary for a cgroup with multiple
bpf progs running.
Let's add new bpf_setsockopt() and bpf_getsockopt() variants for
BPF_CGROUP_INET_SOCK_CREATE.
Note that inet_create() is not under lock_sock() and has the same
semantics with bpf_lsm_unlocked_sockopt_hooks.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v4:
* Use __bpf_setsockopt() instead of _bpf_setsockopt()
* Add getsockopt() for a cgroup with multiple bpf progs running
v3: Remove bpf_func_proto for accept()
v2: Make 2 new bpf_func_proto static
---
net/core/filter.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 63f3baee2daf..b6d514039cf8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5723,6 +5723,40 @@ static const struct bpf_func_proto bpf_sock_addr_getsockopt_proto = {
.arg5_type = ARG_CONST_SIZE,
};
+BPF_CALL_5(bpf_unlocked_sock_setsockopt, struct sock *, sk, int, level,
+ int, optname, char *, optval, int, optlen)
+{
+ return __bpf_setsockopt(sk, level, optname, optval, optlen);
+}
+
+static const struct bpf_func_proto bpf_unlocked_sock_setsockopt_proto = {
+ .func = bpf_unlocked_sock_setsockopt,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY,
+ .arg5_type = ARG_CONST_SIZE,
+};
+
+BPF_CALL_5(bpf_unlocked_sock_getsockopt, struct sock *, sk, int, level,
+ int, optname, char *, optval, int, optlen)
+{
+ return __bpf_getsockopt(sk, level, optname, optval, optlen);
+}
+
+static const struct bpf_func_proto bpf_unlocked_sock_getsockopt_proto = {
+ .func = bpf_unlocked_sock_getsockopt,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg5_type = ARG_CONST_SIZE,
+};
+
BPF_CALL_5(bpf_sock_ops_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
int, level, int, optname, char *, optval, int, optlen)
{
@@ -8051,6 +8085,20 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_get_cg_sock_proto;
case BPF_FUNC_ktime_get_coarse_ns:
return &bpf_ktime_get_coarse_ns_proto;
+ case BPF_FUNC_setsockopt:
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_INET_SOCK_CREATE:
+ return &bpf_unlocked_sock_setsockopt_proto;
+ default:
+ return NULL;
+ }
+ case BPF_FUNC_getsockopt:
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_INET_SOCK_CREATE:
+ return &bpf_unlocked_sock_getsockopt_proto;
+ default:
+ return NULL;
+ }
default:
return bpf_base_func_proto(func_id, prog);
}
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED.
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept() Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE Kuniyuki Iwashima
@ 2025-08-29 1:00 ` Kuniyuki Iwashima
2025-09-02 20:02 ` Martin KaFai Lau
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
4 siblings, 1 reply; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
We will decouple sockets from the global protocol memory accounting
if sockets have SK_BPF_MEMCG_SOCK_ISOLATED.
This can be flagged (and cleared) at the BPF_CGROUP_INET_SOCK_CREATE
hook by bpf_setsockopt() and is inherited to child sockets.
u32 flags = SK_BPF_MEMCG_SOCK_ISOLATED;
bpf_setsockopt(ctx, SOL_SOCKET, SK_BPF_MEMCG_FLAGS,
&flags, sizeof(flags));
bpf_setsockopt(SK_BPF_MEMCG_FLAGS) is only supported at
bpf_unlocked_sock_setsockopt() and not supported on other hooks
for some reasons:
1. UDP charges memory under sk->sk_receive_queue.lock instead
of lock_sock()
2. For TCP child sockets, memory accounting is adjusted only in
__inet_accept() which sk->sk_memcg allocation is deferred to
3. Modifying the flag after skb is charged to sk requires such
adjustment during bpf_setsockopt() and complicates the logic
unnecessarily
We can support other hooks later if a real use case justifies that.
OTOH, bpf_getsockopt() is supported on other hooks, e.g. bpf_iter,
for the debugging purpose.
Given sk->sk_memcg can be accessed in the fast path, it would
be preferable to place the flag field in the same cache line as
sk->sk_memcg.
However, struct sock does not have such a 1-byte hole.
Let's store the flag in the lowest bit of sk->sk_memcg and add
a helper to check the bit.
In the next patch, if mem_cgroup_sk_isolated() returns true,
the socket will not be charged to sk->sk_prot->memory_allocated.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v4:
* Only allow inet_create() to set flags
* Inherit flags from listener to child in sk_clone_lock()
* Support clearing flags
v3:
* Allow setting flags without sk->sk_memcg in sk_bpf_set_get_memcg_flags()
* Preserve flags in __inet_accept()
v2:
* s/mem_cgroup_sk_set_flag/mem_cgroup_sk_set_flags/ when CONFIG_MEMCG=n
* Use CONFIG_CGROUP_BPF instead of CONFIG_BPF_SYSCALL for ifdef
---
include/net/sock.h | 50 ++++++++++++++++++++++++++++++++++
include/uapi/linux/bpf.h | 6 ++++
net/core/filter.c | 43 ++++++++++++++++++++++++++++-
net/core/sock.c | 1 +
net/ipv4/af_inet.c | 4 +++
tools/include/uapi/linux/bpf.h | 6 ++++
6 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 63a6a48afb48..703cb9116c6e 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2596,10 +2596,41 @@ static inline gfp_t gfp_memcg_charge(void)
return in_softirq() ? GFP_ATOMIC : GFP_KERNEL;
}
+#define SK_BPF_MEMCG_FLAG_MASK (SK_BPF_MEMCG_FLAG_MAX - 1)
+#define SK_BPF_MEMCG_PTR_MASK ~SK_BPF_MEMCG_FLAG_MASK
+
#ifdef CONFIG_MEMCG
static inline struct mem_cgroup *mem_cgroup_from_sk(const struct sock *sk)
{
+#ifdef CONFIG_CGROUP_BPF
+ unsigned long val = (unsigned long)sk->sk_memcg;
+
+ val &= SK_BPF_MEMCG_PTR_MASK;
+ return (struct mem_cgroup *)val;
+#else
return sk->sk_memcg;
+#endif
+}
+
+static inline void mem_cgroup_sk_set_flags(struct sock *sk, unsigned short flags)
+{
+#ifdef CONFIG_CGROUP_BPF
+ unsigned long val = (unsigned long)mem_cgroup_from_sk(sk);
+
+ val |= flags;
+ sk->sk_memcg = (struct mem_cgroup *)val;
+#endif
+}
+
+static inline unsigned short mem_cgroup_sk_get_flags(const struct sock *sk)
+{
+#ifdef CONFIG_CGROUP_BPF
+ unsigned long val = (unsigned long)sk->sk_memcg;
+
+ return val & SK_BPF_MEMCG_FLAG_MASK;
+#else
+ return 0;
+#endif
}
static inline bool mem_cgroup_sk_enabled(const struct sock *sk)
@@ -2607,6 +2638,11 @@ static inline bool mem_cgroup_sk_enabled(const struct sock *sk)
return mem_cgroup_sockets_enabled && mem_cgroup_from_sk(sk);
}
+static inline bool mem_cgroup_sk_isolated(const struct sock *sk)
+{
+ return mem_cgroup_sk_get_flags(sk) & SK_BPF_MEMCG_SOCK_ISOLATED;
+}
+
static inline bool mem_cgroup_sk_under_memory_pressure(const struct sock *sk)
{
struct mem_cgroup *memcg = mem_cgroup_from_sk(sk);
@@ -2629,11 +2665,25 @@ static inline struct mem_cgroup *mem_cgroup_from_sk(const struct sock *sk)
return NULL;
}
+static inline void mem_cgroup_sk_set_flags(struct sock *sk, unsigned short flags)
+{
+}
+
+static inline unsigned short mem_cgroup_sk_get_flags(const struct sock *sk)
+{
+ return 0;
+}
+
static inline bool mem_cgroup_sk_enabled(const struct sock *sk)
{
return false;
}
+static inline bool mem_cgroup_sk_isolated(const struct sock *sk)
+{
+ return false;
+}
+
static inline bool mem_cgroup_sk_under_memory_pressure(const struct sock *sk)
{
return false;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 233de8677382..52b8c2278589 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -7182,6 +7182,7 @@ enum {
TCP_BPF_SYN_MAC = 1007, /* Copy the MAC, IP[46], and TCP header */
TCP_BPF_SOCK_OPS_CB_FLAGS = 1008, /* Get or Set TCP sock ops flags */
SK_BPF_CB_FLAGS = 1009, /* Get or set sock ops flags in socket */
+ SK_BPF_MEMCG_FLAGS = 1010, /* Get or Set flags saved in sk->sk_memcg */
};
enum {
@@ -7204,6 +7205,11 @@ enum {
*/
};
+enum {
+ SK_BPF_MEMCG_SOCK_ISOLATED = (1UL << 0),
+ SK_BPF_MEMCG_FLAG_MAX = (1UL << 1),
+};
+
struct bpf_perf_event_value {
__u64 counter;
__u64 enabled;
diff --git a/net/core/filter.c b/net/core/filter.c
index b6d514039cf8..eb2f87a732ef 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5267,6 +5267,35 @@ static int sk_bpf_set_get_cb_flags(struct sock *sk, char *optval, bool getopt)
return 0;
}
+static int sk_bpf_get_memcg_flags(struct sock *sk, char *optval)
+{
+ if (!sk_has_account(sk))
+ return -EOPNOTSUPP;
+
+ *(u32 *)optval = mem_cgroup_sk_get_flags(sk);
+
+ return 0;
+}
+
+static int sk_bpf_set_memcg_flags(struct sock *sk, char *optval, int optlen)
+{
+ u32 flags;
+
+ if (optlen != sizeof(u32))
+ return -EINVAL;
+
+ if (!sk_has_account(sk))
+ return -EOPNOTSUPP;
+
+ flags = *(u32 *)optval;
+ if (flags >= SK_BPF_MEMCG_FLAG_MAX)
+ return -EINVAL;
+
+ mem_cgroup_sk_set_flags(sk, flags);
+
+ return 0;
+}
+
static int sol_socket_sockopt(struct sock *sk, int optname,
char *optval, int *optlen,
bool getopt)
@@ -5284,6 +5313,7 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
case SO_BINDTOIFINDEX:
case SO_TXREHASH:
case SK_BPF_CB_FLAGS:
+ case SK_BPF_MEMCG_FLAGS:
if (*optlen != sizeof(int))
return -EINVAL;
break;
@@ -5293,8 +5323,15 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
return -EINVAL;
}
- if (optname == SK_BPF_CB_FLAGS)
+ switch (optname) {
+ case SK_BPF_CB_FLAGS:
return sk_bpf_set_get_cb_flags(sk, optval, getopt);
+ case SK_BPF_MEMCG_FLAGS:
+ if (!IS_ENABLED(CONFIG_MEMCG) || !getopt)
+ return -EOPNOTSUPP;
+
+ return sk_bpf_get_memcg_flags(sk, optval);
+ }
if (getopt) {
if (optname == SO_BINDTODEVICE)
@@ -5726,6 +5763,10 @@ static const struct bpf_func_proto bpf_sock_addr_getsockopt_proto = {
BPF_CALL_5(bpf_unlocked_sock_setsockopt, struct sock *, sk, int, level,
int, optname, char *, optval, int, optlen)
{
+ if (IS_ENABLED(CONFIG_MEMCG) &&
+ level == SOL_SOCKET && optname == SK_BPF_MEMCG_FLAGS)
+ return sk_bpf_set_memcg_flags(sk, optval, optlen);
+
return __bpf_setsockopt(sk, level, optname, optval, optlen);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 8002ac6293dc..ae30d7d54498 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2515,6 +2515,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
#ifdef CONFIG_MEMCG
/* sk->sk_memcg will be populated at accept() time */
newsk->sk_memcg = NULL;
+ mem_cgroup_sk_set_flags(newsk, mem_cgroup_sk_get_flags(sk));
#endif
cgroup_sk_clone(&newsk->sk_cgrp_data);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d42757f74c6e..9b62f1ae13ba 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -758,12 +758,16 @@ void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *new
(!IS_ENABLED(CONFIG_IP_SCTP) ||
sk_is_tcp(newsk) || sk_is_mptcp(newsk))) {
gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
+ unsigned short flags;
+ flags = mem_cgroup_sk_get_flags(newsk);
mem_cgroup_sk_alloc(newsk);
if (mem_cgroup_from_sk(newsk)) {
int amt;
+ mem_cgroup_sk_set_flags(newsk, flags);
+
/* The socket has not been accepted yet, no need
* to look at newsk->sk_wmem_queued.
*/
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 233de8677382..52b8c2278589 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -7182,6 +7182,7 @@ enum {
TCP_BPF_SYN_MAC = 1007, /* Copy the MAC, IP[46], and TCP header */
TCP_BPF_SOCK_OPS_CB_FLAGS = 1008, /* Get or Set TCP sock ops flags */
SK_BPF_CB_FLAGS = 1009, /* Get or set sock ops flags in socket */
+ SK_BPF_MEMCG_FLAGS = 1010, /* Get or Set flags saved in sk->sk_memcg */
};
enum {
@@ -7204,6 +7205,11 @@ enum {
*/
};
+enum {
+ SK_BPF_MEMCG_SOCK_ISOLATED = (1UL << 0),
+ SK_BPF_MEMCG_FLAG_MAX = (1UL << 1),
+};
+
struct bpf_perf_event_value {
__u64 counter;
__u64 enabled;
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting.
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
` (2 preceding siblings ...)
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
@ 2025-08-29 1:00 ` Kuniyuki Iwashima
2025-09-02 20:16 ` Martin KaFai Lau
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
4 siblings, 1 reply; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
Some protocols (e.g., TCP, UDP) implement memory accounting for socket
buffers and charge memory to per-protocol global counters pointed to by
sk->sk_proto->memory_allocated.
When running under a non-root cgroup, this memory is also charged to the
memcg as "sock" in memory.stat.
Even when a memcg controls memory usage, sockets of such protocols are
still subject to global limits (e.g., /proc/sys/net/ipv4/tcp_mem).
This makes it difficult to accurately estimate and configure appropriate
global limits, especially in multi-tenant environments.
If all workloads were guaranteed to be controlled under memcg, the issue
could be worked around by setting tcp_mem[0~2] to UINT_MAX.
In reality, this assumption does not always hold, and processes not
controlled by memcg lose the seatbelt and can consume memory up to
the global limit, becoming noisy neighbour.
Let's decouple sockets in memcg from the global per-protocol memory
accounting if sockets have SK_BPF_MEMCG_SOCK_ISOLATED in sk->sk_memcg.
This simplifies memcg configuration while keeping the global limits
within a reasonable range.
If mem_cgroup_sk_isolated(sk) returns true, the per-protocol memory
accounting is skipped.
In __inet_accept(), we need to reclaim counts that are already charged
for child sockets because we do not allocate sk->sk_memcg until accept().
Note that trace_sock_exceed_buf_limit() will always show 0 as accounted
for the isolated sockets, but this can be obtained via memory.stat.
Tested with a script that creates local socket pairs and send()s a
bunch of data without recv()ing.
Setup:
# mkdir /sys/fs/cgroup/test
# echo $$ >> /sys/fs/cgroup/test/cgroup.procs
# sysctl -q net.ipv4.tcp_mem="1000 1000 1000"
Without bpf prog:
# prlimit -n=524288:524288 bash -c "python3 pressure.py" &
# cat /sys/fs/cgroup/test/memory.stat | grep sock
sock 22642688
# cat /proc/net/sockstat| grep TCP
TCP: inuse 2006 orphan 0 tw 0 alloc 2008 mem 5376
# ss -tn | head -n 5
State Recv-Q Send-Q Local Address:Port Peer Address:Port
ESTAB 2000 0 127.0.0.1:34479 127.0.0.1:53188
ESTAB 2000 0 127.0.0.1:34479 127.0.0.1:49972
ESTAB 2000 0 127.0.0.1:34479 127.0.0.1:53868
ESTAB 2000 0 127.0.0.1:34479 127.0.0.1:53554
# nstat | grep Pressure || echo no pressure
TcpExtTCPMemoryPressures 1 0.0
With bpf prog in the next patch:
# bpftool prog load sk_memcg.bpf.o /sys/fs/bpf/sk_memcg type cgroup/sock_create
# bpftool cgroup attach /sys/fs/cgroup/test cgroup_inet_sock_create pinned /sys/fs/bpf/sk_memcg
# prlimit -n=524288:524288 bash -c "python3 pressure.py" &
# cat /sys/fs/cgroup/test/memory.stat | grep sock
sock 2757468160
# cat /proc/net/sockstat | grep TCP
TCP: inuse 2006 orphan 0 tw 0 alloc 2008 mem 0
# ss -tn | head -n 5
State Recv-Q Send-Q Local Address:Port Peer Address:Port
ESTAB 111000 0 127.0.0.1:36019 127.0.0.1:49026
ESTAB 110000 0 127.0.0.1:36019 127.0.0.1:45630
ESTAB 110000 0 127.0.0.1:36019 127.0.0.1:44870
ESTAB 111000 0 127.0.0.1:36019 127.0.0.1:45274
# nstat | grep Pressure || echo no pressure
no pressure
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
include/net/proto_memory.h | 15 ++++++--
include/net/tcp.h | 10 ++++--
net/core/sock.c | 64 ++++++++++++++++++++++-----------
net/ipv4/af_inet.c | 12 ++++++-
net/ipv4/inet_connection_sock.c | 1 +
net/ipv4/tcp.c | 3 +-
net/ipv4/tcp_output.c | 10 ++++--
net/mptcp/protocol.c | 3 +-
net/tls/tls_device.c | 4 ++-
9 files changed, 90 insertions(+), 32 deletions(-)
diff --git a/include/net/proto_memory.h b/include/net/proto_memory.h
index 8e91a8fa31b5..8e8432b13515 100644
--- a/include/net/proto_memory.h
+++ b/include/net/proto_memory.h
@@ -31,13 +31,22 @@ static inline bool sk_under_memory_pressure(const struct sock *sk)
if (!sk->sk_prot->memory_pressure)
return false;
- if (mem_cgroup_sk_enabled(sk) &&
- mem_cgroup_sk_under_memory_pressure(sk))
- return true;
+ if (mem_cgroup_sk_enabled(sk)) {
+ if (mem_cgroup_sk_under_memory_pressure(sk))
+ return true;
+
+ if (mem_cgroup_sk_isolated(sk))
+ return false;
+ }
return !!READ_ONCE(*sk->sk_prot->memory_pressure);
}
+static inline bool sk_should_enter_memory_pressure(struct sock *sk)
+{
+ return !mem_cgroup_sk_enabled(sk) || !mem_cgroup_sk_isolated(sk);
+}
+
static inline long
proto_memory_allocated(const struct proto *prot)
{
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2936b8175950..0191a4585bba 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -275,9 +275,13 @@ extern unsigned long tcp_memory_pressure;
/* optimized version of sk_under_memory_pressure() for TCP sockets */
static inline bool tcp_under_memory_pressure(const struct sock *sk)
{
- if (mem_cgroup_sk_enabled(sk) &&
- mem_cgroup_sk_under_memory_pressure(sk))
- return true;
+ if (mem_cgroup_sk_enabled(sk)) {
+ if (mem_cgroup_sk_under_memory_pressure(sk))
+ return true;
+
+ if (mem_cgroup_sk_isolated(sk))
+ return false;
+ }
return READ_ONCE(tcp_memory_pressure);
}
diff --git a/net/core/sock.c b/net/core/sock.c
index ae30d7d54498..bda1655fa9bf 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1046,17 +1046,21 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
if (!charged)
return -ENOMEM;
- /* pre-charge to forward_alloc */
- sk_memory_allocated_add(sk, pages);
- allocated = sk_memory_allocated(sk);
- /* If the system goes into memory pressure with this
- * precharge, give up and return error.
- */
- if (allocated > sk_prot_mem_limits(sk, 1)) {
- sk_memory_allocated_sub(sk, pages);
- mem_cgroup_sk_uncharge(sk, pages);
- return -ENOMEM;
+ if (!mem_cgroup_sk_isolated(sk)) {
+ /* pre-charge to forward_alloc */
+ sk_memory_allocated_add(sk, pages);
+ allocated = sk_memory_allocated(sk);
+
+ /* If the system goes into memory pressure with this
+ * precharge, give up and return error.
+ */
+ if (allocated > sk_prot_mem_limits(sk, 1)) {
+ sk_memory_allocated_sub(sk, pages);
+ mem_cgroup_sk_uncharge(sk, pages);
+ return -ENOMEM;
+ }
}
+
sk_forward_alloc_add(sk, pages << PAGE_SHIFT);
WRITE_ONCE(sk->sk_reserved_mem,
@@ -3154,8 +3158,11 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
return true;
- sk_enter_memory_pressure(sk);
+ if (sk_should_enter_memory_pressure(sk))
+ sk_enter_memory_pressure(sk);
+
sk_stream_moderate_sndbuf(sk);
+
return false;
}
EXPORT_SYMBOL(sk_page_frag_refill);
@@ -3268,18 +3275,30 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
{
bool memcg_enabled = false, charged = false;
struct proto *prot = sk->sk_prot;
- long allocated;
-
- sk_memory_allocated_add(sk, amt);
- allocated = sk_memory_allocated(sk);
+ long allocated = 0;
if (mem_cgroup_sk_enabled(sk)) {
+ bool isolated = mem_cgroup_sk_isolated(sk);
+
memcg_enabled = true;
charged = mem_cgroup_sk_charge(sk, amt, gfp_memcg_charge());
- if (!charged)
+
+ if (isolated && charged)
+ return 1;
+
+ if (!charged) {
+ if (!isolated) {
+ sk_memory_allocated_add(sk, amt);
+ allocated = sk_memory_allocated(sk);
+ }
+
goto suppress_allocation;
+ }
}
+ sk_memory_allocated_add(sk, amt);
+ allocated = sk_memory_allocated(sk);
+
/* Under limit. */
if (allocated <= sk_prot_mem_limits(sk, 0)) {
sk_leave_memory_pressure(sk);
@@ -3358,7 +3377,8 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
trace_sock_exceed_buf_limit(sk, prot, allocated, kind);
- sk_memory_allocated_sub(sk, amt);
+ if (allocated)
+ sk_memory_allocated_sub(sk, amt);
if (charged)
mem_cgroup_sk_uncharge(sk, amt);
@@ -3397,11 +3417,15 @@ EXPORT_SYMBOL(__sk_mem_schedule);
*/
void __sk_mem_reduce_allocated(struct sock *sk, int amount)
{
- sk_memory_allocated_sub(sk, amount);
-
- if (mem_cgroup_sk_enabled(sk))
+ if (mem_cgroup_sk_enabled(sk)) {
mem_cgroup_sk_uncharge(sk, amount);
+ if (mem_cgroup_sk_isolated(sk))
+ return;
+ }
+
+ sk_memory_allocated_sub(sk, amount);
+
if (sk_under_global_memory_pressure(sk) &&
(sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)))
sk_leave_memory_pressure(sk);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 9b62f1ae13ba..adbc8bcb760b 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -95,6 +95,7 @@
#include <net/checksum.h>
#include <net/ip.h>
#include <net/protocol.h>
+#include <net/proto_memory.h>
#include <net/arp.h>
#include <net/route.h>
#include <net/ip_fib.h>
@@ -773,8 +774,17 @@ void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *new
*/
amt = sk_mem_pages(newsk->sk_forward_alloc +
atomic_read(&newsk->sk_rmem_alloc));
- if (amt)
+ if (amt) {
+ /* This amt is already charged globally to
+ * sk_prot->memory_allocated due to lack of
+ * sk_memcg until accept(), thus we need to
+ * reclaim it here if newsk is isolated.
+ */
+ if (mem_cgroup_sk_isolated(newsk))
+ sk_memory_allocated_sub(newsk, amt);
+
mem_cgroup_sk_charge(newsk, amt, gfp);
+ }
}
kmem_cache_charge(newsk, gfp);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index ed10b959a906..f8dd53d40dcf 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -22,6 +22,7 @@
#include <net/tcp.h>
#include <net/sock_reuseport.h>
#include <net/addrconf.h>
+#include <net/proto_memory.h>
#if IS_ENABLED(CONFIG_IPV6)
/* match_sk*_wildcard == true: IPV6_ADDR_ANY equals to any IPv6 addresses
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 71a956fbfc55..dcbd49e2f8af 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -908,7 +908,8 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, gfp_t gfp,
}
__kfree_skb(skb);
} else {
- sk->sk_prot->enter_memory_pressure(sk);
+ if (sk_should_enter_memory_pressure(sk))
+ tcp_enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
}
return NULL;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index dfbac0876d96..f7aa86661219 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3574,12 +3574,18 @@ void sk_forced_mem_schedule(struct sock *sk, int size)
delta = size - sk->sk_forward_alloc;
if (delta <= 0)
return;
+
amt = sk_mem_pages(delta);
sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
- sk_memory_allocated_add(sk, amt);
- if (mem_cgroup_sk_enabled(sk))
+ if (mem_cgroup_sk_enabled(sk)) {
mem_cgroup_sk_charge(sk, amt, gfp_memcg_charge() | __GFP_NOFAIL);
+
+ if (mem_cgroup_sk_isolated(sk))
+ return;
+ }
+
+ sk_memory_allocated_add(sk, amt);
}
/* Send a FIN. The caller locks the socket for us.
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9a287b75c1b3..f7487e22a3f8 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -16,6 +16,7 @@
#include <net/inet_common.h>
#include <net/inet_hashtables.h>
#include <net/protocol.h>
+#include <net/proto_memory.h>
#include <net/tcp_states.h>
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
#include <net/transp_v6.h>
@@ -1016,7 +1017,7 @@ static void mptcp_enter_memory_pressure(struct sock *sk)
mptcp_for_each_subflow(msk, subflow) {
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
- if (first)
+ if (first && sk_should_enter_memory_pressure(ssk))
tcp_enter_memory_pressure(ssk);
sk_stream_moderate_sndbuf(ssk);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f672a62a9a52..6696ef837116 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -35,6 +35,7 @@
#include <linux/netdevice.h>
#include <net/dst.h>
#include <net/inet_connection_sock.h>
+#include <net/proto_memory.h>
#include <net/tcp.h>
#include <net/tls.h>
#include <linux/skbuff_ref.h>
@@ -371,7 +372,8 @@ static int tls_do_allocation(struct sock *sk,
if (!offload_ctx->open_record) {
if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
sk->sk_allocation))) {
- READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
+ if (sk_should_enter_memory_pressure(sk))
+ READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
return -ENOMEM;
}
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED.
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
` (3 preceding siblings ...)
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting Kuniyuki Iwashima
@ 2025-08-29 1:00 ` Kuniyuki Iwashima
2025-09-02 20:26 ` Martin KaFai Lau
4 siblings, 1 reply; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-08-29 1:00 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau
Cc: John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, Kuniyuki Iwashima, bpf, netdev
The test does the following for IPv4/IPv6 x TCP/UDP sockets
with/without BPF prog.
1. Create socket pairs
2. Send a bunch of data that requires more than 256 pages
3. Read memory_allocated from the 3rd column in /proc/net/protocols
4. Check if unread data is charged to memory_allocated
If BPF prog is attached, memory_allocated should not be changed,
but we allow a small error (up to 10 pages) in case other processes
on the host use some amounts of TCP/UDP memory.
At 2., the test actually sends more than 1024 pages because the sysctl
net.core.mem_pcpu_rsv is 256 is by default, which means 256 pages are
buffered per cpu before reporting to sk->sk_prot->memory_allocated.
BUF_SINGLE (1024) * NR_SEND (64) * NR_SOCKETS (64) / 4096
= 1024 pages
When I reduced it to 512 pages, the following assertion for the
non-isolated case got flaky.
ASSERT_GT(memory_allocated[1], memory_allocated[0] + 256, ...)
Another contributor to slowness is 150ms sleep to make sure 1 RCU
grace period passes because UDP recv queue is destroyed after that.
# time ./test_progs -t sk_memcg
#370/1 sk_memcg/TCP :OK
#370/2 sk_memcg/UDP :OK
#370/3 sk_memcg/TCPv6 :OK
#370/4 sk_memcg/UDPv6 :OK
#370 sk_memcg:OK
Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
real 0m1.214s
user 0m0.014s
sys 0m0.318s
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v4:
* Only use inet_create() hook
* Test bpf_getsockopt()
* Add serial_ prefix
* Reduce sleep() and the amount of sent data
---
.../selftests/bpf/prog_tests/sk_memcg.c | 218 ++++++++++++++++++
tools/testing/selftests/bpf/progs/sk_memcg.c | 38 +++
2 files changed, 256 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/sk_memcg.c
create mode 100644 tools/testing/selftests/bpf/progs/sk_memcg.c
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_memcg.c b/tools/testing/selftests/bpf/prog_tests/sk_memcg.c
new file mode 100644
index 000000000000..2d68b00419a2
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sk_memcg.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2025 Google LLC */
+
+#include <test_progs.h>
+#include "sk_memcg.skel.h"
+#include "network_helpers.h"
+
+#define NR_SOCKETS 64
+#define NR_SEND 64
+#define BUF_SINGLE 1024
+#define BUF_TOTAL (BUF_SINGLE * NR_SEND)
+
+struct test_case {
+ char name[10]; /* protocols (%-9s) in /proc/net/protocols, see proto_seq_printf(). */
+ int family;
+ int type;
+ int (*create_sockets)(struct test_case *test_case, int sk[], int len);
+};
+
+static int tcp_create_sockets(struct test_case *test_case, int sk[], int len)
+{
+ int server, i;
+
+ server = start_server(test_case->family, test_case->type, NULL, 0, 0);
+ ASSERT_GE(server, 0, "start_server_str");
+
+ for (i = 0; i < len / 2; i++) {
+ sk[i * 2] = connect_to_fd(server, 0);
+ if (!ASSERT_GE(sk[i * 2], 0, "connect_to_fd"))
+ return sk[i * 2];
+
+ sk[i * 2 + 1] = accept(server, NULL, NULL);
+ if (!ASSERT_GE(sk[i * 2 + 1], 0, "accept"))
+ return sk[i * 2 + 1];
+ }
+
+ close(server);
+
+ return 0;
+}
+
+static int udp_create_sockets(struct test_case *test_case, int sk[], int len)
+{
+ int i, err, rcvbuf = BUF_TOTAL;
+
+ for (i = 0; i < len / 2; i++) {
+ sk[i * 2] = start_server(test_case->family, test_case->type, NULL, 0, 0);
+ if (!ASSERT_GE(sk[i * 2], 0, "start_server"))
+ return sk[i * 2];
+
+ sk[i * 2 + 1] = connect_to_fd(sk[i * 2], 0);
+ if (!ASSERT_GE(sk[i * 2 + 1], 0, "connect_to_fd"))
+ return sk[i * 2 + 1];
+
+ err = connect_fd_to_fd(sk[i * 2], sk[i * 2 + 1], 0);
+ if (!ASSERT_EQ(err, 0, "connect_fd_to_fd"))
+ return err;
+
+ err = setsockopt(sk[i * 2], SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(int));
+ if (!ASSERT_EQ(err, 0, "setsockopt(SO_RCVBUF)"))
+ return err;
+
+ err = setsockopt(sk[i * 2 + 1], SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(int));
+ if (!ASSERT_EQ(err, 0, "setsockopt(SO_RCVBUF)"))
+ return err;
+ }
+
+ return 0;
+}
+
+static int get_memory_allocated(struct test_case *test_case)
+{
+ long memory_allocated = -1;
+ char *line = NULL;
+ size_t unused;
+ FILE *f;
+
+ f = fopen("/proc/net/protocols", "r");
+ if (!ASSERT_OK_PTR(f, "fopen"))
+ goto out;
+
+ while (getline(&line, &unused, f) != -1) {
+ unsigned int unused_0;
+ int unused_1;
+ int ret;
+
+ if (strncmp(line, test_case->name, sizeof(test_case->name)))
+ continue;
+
+ ret = sscanf(line + sizeof(test_case->name), "%4u %6d %6ld",
+ &unused_0, &unused_1, &memory_allocated);
+ ASSERT_EQ(ret, 3, "sscanf");
+ break;
+ }
+
+ ASSERT_NEQ(memory_allocated, -1, "get_memory_allocated");
+
+ free(line);
+ fclose(f);
+out:
+ return memory_allocated;
+}
+
+static int check_isolated(struct test_case *test_case, bool isolated)
+{
+ char buf[BUF_SINGLE] = {};
+ long memory_allocated[2];
+ int sk[NR_SOCKETS] = {};
+ int err = -1, i, j;
+
+ memory_allocated[0] = get_memory_allocated(test_case);
+ if (!ASSERT_GE(memory_allocated[0], 0, "memory_allocated[0]"))
+ goto out;
+
+ err = test_case->create_sockets(test_case, sk, ARRAY_SIZE(sk));
+ if (err)
+ goto close;
+
+ /* Must allocate pages >= net.core.mem_pcpu_rsv */
+ for (i = 0; i < ARRAY_SIZE(sk); i++) {
+ for (j = 0; j < NR_SEND; j++) {
+ int bytes = send(sk[i], buf, sizeof(buf), 0);
+
+ /* Avoid too noisy logs when something failed. */
+ if (bytes != sizeof(buf))
+ ASSERT_EQ(bytes, sizeof(buf), "send");
+ }
+ }
+
+ memory_allocated[1] = get_memory_allocated(test_case);
+ if (!ASSERT_GE(memory_allocated[1], 0, "memory_allocated[1]"))
+ goto close;
+
+ if (isolated) {
+ ASSERT_LE(memory_allocated[1], memory_allocated[0] + 10, "isolated");
+ } else {
+ /* By default, net.core.mem_pcpu_rsv == 256 pages */
+ ASSERT_GT(memory_allocated[1], memory_allocated[0] + 256, "not isolated");
+ }
+
+close:
+ for (i = 0; i < ARRAY_SIZE(sk); i++)
+ close(sk[i]);
+
+ if (test_case->type == SOCK_DGRAM) {
+ /* Give 150ms to let RCU destruct UDP sockets */
+ usleep(150 * 1000);
+ }
+out:
+ return err;
+}
+
+void run_test(struct test_case *test_case)
+{
+ struct sk_memcg *skel;
+ int cgroup, err;
+
+ skel = sk_memcg__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ cgroup = test__join_cgroup("/sk_memcg");
+ if (!ASSERT_GE(cgroup, 0, "join_cgroup"))
+ goto destroy_skel;
+
+ err = check_isolated(test_case, false);
+ if (!ASSERT_EQ(err, 0, "test_isolated(false)"))
+ goto close_cgroup;
+
+ skel->links.sock_create = bpf_program__attach_cgroup(skel->progs.sock_create, cgroup);
+ if (!ASSERT_OK_PTR(skel->links.sock_create, "attach_cgroup(sock_create)"))
+ goto close_cgroup;
+
+ err = check_isolated(test_case, true);
+ ASSERT_EQ(err, 0, "test_isolated(false)");
+
+close_cgroup:
+ close(cgroup);
+destroy_skel:
+ sk_memcg__destroy(skel);
+}
+
+struct test_case test_cases[] = {
+ {
+ .name = "TCP ",
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .create_sockets = tcp_create_sockets,
+ },
+ {
+ .name = "UDP ",
+ .family = AF_INET,
+ .type = SOCK_DGRAM,
+ .create_sockets = udp_create_sockets,
+ },
+ {
+ .name = "TCPv6 ",
+ .family = AF_INET6,
+ .type = SOCK_STREAM,
+ .create_sockets = tcp_create_sockets,
+ },
+ {
+ .name = "UDPv6 ",
+ .family = AF_INET6,
+ .type = SOCK_DGRAM,
+ .create_sockets = udp_create_sockets,
+ },
+};
+
+void serial_test_sk_memcg(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
+ test__start_subtest(test_cases[i].name);
+ run_test(&test_cases[i]);
+ }
+}
diff --git a/tools/testing/selftests/bpf/progs/sk_memcg.c b/tools/testing/selftests/bpf/progs/sk_memcg.c
new file mode 100644
index 000000000000..a613c1deeede
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sk_memcg.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2025 Google LLC */
+
+#include "bpf_tracing_net.h"
+#include <bpf/bpf_helpers.h>
+#include <errno.h>
+
+SEC("cgroup/sock_create")
+int sock_create(struct bpf_sock *ctx)
+{
+ u32 flags = SK_BPF_MEMCG_SOCK_ISOLATED;
+ int err;
+
+ err = bpf_setsockopt(ctx, SOL_SOCKET, SK_BPF_MEMCG_FLAGS,
+ &flags, sizeof(flags));
+ if (err)
+ goto err;
+
+ flags = 0;
+
+ err = bpf_getsockopt(ctx, SOL_SOCKET, SK_BPF_MEMCG_FLAGS,
+ &flags, sizeof(flags));
+ if (err)
+ goto err;
+
+ if (flags != SK_BPF_MEMCG_SOCK_ISOLATED) {
+ err = -EINVAL;
+ goto err;
+ }
+
+ return 1;
+
+err:
+ bpf_set_retval(err);
+ return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
--
2.51.0.318.gd7df087d1a-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept().
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept() Kuniyuki Iwashima
@ 2025-09-02 18:55 ` Martin KaFai Lau
2025-09-02 19:32 ` Kuniyuki Iwashima
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2025-09-02 18:55 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> mem_cgroup_sk_alloc() is called for SCTP before __inet_accept(),
> so I added the protocol check in __inet_accept(), but this can be
> removed once SCTP uses sk_clone_lock().
> void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *newsk)
> {
> + /* TODO: use sk_clone_lock() in SCTP and remove protocol checks */
> + if (mem_cgroup_sockets_enabled &&
> + (!IS_ENABLED(CONFIG_IP_SCTP) ||
> + sk_is_tcp(newsk) || sk_is_mptcp(newsk))) {
Instead of protocol check, is it the same as checking
"if (mem_cgroup_sockets_enabled && !mem_cgroup_from_sk(newsk))"
> + gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
> +
> + mem_cgroup_sk_alloc(newsk);
> +
> + if (mem_cgroup_from_sk(newsk)) {
> + int amt;
> +
> + /* The socket has not been accepted yet, no need
> + * to look at newsk->sk_wmem_queued.
> + */
> + amt = sk_mem_pages(newsk->sk_forward_alloc +
> + atomic_read(&newsk->sk_rmem_alloc));
> + if (amt)
> + mem_cgroup_sk_charge(newsk, amt, gfp);
> + }
> +
> + kmem_cache_charge(newsk, gfp);
> + }
> +
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE.
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE Kuniyuki Iwashima
@ 2025-09-02 19:10 ` Martin KaFai Lau
2025-09-02 19:33 ` Kuniyuki Iwashima
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2025-09-02 19:10 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> +BPF_CALL_5(bpf_unlocked_sock_setsockopt, struct sock *, sk, int, level,
> + int, optname, char *, optval, int, optlen)
> +{
> + return __bpf_setsockopt(sk, level, optname, optval, optlen);
> +}
> +
> +static const struct bpf_func_proto bpf_unlocked_sock_setsockopt_proto = {
nit. There is a bpf_unlocked_"sk"_{get,set}sockopt_proto which its .func is also
taking "struct sock *". This one is sock_create specific, how about renaming it
to bpf_sock_create_{get,set}sockopt_proto. The same for the its .func.
> + .func = bpf_unlocked_sock_setsockopt,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_ANYTHING,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY,
> + .arg5_type = ARG_CONST_SIZE,
> +};
> +
> +BPF_CALL_5(bpf_unlocked_sock_getsockopt, struct sock *, sk, int, level,
> + int, optname, char *, optval, int, optlen)
> +{
> + return __bpf_getsockopt(sk, level, optname, optval, optlen);
> +}
> +
> +static const struct bpf_func_proto bpf_unlocked_sock_getsockopt_proto = {
> + .func = bpf_unlocked_sock_getsockopt,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_ANYTHING,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_PTR_TO_UNINIT_MEM,
> + .arg5_type = ARG_CONST_SIZE,
> +};
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept().
2025-09-02 18:55 ` Martin KaFai Lau
@ 2025-09-02 19:32 ` Kuniyuki Iwashima
0 siblings, 0 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-02 19:32 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On Tue, Sep 2, 2025 at 11:55 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> > mem_cgroup_sk_alloc() is called for SCTP before __inet_accept(),
> > so I added the protocol check in __inet_accept(), but this can be
> > removed once SCTP uses sk_clone_lock().
>
> > void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *newsk)
> > {
> > + /* TODO: use sk_clone_lock() in SCTP and remove protocol checks */
> > + if (mem_cgroup_sockets_enabled &&
> > + (!IS_ENABLED(CONFIG_IP_SCTP) ||
> > + sk_is_tcp(newsk) || sk_is_mptcp(newsk))) {
>
> Instead of protocol check, is it the same as checking
> "if (mem_cgroup_sockets_enabled && !mem_cgroup_from_sk(newsk))"
sk_memcg could be NULL when the socket is belongs to the root
memcg, and then we shouldn't call kmem_cache_charge() for SCTP.
Also, SCTP's child sockets are not supported until we convert them to
sk_clone_lock(), and I plan to do so after this series lands on net-next.
https://github.com/q2ven/linux/commits/522_sctp_sk_clone_lock/
>
> > + gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
> > +
> > + mem_cgroup_sk_alloc(newsk);
> > +
> > + if (mem_cgroup_from_sk(newsk)) {
> > + int amt;
> > +
> > + /* The socket has not been accepted yet, no need
> > + * to look at newsk->sk_wmem_queued.
> > + */
> > + amt = sk_mem_pages(newsk->sk_forward_alloc +
> > + atomic_read(&newsk->sk_rmem_alloc));
> > + if (amt)
> > + mem_cgroup_sk_charge(newsk, amt, gfp);
> > + }
> > +
> > + kmem_cache_charge(newsk, gfp);
> > + }
> > +
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE.
2025-09-02 19:10 ` Martin KaFai Lau
@ 2025-09-02 19:33 ` Kuniyuki Iwashima
0 siblings, 0 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-02 19:33 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On Tue, Sep 2, 2025 at 12:10 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> > +BPF_CALL_5(bpf_unlocked_sock_setsockopt, struct sock *, sk, int, level,
> > + int, optname, char *, optval, int, optlen)
> > +{
> > + return __bpf_setsockopt(sk, level, optname, optval, optlen);
> > +}
> > +
> > +static const struct bpf_func_proto bpf_unlocked_sock_setsockopt_proto = {
>
> nit. There is a bpf_unlocked_"sk"_{get,set}sockopt_proto which its .func is also
> taking "struct sock *". This one is sock_create specific, how about renaming it
> to bpf_sock_create_{get,set}sockopt_proto. The same for the its .func.
Sounds better to me :)
Will rename both.
>
>
> > + .func = bpf_unlocked_sock_setsockopt,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_PTR_TO_CTX,
> > + .arg2_type = ARG_ANYTHING,
> > + .arg3_type = ARG_ANYTHING,
> > + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY,
> > + .arg5_type = ARG_CONST_SIZE,
> > +};
> > +
> > +BPF_CALL_5(bpf_unlocked_sock_getsockopt, struct sock *, sk, int, level,
> > + int, optname, char *, optval, int, optlen)
> > +{
> > + return __bpf_getsockopt(sk, level, optname, optval, optlen);
> > +}
> > +
> > +static const struct bpf_func_proto bpf_unlocked_sock_getsockopt_proto = {
> > + .func = bpf_unlocked_sock_getsockopt,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_PTR_TO_CTX,
> > + .arg2_type = ARG_ANYTHING,
> > + .arg3_type = ARG_ANYTHING,
> > + .arg4_type = ARG_PTR_TO_UNINIT_MEM,
> > + .arg5_type = ARG_CONST_SIZE,
> > +};
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED.
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
@ 2025-09-02 20:02 ` Martin KaFai Lau
2025-09-02 20:13 ` Kuniyuki Iwashima
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2025-09-02 20:02 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> static int sol_socket_sockopt(struct sock *sk, int optname,
> char *optval, int *optlen,
> bool getopt)
> @@ -5284,6 +5313,7 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> case SO_BINDTOIFINDEX:
> case SO_TXREHASH:
> case SK_BPF_CB_FLAGS:
> + case SK_BPF_MEMCG_FLAGS:
> if (*optlen != sizeof(int))
> return -EINVAL;
> break;
> @@ -5293,8 +5323,15 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> return -EINVAL;
> }
>
> - if (optname == SK_BPF_CB_FLAGS)
> + switch (optname) {
> + case SK_BPF_CB_FLAGS:
> return sk_bpf_set_get_cb_flags(sk, optval, getopt);
> + case SK_BPF_MEMCG_FLAGS:
I would remove the getsockopt only support from the other hooks that cannot do
the setsockopt. There are other ways for them to read sk->sk_memcg if it is
really needed.
> + if (!IS_ENABLED(CONFIG_MEMCG) || !getopt)
> + return -EOPNOTSUPP;
> +
> + return sk_bpf_get_memcg_flags(sk, optval);
Instead, do this only in bpf_sock_create_getsockopt.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED.
2025-09-02 20:02 ` Martin KaFai Lau
@ 2025-09-02 20:13 ` Kuniyuki Iwashima
0 siblings, 0 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-02 20:13 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On Tue, Sep 2, 2025 at 1:02 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> > static int sol_socket_sockopt(struct sock *sk, int optname,
> > char *optval, int *optlen,
> > bool getopt)
> > @@ -5284,6 +5313,7 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> > case SO_BINDTOIFINDEX:
> > case SO_TXREHASH:
> > case SK_BPF_CB_FLAGS:
> > + case SK_BPF_MEMCG_FLAGS:
> > if (*optlen != sizeof(int))
> > return -EINVAL;
> > break;
> > @@ -5293,8 +5323,15 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> > return -EINVAL;
> > }
> >
> > - if (optname == SK_BPF_CB_FLAGS)
> > + switch (optname) {
> > + case SK_BPF_CB_FLAGS:
> > return sk_bpf_set_get_cb_flags(sk, optval, getopt);
> > + case SK_BPF_MEMCG_FLAGS:
>
> I would remove the getsockopt only support from the other hooks that cannot do
> the setsockopt. There are other ways for them to read sk->sk_memcg if it is
> really needed.
Ah, I forgot bpf_core_cast().
>
> > + if (!IS_ENABLED(CONFIG_MEMCG) || !getopt)
> > + return -EOPNOTSUPP;
> > +
> > + return sk_bpf_get_memcg_flags(sk, optval);
>
> Instead, do this only in bpf_sock_create_getsockopt.
Will do.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting.
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting Kuniyuki Iwashima
@ 2025-09-02 20:16 ` Martin KaFai Lau
2025-09-02 20:45 ` Kuniyuki Iwashima
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2025-09-02 20:16 UTC (permalink / raw)
To: Kuniyuki Iwashima, Shakeel Butt
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Neal Cardwell, Willem de Bruijn, Mina Almasry,
Kuniyuki Iwashima, bpf, netdev
On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> +static inline bool sk_should_enter_memory_pressure(struct sock *sk)
> +{
> + return !mem_cgroup_sk_enabled(sk) || !mem_cgroup_sk_isolated(sk);
> +}
> +
> static inline long
> proto_memory_allocated(const struct proto *prot)
> {
> @@ -3154,8 +3158,11 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
> if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
> return true;
>
> - sk_enter_memory_pressure(sk);
> + if (sk_should_enter_memory_pressure(sk))
> + sk_enter_memory_pressure(sk);
> +
> sk_stream_moderate_sndbuf(sk);
> +
> return false;
> }
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 71a956fbfc55..dcbd49e2f8af 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -908,7 +908,8 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, gfp_t gfp,
> }
> __kfree_skb(skb);
> } else {
> - sk->sk_prot->enter_memory_pressure(sk);
> + if (sk_should_enter_memory_pressure(sk))
> + tcp_enter_memory_pressure(sk);
This change from sk_prot->enter_memory_pressure to tcp_enter_memory_pressure
looks fine. A qq / nit, have you thought about checking
sk_should_enter_memory_pressure inside the tcp_enter_memory_pressure(sk) /
sk_enter_memory_pressure(sk) ?
Other changes of patch 4 lgtm.
Shakeel, you have ack-ed patch 1. Will you take a look at patch 3 and patch 4 also?
> sk_stream_moderate_sndbuf(sk);
> }
> @@ -1016,7 +1017,7 @@ static void mptcp_enter_memory_pressure(struct sock *sk)
> mptcp_for_each_subflow(msk, subflow) {
> struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>
> - if (first)
> + if (first && sk_should_enter_memory_pressure(ssk))
> tcp_enter_memory_pressure(ssk);
> sk_stream_moderate_sndbuf(ssk);
>
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index f672a62a9a52..6696ef837116 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -35,6 +35,7 @@
> #include <linux/netdevice.h>
> #include <net/dst.h>
> #include <net/inet_connection_sock.h>
> +#include <net/proto_memory.h>
> #include <net/tcp.h>
> #include <net/tls.h>
> #include <linux/skbuff_ref.h>
> @@ -371,7 +372,8 @@ static int tls_do_allocation(struct sock *sk,
> if (!offload_ctx->open_record) {
> if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
> sk->sk_allocation))) {
> - READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
> + if (sk_should_enter_memory_pressure(sk))
> + READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
> sk_stream_moderate_sndbuf(sk);
> return -ENOMEM;
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED.
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
@ 2025-09-02 20:26 ` Martin KaFai Lau
2025-09-02 20:49 ` Kuniyuki Iwashima
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2025-09-02 20:26 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> The test does the following for IPv4/IPv6 x TCP/UDP sockets
> with/without BPF prog.
>
> 1. Create socket pairs
> 2. Send a bunch of data that requires more than 256 pages
> 3. Read memory_allocated from the 3rd column in /proc/net/protocols
> 4. Check if unread data is charged to memory_allocated
>
> If BPF prog is attached, memory_allocated should not be changed,
> but we allow a small error (up to 10 pages) in case other processes
> on the host use some amounts of TCP/UDP memory.
>
> At 2., the test actually sends more than 1024 pages because the sysctl
> net.core.mem_pcpu_rsv is 256 is by default, which means 256 pages are
> buffered per cpu before reporting to sk->sk_prot->memory_allocated.
>
> BUF_SINGLE (1024) * NR_SEND (64) * NR_SOCKETS (64) / 4096
> = 1024 pages
>
> When I reduced it to 512 pages, the following assertion for the
> non-isolated case got flaky.
>
> ASSERT_GT(memory_allocated[1], memory_allocated[0] + 256, ...)
>
> Another contributor to slowness is 150ms sleep to make sure 1 RCU
> grace period passes because UDP recv queue is destroyed after that.
There is a kern_sync_rcu() in testing_helpers.c.
>
> # time ./test_progs -t sk_memcg
> #370/1 sk_memcg/TCP :OK
> #370/2 sk_memcg/UDP :OK
> #370/3 sk_memcg/TCPv6 :OK
> #370/4 sk_memcg/UDPv6 :OK
> #370 sk_memcg:OK
> Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
>
> real 0m1.214s
> user 0m0.014s
> sys 0m0.318s
Thanks. It finished much faster in my setup also comparing with the earlier
revision. However, it is a bit flaky when I run it in a loop:
check_isolated:FAIL:not isolated unexpected not isolated: actual 861 <= expected 861
I usually can hit this at ~40-th iteration.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting.
2025-09-02 20:16 ` Martin KaFai Lau
@ 2025-09-02 20:45 ` Kuniyuki Iwashima
0 siblings, 0 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-02 20:45 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Shakeel Butt, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Johannes Weiner, Michal Hocko, Roman Gushchin, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
Willem de Bruijn, Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On Tue, Sep 2, 2025 at 1:17 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> > +static inline bool sk_should_enter_memory_pressure(struct sock *sk)
> > +{
> > + return !mem_cgroup_sk_enabled(sk) || !mem_cgroup_sk_isolated(sk);
> > +}
> > +
> > static inline long
> > proto_memory_allocated(const struct proto *prot)
> > {
>
> > @@ -3154,8 +3158,11 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
> > if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
> > return true;
> >
> > - sk_enter_memory_pressure(sk);
> > + if (sk_should_enter_memory_pressure(sk))
> > + sk_enter_memory_pressure(sk);
> > +
> > sk_stream_moderate_sndbuf(sk);
> > +
> > return false;
> > }
>
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 71a956fbfc55..dcbd49e2f8af 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -908,7 +908,8 @@ struct sk_buff *tcp_stream_alloc_skb(struct sock *sk, gfp_t gfp,
> > }
> > __kfree_skb(skb);
> > } else {
> > - sk->sk_prot->enter_memory_pressure(sk);
> > + if (sk_should_enter_memory_pressure(sk))
> > + tcp_enter_memory_pressure(sk);
>
> This change from sk_prot->enter_memory_pressure to tcp_enter_memory_pressure
> looks fine. A qq / nit, have you thought about checking
> sk_should_enter_memory_pressure inside the tcp_enter_memory_pressure(sk) /
> sk_enter_memory_pressure(sk) ?
Yes, I wanted to centralise the check to sk_enter_memory_pressure(),
but it calls tcp_enter_memory_pressure() (or sctp one) and they are also
called directly, so I finally placed the check outside of the functions.
>
> Other changes of patch 4 lgtm.
>
> Shakeel, you have ack-ed patch 1. Will you take a look at patch 3 and patch 4 also?
>
> > sk_stream_moderate_sndbuf(sk);
> > }
>
> > @@ -1016,7 +1017,7 @@ static void mptcp_enter_memory_pressure(struct sock *sk)
> > mptcp_for_each_subflow(msk, subflow) {
> > struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> >
> > - if (first)
> > + if (first && sk_should_enter_memory_pressure(ssk))
> > tcp_enter_memory_pressure(ssk);
> > sk_stream_moderate_sndbuf(ssk);
> >
> > diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> > index f672a62a9a52..6696ef837116 100644
> > --- a/net/tls/tls_device.c
> > +++ b/net/tls/tls_device.c
> > @@ -35,6 +35,7 @@
> > #include <linux/netdevice.h>
> > #include <net/dst.h>
> > #include <net/inet_connection_sock.h>
> > +#include <net/proto_memory.h>
> > #include <net/tcp.h>
> > #include <net/tls.h>
> > #include <linux/skbuff_ref.h>
> > @@ -371,7 +372,8 @@ static int tls_do_allocation(struct sock *sk,
> > if (!offload_ctx->open_record) {
> > if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
> > sk->sk_allocation))) {
> > - READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
> > + if (sk_should_enter_memory_pressure(sk))
> > + READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
> > sk_stream_moderate_sndbuf(sk);
> > return -ENOMEM;
> > }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED.
2025-09-02 20:26 ` Martin KaFai Lau
@ 2025-09-02 20:49 ` Kuniyuki Iwashima
0 siblings, 0 replies; 16+ messages in thread
From: Kuniyuki Iwashima @ 2025-09-02 20:49 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
John Fastabend, Stanislav Fomichev, Johannes Weiner, Michal Hocko,
Roman Gushchin, Shakeel Butt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Neal Cardwell, Willem de Bruijn,
Mina Almasry, Kuniyuki Iwashima, bpf, netdev
On Tue, Sep 2, 2025 at 1:26 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 8/28/25 6:00 PM, Kuniyuki Iwashima wrote:
> > The test does the following for IPv4/IPv6 x TCP/UDP sockets
> > with/without BPF prog.
> >
> > 1. Create socket pairs
> > 2. Send a bunch of data that requires more than 256 pages
> > 3. Read memory_allocated from the 3rd column in /proc/net/protocols
> > 4. Check if unread data is charged to memory_allocated
> >
> > If BPF prog is attached, memory_allocated should not be changed,
> > but we allow a small error (up to 10 pages) in case other processes
> > on the host use some amounts of TCP/UDP memory.
> >
> > At 2., the test actually sends more than 1024 pages because the sysctl
> > net.core.mem_pcpu_rsv is 256 is by default, which means 256 pages are
> > buffered per cpu before reporting to sk->sk_prot->memory_allocated.
> >
> > BUF_SINGLE (1024) * NR_SEND (64) * NR_SOCKETS (64) / 4096
> > = 1024 pages
> >
> > When I reduced it to 512 pages, the following assertion for the
> > non-isolated case got flaky.
> >
> > ASSERT_GT(memory_allocated[1], memory_allocated[0] + 256, ...)
> >
> > Another contributor to slowness is 150ms sleep to make sure 1 RCU
> > grace period passes because UDP recv queue is destroyed after that.
>
> There is a kern_sync_rcu() in testing_helpers.c.
Nice helper :) Will use it.
>
> >
> > # time ./test_progs -t sk_memcg
> > #370/1 sk_memcg/TCP :OK
> > #370/2 sk_memcg/UDP :OK
> > #370/3 sk_memcg/TCPv6 :OK
> > #370/4 sk_memcg/UDPv6 :OK
> > #370 sk_memcg:OK
> > Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
> >
> > real 0m1.214s
> > user 0m0.014s
> > sys 0m0.318s
>
> Thanks. It finished much faster in my setup also comparing with the earlier
> revision. However, it is a bit flaky when I run it in a loop:
>
> check_isolated:FAIL:not isolated unexpected not isolated: actual 861 <= expected 861
>
> I usually can hit this at ~40-th iteration.
Oh.. I tested ~10 times manually but will try in a tight loop.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-09-02 20:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 1:00 [PATCH v4 bpf-next/net 0/5] bpf: Allow decoupling memcg from sk->sk_prot->memory_allocated Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 1/5] tcp: Save lock_sock() for memcg in inet_csk_accept() Kuniyuki Iwashima
2025-09-02 18:55 ` Martin KaFai Lau
2025-09-02 19:32 ` Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 2/5] bpf: Support bpf_setsockopt() for BPF_CGROUP_INET_SOCK_CREATE Kuniyuki Iwashima
2025-09-02 19:10 ` Martin KaFai Lau
2025-09-02 19:33 ` Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 3/5] bpf: Introduce SK_BPF_MEMCG_FLAGS and SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
2025-09-02 20:02 ` Martin KaFai Lau
2025-09-02 20:13 ` Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 4/5] net-memcg: Allow decoupling memcg from global protocol memory accounting Kuniyuki Iwashima
2025-09-02 20:16 ` Martin KaFai Lau
2025-09-02 20:45 ` Kuniyuki Iwashima
2025-08-29 1:00 ` [PATCH v4 bpf-next/net 5/5] selftest: bpf: Add test for SK_BPF_MEMCG_SOCK_ISOLATED Kuniyuki Iwashima
2025-09-02 20:26 ` Martin KaFai Lau
2025-09-02 20:49 ` Kuniyuki Iwashima
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).