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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Mahdi Faramarzpour wrote:
> On Fri, Jan 23, 2026 at 12:56 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > 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"). note that SNMP
> > > counters are incremented currently by the caller for skb. And
> > > that these skbs on the intermediate queue cannot be counted
> > > there so need similar logic in their error path.
> > >
> > > Signed-off-by: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> > > ---
> > > v5:
> > >   - check if drop counts are non-zero before increasing countrers
> > > v4: https://lore.kernel.org/netdev/20260108102950.49417-1-mahdifrmx@gmail.com/
> > >   - move all changes to unlikely(to_drop) branch
> > > v3: https://lore.kernel.org/netdev/20260105114732.140719-1-mahdifrmx@gmail.com/
> > >   - remove the unreachable UDP_MIB_RCVBUFERRORS code
> > > v2: https://lore.kernel.org/netdev/20260105071218.10785-1-mahdifrmx@gmail.com/
> > >   - change ENOMEM to ENOBUFS
> > > v1: https://lore.kernel.org/netdev/20260104105732.427691-1-mahdifrmx@gmail.com/
> > > ---
> > >  net/ipv4/udp.c | 19 ++++++++++++++++++-
> > >  1 file changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > index ffe074cb5..41cf8f7ab 100644
> > > --- a/net/ipv4/udp.c
> > > +++ b/net/ipv4/udp.c
> > > @@ -1793,14 +1793,31 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> > >       }
> > >
> > >       if (unlikely(to_drop)) {
> > > +             int err_ipv4 = 0;
> > > +             int err_ipv6 = 0;
> >
> > nit: whitespace between variable definition and code block
> >
> > >               for (nb = 0; to_drop != NULL; nb++) {
> > >                       skb = to_drop;
> > > +                     if (skb->protocol == htons(ETH_P_IP))
> > > +                             err_ipv4++;
> > > +                     else
> > > +                             err_ipv6++;
> >
> > No need for separate counters.
> >
> > All counters will either update IPv4 or IPv6 stats. Similar to how
> > the caller of _udp_enqueue_schedule_sk is either __udp_queue_rcv_skb
> > or __udpv6_queue_rcv_skb and chooses the SNMP stat based on that.
> >
> > Can check sk_family == PF_INET6 once.
> I doubt this. How does this work in case of a dual-stack socket?

I see your point. The question is whether the stats represent socket
family or individual datagrams arriving on that socket. There is
something to be said for the latter.

But given that __udpv6_queue_rcv_skb unconditionally increments UDP6
stats on drop in __udp_enqueue_schedule_skb, the established behavior
seems to be to follow the socket family.