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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxxx
> > 
> > [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")