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@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 186a5b6499e65039b5037906ec518baac2833b1d
> > Author: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
> > 
> >    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 ...)?