netdev - Re: [PATCH net-next] udp: add drop count for packets in udp_prod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <willemdebruijn.kernel.21c4d3b7b8f9d@gmail.com>
Date: Wed, 07 Jan 2026 10:09:57 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Mahdi Faramarzpour <mahdifrmx@...il.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, 
 netdev@...r.kernel.org, 
 davem@...emloft.net, 
 dsahern@...nel.org, 
 edumazet@...gle.com, 
 pabeni@...hat.com, 
 horms@...nel.org
Subject: Re: [PATCH net-next] udp: add drop count for packets in
 udp_prod_queue

Mahdi Faramarzpour wrote:
> On Tue, Jan 6, 2026 at 10:52 PM Willem de Bruijn
> <willemdebruijn.kernel@...il.com> wrote:
> >
> > Mahdi Faramarzpour wrote:
> > > On Tue, Jan 6, 2026 at 5:24 AM Jakub Kicinski <kuba@...nel.org> wrote:
> > > >
> > > > On Mon,  5 Jan 2026 15:17:32 +0330 Mahdi Faramarzpour wrote:
> > > > > This commit adds SNMP drop count increment for the packets in
> > > > > per NUMA queues which were introduced in commit b650bf0977d3
> > > > > ("udp: remove busylock and add per NUMA queues").
> >
> > Can you give some rationale why the existing counters are insufficient
> > and why you chose to change then number of counters you suggest
> > between revisions of your patch?
> >
> The difference between revisions is due to me realizing that the only error the
> udp_rmem_schedule returns is ENOBUFS, which is mapped to UDP_MIB_MEMERRORS
> (refer to function __udp_queue_rcv_skb), and thus UDP_MIB_RCVBUFERRORS
> need not increase.

I see. Please make such a note in the revision changelog. See also

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#changes-requested

> > This code adds some cost to the hot path. The blamed commit added
> > drop counters, most likely weighing the value of counters against
> > their cost. I don't immediately see reason to revisit that.
> >
> AFAIU the drop_counter is per socket, while the counters added in this
> patch correspond
> to /proc/net/{snmp,snmp6} pseudofiles. This patch implements the todo
> comment added in
> the blamed commit.

Ah indeed.

The entire logic can be inside the unlikely(to_drop) branch right?
No need to initialize the counters in the hot path, or do the
skb->protocol earlier?

The previous busylock approach could also drop packets at this stage
(goto uncharge_drop), and the skb is also dropped if exceeding rcvbuf.
Neither of those conditions update SNMP stats. I'd like to understand
what makes this case different.

> > > >
> > > > You must not submit more than one version of a patch within a 24h
> > > > period.
> > > Hi Jakub and sorry for the noise, didn't know that. Is there any way to check
> > > my patch against all patchwork checks ,specially the AI-reviewer
> > > before submitting it?
> >
> > See https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> >
> thanks.