[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CA+KdSGNvJ3np3gitnSmztxD9n=WuNcRHGsvKsJUE+FDJM6Zmjw@mail.gmail.com>
Date: Sat, 24 Jan 2026 09:50:02 +0330
From: Mahdi Faramarzpour <mahdifrmx@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, dsahern@...nel.org,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
kshitiz.bartariya@...omail.in
Subject: Re: [PATCH net-next] udp: add drop count for packets in udp_prod_queue
On Fri, Jan 23, 2026 at 12:56 AM Willem de Bruijn
<willemdebruijn.kernel@...il.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@...il.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?
>
> > to_drop = skb->next;
> > skb_mark_not_on_list(skb);
> > - /* TODO: update SNMP values. */
> > sk_skb_reason_drop(sk, skb, SKB_DROP_REASON_PROTO_MEM);
> > }
> > numa_drop_add(&udp_sk(sk)->drop_counters, nb);
> > + if (err_ipv4 > 0) {
> > + SNMP_ADD_STATS(__UDPX_MIB(sk, true), UDP_MIB_MEMERRORS,
> > + err_ipv4);
> > + SNMP_ADD_STATS(__UDPX_MIB(sk, true), UDP_MIB_INERRORS,
> > + err_ipv4);
> > + }
> > + if (err_ipv6 > 0) {
> > + SNMP_ADD_STATS(__UDPX_MIB(sk, false), UDP_MIB_MEMERRORS,
> > + err_ipv6);
> > + SNMP_ADD_STATS(__UDPX_MIB(sk, false), UDP_MIB_INERRORS,
> > + err_ipv6);
> > + }
> > }
> >
> > atomic_sub(total_size, &udp_prod_queue->rmem_alloc);
> > --
> > 2.34.1
> >
>
>
Powered by blists - more mailing lists