[PATCH net] nexthop: account nexthop allocations to memcg

[PATCH net] nexthop: account nexthop allocations to memcg

[Yizhou Zhao]

The nexthop creation path allocates struct nexthop, struct nh_info,
group storage, resilient bucket tables, transient notifier tables and
per-group stats without memcg accounting. As a result, a task with
CAP_NET_ADMIN can create nexthops and resilient groups whose kernel
memory is not charged to the task's memory cgroup, so memory.max does
not constrain this part of the workload.

A QEMU reproduction with Docker confirmed the problem. A container
granted CAP_NET_ADMIN and limited to memory.max=128M could repeatedly
create resilient nexthops while memory.current stayed at 589824 and host
VmallocUsed rose from 67044 kB to 231860 kB.

Fix this by charging the nexthop creation-path allocations to the
caller's memcg with GFP_KERNEL_ACCOUNT / __GFP_ACCOUNT, including the
per-group stats allocation added later. With this change, the same
workload hits memcg OOM with GFP_KERNEL_ACCOUNT in the allocation path
and host VmallocUsed stays flat at 67044 kB, so memory.max constrains
the workload again.

Fixes: ab84be7e54fc ("net: Initial nexthop code")
Fixes: f4676ea74b85 ("net: nexthop: Add nexthop group entry stats")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: Claude-Code:GLM-5.2
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 44fe75004cac..c8685c583579 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -169,7 +169,7 @@ static int nh_notifier_res_table_info_init(struct nh_notifier_info *info,
 	info->type = NH_NOTIFIER_INFO_TYPE_RES_TABLE;
 	size = struct_size(info->nh_res_table, nhs, num_nh_buckets);
 	info->nh_res_table = __vmalloc(size, GFP_KERNEL | __GFP_ZERO |
-				       __GFP_NOWARN);
+				       __GFP_NOWARN | __GFP_ACCOUNT);
 	if (!info->nh_res_table)
 		return -ENOMEM;
 
