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 Wed, Jan 28, 2026 at 4:07 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Jan 28, 2026 at 1:27 PM Mahdi Faramarzpour <mahdifrmx@gmail.com> wrote:
> > >
> > > On Wed, Jan 28, 2026 at 3:14 PM Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > > On Wed, Jan 28, 2026 at 8:03 AM Mahdi Faramarzpour <mahdifrmx@gmail.com> 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>
> > > > > ---
> > > > > v6:
> > > > >   - increasing a single counter based on socket family
> > > > > v5: https://lore.kernel.org/netdev/20260122185357.50922-1-mahdifrmx@gmail.com/
> > > > >   - 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 | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > > > index ffe074cb5..dd302f5fb 100644
> > > > > --- a/net/ipv4/udp.c
> > > > > +++ b/net/ipv4/udp.c
> > > > > @@ -1797,10 +1797,13 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> > > > >                         skb = to_drop;
> > > > >                         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);
> > > > > +               SNMP_ADD_STATS(__UDPX_MIB(sk, (sk->sk_family == PF_INET)),
> > > > > +                              UDP_MIB_MEMERRORS, nb);
> > > > > +               SNMP_ADD_STATS(__UDPX_MIB(sk, (sk->sk_family == PF_INET)),
> > > > > +                              UDP_MIB_INERRORS, nb);
> > > > >         }
> > > >
> > > > Hmm.. I have not followed your prior attempts, sorry for coming late.
> > > >
> > > > Note that an UDP AF_INET6 socket for which ipv6_only_sock(sk) is false
> > > > can receive
> > > > IPv4 and IPv6 UDP packets, if bound to any addresses.
> > > >
> > > > The to_drop list could contain a mix of IPv4 and IPv6 packets.
> > > >
> > > > Ideally we should track SNMP values in the proper space (Ipv4 , IPv6).
> > > >
> > > > See IPV6_V6ONLY and /proc/sys/net/ipv6/bindv6only for reference.
> > > This is the exact discussion I had with Willem de Bruijn:
> > > https://lore.kernel.org/netdev/willemdebruijn.kernel.17f36003699cb@gmail.com/
> >
> > I think Willem was wrong :)
> >
> > __udp_enqueue_schedule_skb() is called either from
> > __udp_queue_rcv_skb() for IPv4 packets,
> > or __udpv6_queue_rcv_skb() for IPv6 packets, and they both can feed
> > packets to a single and shared UDP socket.
> >
> > An IPv4 packet will not go through __udpv6_queue_rcv_skb().

Happy to be wrong :) Thanks for the correction.

> >
> > It would be deceptive to increment IPV6 UDP counters if a host only
> > receives IPv4 UDP packets.
> Ok, in case other people wonder which logic is in fact true, refer to
> net/core/dev.c in which
> netif_receive_skb is defined that's called from device drivers. This
> routine checks the packet
> against a list of struct packet_types, ultimately finding the matched
> type and calling its
> registered func field, that are ip_rcv & ip6_rcv in case of the two
> ipv4 & ipv6 types. So, the
> appropriate routine is chosen based on packet type, not socket family.
> 
> That being said, I think the previous patch was just fine, unless
> Willem wants the newline
> after variable declarations:
> https://lore.kernel.org/netdev/20260122185357.50922-1-mahdifrmx@gmail.com/


You'll have to resubmit anyway. Then please address the minor
stylistic point.