All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Mahdi Faramarzpour <mahdifrmx@gmail.com>,
	 Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	 netdev@vger.kernel.org,  davem@davemloft.net,
	 dsahern@kernel.org,  edumazet@google.com,  pabeni@redhat.com,
	 horms@kernel.org
Subject: Re: [PATCH net-next] udp: add drop count for packets in udp_prod_queue
Date: Wed, 07 Jan 2026 10:09:57 -0500	[thread overview]
Message-ID: <willemdebruijn.kernel.21c4d3b7b8f9d@gmail.com> (raw)
In-Reply-To: <CA+KdSGOzzb=vMWh6UG-OFSQgEapS4Ckwf5K8hwYy8hz4N9RVMg@mail.gmail.com>

Mahdi Faramarzpour wrote:
> On Tue, Jan 6, 2026 at 10:52 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Mahdi Faramarzpour wrote:
> > > On Tue, Jan 6, 2026 at 5:24 AM Jakub Kicinski <kuba@kernel.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.



  reply	other threads:[~2026-01-07 15:09 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-05 11:47 [PATCH net-next] udp: add drop count for packets in udp_prod_queue Mahdi Faramarzpour
2026-01-06  1:54 ` Jakub Kicinski
2026-01-06  6:11   ` Mahdi Faramarzpour
2026-01-06 19:22     ` Willem de Bruijn
2026-01-07  9:27       ` Mahdi Faramarzpour
2026-01-07 15:09         ` Willem de Bruijn [this message]
2026-01-07 21:46           ` Mahdi Faramarzpour
2026-01-07 22:37             ` Willem de Bruijn
2026-01-06 23:02     ` Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2026-01-29  8:38 Mahdi Faramarzpour
2026-01-29 17:17 ` Willem de Bruijn
2026-01-29 17:28   ` Eric Dumazet
2026-01-31  1:40 ` patchwork-bot+netdevbpf
2026-01-28  7:03 Mahdi Faramarzpour
2026-01-28 11:43 ` Eric Dumazet
2026-01-28 12:27   ` Mahdi Faramarzpour
2026-01-28 12:37     ` Eric Dumazet
2026-01-28 14:56       ` Mahdi Faramarzpour
2026-01-28 18:54         ` Willem de Bruijn
2026-01-22 18:53 Mahdi Faramarzpour
2026-01-22 21:26 ` Willem de Bruijn
2026-01-23  8:14   ` Paolo Abeni
2026-01-23 14:41     ` Willem de Bruijn
2026-01-23 15:25       ` Paolo Abeni
2026-01-24 15:36         ` Willem de Bruijn
2026-01-24  6:20   ` Mahdi Faramarzpour
2026-01-24 15:32     ` Willem de Bruijn
2026-01-08 10:29 Mahdi Faramarzpour
2026-01-08 15:05 ` Willem de Bruijn
2026-01-05  7:12 Mahdi Faramarzpour
2026-01-04 10:57 Mahdi Faramarzpour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=willemdebruijn.kernel.21c4d3b7b8f9d@gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=mahdifrmx@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.