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

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

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.

>  			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
>