[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+KdSGOW0+V9KTA6CebvJ5dSqBxCV5XFAJshJByQ36=GWX6yiQ@mail.gmail.com>
Date: Thu, 8 Jan 2026 01:16:39 +0330
From: Mahdi Faramarzpour <mahdifrmx@...il.com>
To: 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
On Wed, Jan 7, 2026 at 6:39 PM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> 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
>
Ok.
> > > 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?
>
Right.
> 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.
>
The difference comes from the intermediate udp_prod_queue which contains
packets from calls to __udp_enqueue_schedule_skb that reached this branch:
if (!llist_add(&skb->ll_node, &udp_prod_queue->ll_root))
return 0;
these packets might be dropped in batch later by the call that reaches the
unlikely(to_drop) branch, and thus SNMP stats must increase. Note that such
packets are only dropped due to the ENOBUFS returned from udp_rmem_schedule.
> > > > >
> > > > > 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.
>
>
Powered by blists - more mailing lists