@@ -533,7 +533,7 @@ static struct nexthop *nexthop_alloc(void)
 {
 	struct nexthop *nh;
 
-	nh = kzalloc_obj(struct nexthop);
+	nh = kzalloc_obj(struct nexthop, GFP_KERNEL_ACCOUNT);
 	if (nh) {
 		INIT_LIST_HEAD(&nh->fi_list);
 		INIT_LIST_HEAD(&nh->f6i_list);
@@ -548,7 +548,7 @@ static struct nh_group *nexthop_grp_alloc(u16 num_nh)
 {
 	struct nh_group *nhg;
 
-	nhg = kzalloc_flex(*nhg, nh_entries, num_nh);
+	nhg = kzalloc_flex(*nhg, nh_entries, num_nh, GFP_KERNEL_ACCOUNT);
 	if (nhg)
 		nhg->num_nh = num_nh;
 
@@ -565,7 +565,8 @@ nexthop_res_table_alloc(struct net *net, u32 nhg_id, struct nh_config *cfg)
 	unsigned long size;
 
 	size = struct_size(res_table, nh_buckets, num_nh_buckets);
-	res_table = __vmalloc(size, GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN);
+	res_table = __vmalloc(size, GFP_KERNEL | __GFP_ZERO | __GFP_NOWARN |
+			      __GFP_ACCOUNT);
 	if (!res_table)
 		return NULL;
 
@@ -2799,7 +2800,8 @@ static struct nexthop *nexthop_create_group(struct net *net,
 			nhg->has_v4 = true;
 
 		nhg->nh_entries[i].stats =
-			netdev_alloc_pcpu_stats(struct nh_grp_entry_stats);
+			__netdev_alloc_pcpu_stats(struct nh_grp_entry_stats,
+						  GFP_KERNEL_ACCOUNT);
 		if (!nhg->nh_entries[i].stats) {
 			err = -ENOMEM;
 			nexthop_put(nhe);
@@ -2943,7 +2945,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
 	if (!nh)
 		return ERR_PTR(-ENOMEM);
 
-	nhi = kzalloc_obj(*nhi);
+	nhi = kzalloc_obj(*nhi, GFP_KERNEL_ACCOUNT);
 	if (!nhi) {
 		kfree(nh);
 		return ERR_PTR(-ENOMEM);

--
2.47.3

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
commit 186a5b6499e65039b5037906ec518baac2833b1d
Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>

    nexthop: account nexthop allocations to memcg
    
    The nexthop creation path allocates struct nexthop, struct nh_info,
    group storage, resilient bucket tables, transient notifier tables and
    per-group stats without memcg accounting. As a result, a task with
    CAP_NET_ADMIN can create nexthops and resilient groups whose kernel
    memory is not charged to the task's memory cgroup, so memory.max does
    not constrain this part of the workload.

> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 44fe75004cacb..c8685c5835794 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
[ ... ]
> @@ -2943,7 +2945,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
>  	if (!nh)
>  		return ERR_PTR(-ENOMEM);
>  
> -	nhi = kzalloc_obj(*nhi);
> +	nhi = kzalloc_obj(*nhi, GFP_KERNEL_ACCOUNT);

[Severity: High]
This is a pre-existing issue, but does this patch miss some per-CPU
allocations in the nexthop creation path?

When creating IPv4 and IPv6 nexthops, nh_create_ipv6() and fib_nh_init()
explicitly pass GFP_KERNEL without __GFP_ACCOUNT down to the initialization
functions:

net/ipv4/nexthop.c:nh_create_ipv6() {
    ...
    err = fib6_nh_init(net, fib6_nh, &fib6_cfg, GFP_KERNEL, extack);
    ...
}

This results in unaccounted per-CPU allocations:

net/ipv6/route.c:fib6_nh_init() {
    ...
    fib6_nh->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, gfp_flags);
    ...
}

A similar missing accounting happens for IPv4 in fib_nh_common_init():

net/ipv4/fib_semantics.c:fib_nh_common_init() {
    ...
    nhc->nhc_pcpu_rth_output = alloc_percpu_gfp(struct rtable __rcu *,
                                                gfp_flags);
    ...
}

Could an unprivileged user with CAP_NET_ADMIN still bypass memcg limits by
exhausting memory through these per-CPU allocations?


[Severity: High]
This is a pre-existing issue, but do we also need to account for Lightweight
Tunnel (LWT) state allocations?

During nexthop creation with encapsulation attributes, fib_nh_common_init()
triggers LWT state allocation:

net/ipv4/fib_semantics.c:fib_nh_common_init() {
    ...
    err = lwtunnel_build_state(net, encap_type, encap,
                               nhc->nhc_family, cfg, &lwtstate,
                               extack);
    ...
}

The allocation unconditionally uses GFP_ATOMIC without __GFP_ACCOUNT:

net/core/lwtunnel.c:lwtunnel_state_alloc() {
    ...
    lws = kzalloc(sizeof(*lws) + encap_len, GFP_ATOMIC);
    ...
}

Could a task in a container bypass memory.max limits by creating many nexthops
with encapsulation attributes?

>  	if (!nhi) {
>  		kfree(nh);
>  		return ERR_PTR(-ENOMEM);

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
commit 1014ad1eb712722973916d53a1c5348624a80550
Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>

nexthop: account nexthop allocations to memcg

This adds GFP_KERNEL_ACCOUNT / __GFP_ACCOUNT to the nexthop creation path
allocations (struct nexthop, struct nh_info, group storage, resilient
bucket table, per-group stats) and to the transient resilient-table
notifier buffer in nh_notifier_res_table_info_init().  The stated goal is
to make memory.max constrain nexthop creation by a CAP_NET_ADMIN task in
a container.

A couple of questions below about the notifier-table hunk and about the
tags.

>  nexthop: account nexthop allocations to memcg
>
>  Fixes: ab84be7e54fc ("net: Initial nexthop code")
>  Fixes: f4676ea74b85 ("net: nexthop: Add nexthop group entry stats")
>  Cc: stable@vger.kernel.org

[Severity: Low]
Are the Fixes: tags and the stable Cc appropriate here?  Switching
allocations to GFP_KERNEL_ACCOUNT does not fix an incorrect result, it
changes the success/failure profile of an existing uAPI operation.

After a stable backport, RTM_NEWNEXTHOP can start returning -ENOMEM
inside a memory.max-constrained cgroup on kernels where it previously
succeeded, which looks like a functional change for routing daemons
running in containers.

The operation still requires CAP_NET_ADMIN in the netns owner's user
namespace, so no privilege boundary is being closed - only accounting
accuracy changes.  Would net-next without the Fixes:/Cc: stable tags be a
better target, with the transient notifier hunk split out?

>  The nexthop creation path allocates struct nexthop, struct nh_info,
>  group storage, resilient bucket tables, transient notifier tables and
>  per-group stats without memcg accounting.

[Severity: Low]
The changelog says "transient notifier tables" in the plural, but only
one of the five transient notifier allocations in net/ipv4/nexthop.c is
converted.  These remain on plain GFP_KERNEL:

  nh_notifier_single_info_init()
	info->nh = kzalloc_obj(*info->nh);

  nh_notifier_mpath_info_init()
	info->nh_grp = kzalloc_flex(*info->nh_grp, nh_entries, num_nh);

  nh_notifier_res_bucket_info_init()
	info->nh_res_bucket = kzalloc_obj(*info->nh_res_bucket);

  nh_notifier_grp_hw_stats_init()
	info->nh_grp_hw_stats = kzalloc_flex(*info->nh_grp_hw_stats, stats,
					     nhg->num_nh);

Since nh_notifier_grp_info_init() dispatches to
nh_notifier_mpath_info_init() for hash_threshold (MPATH) groups, a group
created through the exact path described in the changelog gets no
notifier-table accounting at all, and call_nexthop_res_table_notifiers()
also builds an unaccounted num_nh-sized mpath table during resilient
group replace.

Could the policy be made all-or-nothing here, and the changelog adjusted
to match what the code does?

> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 44fe75004cac..c8685c583579 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -169,7 +169,7 @@ static int nh_notifier_res_table_info_init(struct nh_notifier_info *info,
>  	info->type = NH_NOTIFIER_INFO_TYPE_RES_TABLE;
>  	size = struct_size(info->nh_res_table, nhs, num_nh_buckets);
>  	info->nh_res_table = __vmalloc(size, GFP_KERNEL | __GFP_ZERO |
> -				       __GFP_NOWARN);
> +				       __GFP_NOWARN | __GFP_ACCOUNT);
>  	if (!info->nh_res_table)
>  		return -ENOMEM;

[Severity: Medium]
Should this particular allocation be accounted at all?  It is not on the
nexthop creation path the changelog describes.  The buffer only exists to
hand resilient-group state to notifier listeners and is released before
the notifier call returns:

net/ipv4/nexthop.c:nh_notifier_grp_info_fini() {
	if (nhg->hash_threshold)
		kfree(info->nh_grp);
	else if (nhg->resilient)
		vfree(info->nh_res_table);
}

So it cannot accumulate, and charging it does not add containment.  Two
side effects come with it though.

First, whose memcg gets charged?  The notifier paths run in the context
of whoever triggers the notification, not the creator of the nexthop:

  remove_nexthop() -> call_nexthop_notifiers(NEXTHOP_EVENT_DEL)
	(also reached via nh_netdev_event -> nexthop_flush_dev)
  replace_nexthop_single() err_notify -> call_nexthop_notifiers(REPLACE)
  remove_nh_grp_entry() -> call_nexthop_notifiers(REPLACE)
  register_nexthop_notifier() / __unregister_nexthop_notifier()
	-> nexthops_dump() -> nh_notifier_info_init() for every
	   existing nexthop

Can a cgroup end up charged here for nexthops belonging to other
cgroups, for example when a driver module registers a nexthop notifier
and the replay loop walks every resilient nexthop in the netns?

Second, does this make notifier delivery fail on demand?  __GFP_ACCOUNT
is honoured for vmalloc pages, so with a memcg at memory.max this
__vmalloc() can now fail while the system is globally healthy, and
__GFP_NOWARN keeps it quiet.  The failure happens before the chain is
called:

net/ipv4/nexthop.c:call_nexthop_notifiers() {
	err = nh_notifier_info_init(&info, nh);
	if (err) {
		NL_SET_ERR_MSG(extack, "Failed to initialize nexthop notifier info");
		return err;
	}

and the callers do not stop on that error:

net/ipv4/nexthop.c:remove_nexthop() {
	call_nexthop_notifiers(net, NEXTHOP_EVENT_DEL, nh, NULL);

	/* remove from the tree */
	rb_erase(&nh->rb_node, &net->nexthop.rb_root);

The return value is discarded and the nexthop is erased and freed
anyway.  replace_nexthop_single()'s err_notify path also discards it,
remove_nh_grp_entry() only pr_err()s, and __unregister_nexthop_notifier()
ignores nexthops_dump()'s status while nexthops_dump() breaks out at the
first error, skipping DEL for all remaining nexthops.

The fail-silent error handling in those callers is pre-existing and not
introduced by this patch, but does this change turn it into something a
task can trigger at will from inside a constrained cgroup?  With an
offload listener such as mlxsw or netdevsim registered, that would leave
driver state for nexthops the kernel has already freed, and would let
register_nexthop_notifier() be made to fail for the whole netns.

Would keeping nh_notifier_res_table_info_init() on plain GFP_KERNEL and
accounting only the persistent objects (struct nexthop, struct nh_info,
struct nh_group, res_table, per-group stats) avoid all of this?

[ ... ]
-- 
pw-bot: cr

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Yizhou Zhao]

Hi Jakub,

Thank you for both reviews. This is the reply for the first one.

> On Jul 30, 2026, at 08:38, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> --- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
> commit 186a5b6499e65039b5037906ec518baac2833b1d
> Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> 
>    nexthop: account nexthop allocations to memcg
> 
>    The nexthop creation path allocates struct nexthop, struct nh_info,
>    group storage, resilient bucket tables, transient notifier tables and
>    per-group stats without memcg accounting. As a result, a task with
>    CAP_NET_ADMIN can create nexthops and resilient groups whose kernel
>    memory is not charged to the task's memory cgroup, so memory.max does
>    not constrain this part of the workload.
> 
>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>> index 44fe75004cacb..c8685c5835794 100644
>> --- a/net/ipv4/nexthop.c
>> +++ b/net/ipv4/nexthop.c
> [ ... ]
>> @@ -2943,7 +2945,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
>> if (!nh)
>> return ERR_PTR(-ENOMEM);
>> 
>> - nhi = kzalloc_obj(*nhi);
>> + nhi = kzalloc_obj(*nhi, GFP_KERNEL_ACCOUNT);
> 
> [Severity: High]
> This is a pre-existing issue, but does this patch miss some per-CPU
> allocations in the nexthop creation path?

Yes. I confirmed that the original patch misses these allocations.

With only the persistent nexthop objects accounted, IPv4 and IPv6 nexthop
creation can still allocate nhc_pcpu_rth_output and rt6i_pcpu without memcg
accounting, so memory.max does not constrain that memory.

We will fix this in the next revision by passing GFP_KERNEL_ACCOUNT through
fib_nh_init() and fib6_nh_init() for persistent nexthop creation, while
leaving regular route creation with GFP_KERNEL.

Please let me know if you would prefer a different split.

> 
> When creating IPv4 and IPv6 nexthops, nh_create_ipv6() and fib_nh_init()
> explicitly pass GFP_KERNEL without __GFP_ACCOUNT down to the initialization
> functions:
> 
> net/ipv4/nexthop.c:nh_create_ipv6() {
>    ...
>    err = fib6_nh_init(net, fib6_nh, &fib6_cfg, GFP_KERNEL, extack);
>    ...
> }
> 
> This results in unaccounted per-CPU allocations:
> 
> net/ipv6/route.c:fib6_nh_init() {
>    ...
>    fib6_nh->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, gfp_flags);
>    ...
> }
> 
> A similar missing accounting happens for IPv4 in fib_nh_common_init():
> 
> net/ipv4/fib_semantics.c:fib_nh_common_init() {
>    ...
>    nhc->nhc_pcpu_rth_output = alloc_percpu_gfp(struct rtable __rcu *,
>                                                gfp_flags);
>    ...
> }
> 
> Could an unprivileged user with CAP_NET_ADMIN still bypass memcg limits by
> exhausting memory through these per-CPU allocations?
> 

Yes. I confirmed this in the QEMU/KVM Docker reproduction.

> 
> [Severity: High]
> This is a pre-existing issue, but do we also need to account for Lightweight
> Tunnel (LWT) state allocations?
> 
> During nexthop creation with encapsulation attributes, fib_nh_common_init()
> triggers LWT state allocation:
> 
> net/ipv4/fib_semantics.c:fib_nh_common_init() {
>    ...
>    err = lwtunnel_build_state(net, encap_type, encap,
>                               nhc->nhc_family, cfg, &lwtstate,
>                               extack);
>    ...
> }
> 
> The allocation unconditionally uses GFP_ATOMIC without __GFP_ACCOUNT:
> 
> net/core/lwtunnel.c:lwtunnel_state_alloc() {
>    ...
>    lws = kzalloc(sizeof(*lws) + encap_len, GFP_ATOMIC);
>    ...
> }
> 
> Could a task in a container bypass memory.max limits by creating many nexthops
> with encapsulation attributes?

Yes. I reproduced this with an SRv6 NHA_ENCAP nexthop workload in a
QEMU/KVM guest running Docker with memory.max=128M.

> 
>> if (!nhi) {
>> kfree(nh);
>> return ERR_PTR(-ENOMEM);

Regards,
Yizhou

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Yizhou Zhao]

Hi Jakub,

Thank you for both reviews. This is the reply for the second review.

> On Jul 30, 2026, at 08:39, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> --- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
> commit 1014ad1eb712722973916d53a1c5348624a80550
> Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> 
> nexthop: account nexthop allocations to memcg
> 
> This adds GFP_KERNEL_ACCOUNT / __GFP_ACCOUNT to the nexthop creation path
> allocations (struct nexthop, struct nh_info, group storage, resilient
> bucket table, per-group stats) and to the transient resilient-table
> notifier buffer in nh_notifier_res_table_info_init().  The stated goal is
> to make memory.max constrain nexthop creation by a CAP_NET_ADMIN task in
> a container.
> 
> A couple of questions below about the notifier-table hunk and about the
> tags.
> 
>> nexthop: account nexthop allocations to memcg
>> 
>> Fixes: ab84be7e54fc ("net: Initial nexthop code")
>> Fixes: f4676ea74b85 ("net: nexthop: Add nexthop group entry stats")
>> Cc: stable@vger.kernel.org
> 
> [Severity: Low]
> Are the Fixes: tags and the stable Cc appropriate here?  Switching
> allocations to GFP_KERNEL_ACCOUNT does not fix an incorrect result, it
> changes the success/failure profile of an existing uAPI operation.
> 
> After a stable backport, RTM_NEWNEXTHOP can start returning -ENOMEM
> inside a memory.max-constrained cgroup on kernels where it previously
> succeeded, which looks like a functional change for routing daemons
> running in containers.
> 
> The operation still requires CAP_NET_ADMIN in the netns owner's user
> namespace, so no privilege boundary is being closed - only accounting
> accuracy changes.  Would net-next without the Fixes:/Cc: stable tags be a
> better target, with the transient notifier hunk split out?

I agree that the fix changes the point at which RTM_NEWNEXTHOP can fail
under memory.max. However, I view that failure as the intended enforcement
of memory.max: the current uncharged allocations let a cgroup retain kernel
memory outside its configured hard limit.

For that reason, I had considered this a bug fix suitable for net, and
possibly stable once the patches are properly split and validated. Do you
prefer net-next because of the breadth and regression risk of the allocation
changes, rather than the -ENOMEM behavior itself? I will follow your
guidance on the target and tags.

> 
>> The nexthop creation path allocates struct nexthop, struct nh_info,
>> group storage, resilient bucket tables, transient notifier tables and
>> per-group stats without memcg accounting.
> 
> [Severity: Low]
> The changelog says "transient notifier tables" in the plural, but only
> one of the five transient notifier allocations in net/ipv4/nexthop.c is
> converted.  These remain on plain GFP_KERNEL:
> 
>  nh_notifier_single_info_init()
> info->nh = kzalloc_obj(*info->nh);
> 
>  nh_notifier_mpath_info_init()
> info->nh_grp = kzalloc_flex(*info->nh_grp, nh_entries, num_nh);
> 
>  nh_notifier_res_bucket_info_init()
> info->nh_res_bucket = kzalloc_obj(*info->nh_res_bucket);
> 
>  nh_notifier_grp_hw_stats_init()
> info->nh_grp_hw_stats = kzalloc_flex(*info->nh_grp_hw_stats, stats,
>     nhg->num_nh);
> 
> Since nh_notifier_grp_info_init() dispatches to
> nh_notifier_mpath_info_init() for hash_threshold (MPATH) groups, a group
> created through the exact path described in the changelog gets no
> notifier-table accounting at all, and call_nexthop_res_table_notifiers()
> also builds an unaccounted num_nh-sized mpath table during resilient
> group replace.
> 
> Could the policy be made all-or-nothing here, and the changelog adjusted
> to match what the code does?

Agreed. The partial notifier conversion and the corresponding changelog
wording are both incorrect.

After reviewing the lifetime and call contexts, I will make the policy here
"none": I will drop the notifier hunk rather than convert the remaining
notifier allocations or split it into a separate patch. These buffers are
temporary notification payloads, freed after the callback, rather than
persistent nexthop state. They can also be allocated during notifier replay
or deletion, where GFP_KERNEL_ACCOUNT would charge the current context
rather than the nexthop creator and could make notification delivery fail.

I will remove "transient notifier tables" from the commit message. The
revised patch will describe and account only persistent nexthop allocations.

> 
>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>> index 44fe75004cac..c8685c583579 100644
>> --- a/net/ipv4/nexthop.c
>> +++ b/net/ipv4/nexthop.c
>> @@ -169,7 +169,7 @@ static int nh_notifier_res_table_info_init(struct nh_notifier_info *info,
>> info->type = NH_NOTIFIER_INFO_TYPE_RES_TABLE;
>> size = struct_size(info->nh_res_table, nhs, num_nh_buckets);
>> info->nh_res_table = __vmalloc(size, GFP_KERNEL | __GFP_ZERO |
>> -       __GFP_NOWARN);
>> +       __GFP_NOWARN | __GFP_ACCOUNT);
>> if (!info->nh_res_table)
>> return -ENOMEM;
> 
> [Severity: Medium]
> Should this particular allocation be accounted at all?  It is not on the
> nexthop creation path the changelog describes.  The buffer only exists to
> hand resilient-group state to notifier listeners and is released before
> the notifier call returns:
> 
> net/ipv4/nexthop.c:nh_notifier_grp_info_fini() {
> if (nhg->hash_threshold)
> kfree(info->nh_grp);
> else if (nhg->resilient)
> vfree(info->nh_res_table);
> }
> 
> So it cannot accumulate, and charging it does not add containment.  Two
> side effects come with it though.
> 
> First, whose memcg gets charged?  The notifier paths run in the context
> of whoever triggers the notification, not the creator of the nexthop:
> 
>  remove_nexthop() -> call_nexthop_notifiers(NEXTHOP_EVENT_DEL)
> (also reached via nh_netdev_event -> nexthop_flush_dev)
>  replace_nexthop_single() err_notify -> call_nexthop_notifiers(REPLACE)
>  remove_nh_grp_entry() -> call_nexthop_notifiers(REPLACE)
>  register_nexthop_notifier() / __unregister_nexthop_notifier()
> -> nexthops_dump() -> nh_notifier_info_init() for every
>   existing nexthop
> 
> Can a cgroup end up charged here for nexthops belonging to other
> cgroups, for example when a driver module registers a nexthop notifier
> and the replay loop walks every resilient nexthop in the netns?
> 
> Second, does this make notifier delivery fail on demand?  __GFP_ACCOUNT
> is honoured for vmalloc pages, so with a memcg at memory.max this
> __vmalloc() can now fail while the system is globally healthy, and
> __GFP_NOWARN keeps it quiet.  The failure happens before the chain is
> called:
> 
> net/ipv4/nexthop.c:call_nexthop_notifiers() {
> err = nh_notifier_info_init(&info, nh);
> if (err) {
> NL_SET_ERR_MSG(extack, "Failed to initialize nexthop notifier info");
> return err;
> }
> 
> and the callers do not stop on that error:
> 
> net/ipv4/nexthop.c:remove_nexthop() {
> call_nexthop_notifiers(net, NEXTHOP_EVENT_DEL, nh, NULL);
> 
> /* remove from the tree */
> rb_erase(&nh->rb_node, &net->nexthop.rb_root);
> 
> The return value is discarded and the nexthop is erased and freed
> anyway.  replace_nexthop_single()'s err_notify path also discards it,
> remove_nh_grp_entry() only pr_err()s, and __unregister_nexthop_notifier()
> ignores nexthops_dump()'s status while nexthops_dump() breaks out at the
> first error, skipping DEL for all remaining nexthops.
> 
> The fail-silent error handling in those callers is pre-existing and not
> introduced by this patch, but does this change turn it into something a
> task can trigger at will from inside a constrained cgroup?  With an
> offload listener such as mlxsw or netdevsim registered, that would leave
> driver state for nexthops the kernel has already freed, and would let
> register_nexthop_notifier() be made to fail for the whole netns.
> 
> Would keeping nh_notifier_res_table_info_init() on plain GFP_KERNEL and
> accounting only the persistent objects (struct nexthop, struct nh_info,
> struct nh_group, res_table, per-group stats) avoid all of this?

I agree that this allocation should remain on plain GFP_KERNEL.

__GFP_ACCOUNT would charge the vmalloc backing pages to the current memcg,
not to the cgroup which created the resilient nexthop. Therefore notifier
replay or deletion could charge an unrelated cgroup while walking existing
nexthops in the netns.

It could also make nh_notifier_info_init() fail before the notifier chain is
called when the current cgroup is at memory.max. Since some callers discard
that error and continue, this could skip notification for an object which is
then removed or otherwise changed.

I will remove this hunk rather than account the other notifier allocations,
and remove the notifier-table wording from the changelog. The revised patch
will account only persistent nexthop state, for which the allocation context
and lifetime are directly attributable to the nexthop operation.

> 
> [ ... ]
> -- 
> pw-bot: cr

Please let me know if further changes are needed.

Thanks,
Yizhou

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Ido Schimmel]

On Sat, Aug 01, 2026 at 02:56:29PM +0800, Yizhou Zhao wrote:
> Hi Jakub,
> 
> Thank you for both reviews. This is the reply for the first one.
> 
> > On Jul 30, 2026, at 08:38, Jakub Kicinski <kuba@kernel.org> wrote:
> > 
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > ---
> > --- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
> > commit 186a5b6499e65039b5037906ec518baac2833b1d
> > Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> > 
> >    nexthop: account nexthop allocations to memcg
> > 
> >    The nexthop creation path allocates struct nexthop, struct nh_info,
> >    group storage, resilient bucket tables, transient notifier tables and
> >    per-group stats without memcg accounting. As a result, a task with
> >    CAP_NET_ADMIN can create nexthops and resilient groups whose kernel
> >    memory is not charged to the task's memory cgroup, so memory.max does
> >    not constrain this part of the workload.
> > 
> >> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> >> index 44fe75004cacb..c8685c5835794 100644
> >> --- a/net/ipv4/nexthop.c
> >> +++ b/net/ipv4/nexthop.c
> > [ ... ]
> >> @@ -2943,7 +2945,7 @@ static struct nexthop *nexthop_create(struct net *net, struct nh_config *cfg,
> >> if (!nh)
> >> return ERR_PTR(-ENOMEM);
> >> 
> >> - nhi = kzalloc_obj(*nhi);
> >> + nhi = kzalloc_obj(*nhi, GFP_KERNEL_ACCOUNT);
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does this patch miss some per-CPU
> > allocations in the nexthop creation path?
> 
> Yes. I confirmed that the original patch misses these allocations.
> 
> With only the persistent nexthop objects accounted, IPv4 and IPv6 nexthop
> creation can still allocate nhc_pcpu_rth_output and rt6i_pcpu without memcg
> accounting, so memory.max does not constrain that memory.
> 
> We will fix this in the next revision by passing GFP_KERNEL_ACCOUNT through
> fib_nh_init() and fib6_nh_init() for persistent nexthop creation, while
> leaving regular route creation with GFP_KERNEL.
> 
> Please let me know if you would prefer a different split.

What is the reason for the split? Why only charge the nexthop when it's
created via the nexthop API (e.g., ip nexthop add ...), but not when
it's implicitly created via the route API (e.g., ip route add ...)?

Re: [PATCH net] nexthop: account nexthop allocations to memcg

[Ido Schimmel]

On Sat, Aug 01, 2026 at 03:13:46PM +0800, Yizhou Zhao wrote:
> Hi Jakub,
> 
> Thank you for both reviews. This is the reply for the second review.
> 
> > On Jul 30, 2026, at 08:39, Jakub Kicinski <kuba@kernel.org> wrote:
> > 
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > ---
> > --- Patch [1]: [PATCH net] nexthop: account nexthop allocations to memcg ---
> > commit 1014ad1eb712722973916d53a1c5348624a80550
> > Author: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> > 
> > nexthop: account nexthop allocations to memcg
> > 
> > This adds GFP_KERNEL_ACCOUNT / __GFP_ACCOUNT to the nexthop creation path
> > allocations (struct nexthop, struct nh_info, group storage, resilient
> > bucket table, per-group stats) and to the transient resilient-table
> > notifier buffer in nh_notifier_res_table_info_init().  The stated goal is
> > to make memory.max constrain nexthop creation by a CAP_NET_ADMIN task in
> > a container.
> > 
> > A couple of questions below about the notifier-table hunk and about the
> > tags.
> > 
> >> nexthop: account nexthop allocations to memcg
> >> 
> >> Fixes: ab84be7e54fc ("net: Initial nexthop code")
> >> Fixes: f4676ea74b85 ("net: nexthop: Add nexthop group entry stats")
> >> Cc: stable@vger.kernel.org
> > 
> > [Severity: Low]
> > Are the Fixes: tags and the stable Cc appropriate here?  Switching
> > allocations to GFP_KERNEL_ACCOUNT does not fix an incorrect result, it
> > changes the success/failure profile of an existing uAPI operation.
> > 
> > After a stable backport, RTM_NEWNEXTHOP can start returning -ENOMEM
> > inside a memory.max-constrained cgroup on kernels where it previously
> > succeeded, which looks like a functional change for routing daemons
> > running in containers.
> > 
> > The operation still requires CAP_NET_ADMIN in the netns owner's user
> > namespace, so no privilege boundary is being closed - only accounting
> > accuracy changes.  Would net-next without the Fixes:/Cc: stable tags be a
> > better target, with the transient notifier hunk split out?
> 
> I agree that the fix changes the point at which RTM_NEWNEXTHOP can fail
> under memory.max. However, I view that failure as the intended enforcement
> of memory.max: the current uncharged allocations let a cgroup retain kernel
> memory outside its configured hard limit.
> 
> For that reason, I had considered this a bug fix suitable for net, and
> possibly stable once the patches are properly split and validated. Do you
> prefer net-next because of the breadth and regression risk of the allocation
> changes, rather than the -ENOMEM behavior itself? I will follow your
> guidance on the target and tags.

The patch does not fix a regression (accounting nexthop objects never
worked) and does not fix a crash, so I vote for net-next. Also, similar
patches were applied to net-next in the past and without a Fixes tag:

425b9c7f51c9 ("memcg: accounting for objects allocated for new netdevice")
5d26cff5bdbe ("net: account alternate interface name memory")
6126891c6d4f ("memcg: enable accounting for IP address and routing-related objects")