Re: [PATCH v4 2/2] net: fec: add xdp and page pool statistics

[Andrew Lunn]

> @@ -1582,6 +1586,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
>  	struct bpf_prog *xdp_prog = READ_ONCE(fep->xdp_prog);
>  	u32 ret, xdp_result = FEC_ENET_XDP_PASS;
>  	u32 data_start = FEC_ENET_XDP_HEADROOM;
> +	u32 xdp_stats[XDP_STATS_TOTAL];
>  	struct xdp_buff xdp;
>  	struct page *page;
>  	u32 sub_len = 4;
> @@ -1656,11 +1661,13 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
>  		fec_enet_update_cbd(rxq, bdp, index);
>  
>  		if (xdp_prog) {
> +			memset(xdp_stats, 0, sizeof(xdp_stats));
>  			xdp_buff_clear_frags_flag(&xdp);
>  			/* subtract 16bit shift and FCS */
>  			xdp_prepare_buff(&xdp, page_address(page),
>  					 data_start, pkt_len - sub_len, false);
> -			ret = fec_enet_run_xdp(fep, xdp_prog, &xdp, rxq, index);
> +			ret = fec_enet_run_xdp(fep, xdp_prog, &xdp, rxq,
> +					       xdp_stats, index);
>  			xdp_result |= ret;
>  			if (ret != FEC_ENET_XDP_PASS)
>  				goto rx_processing_done;
> @@ -1762,6 +1769,15 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
>  	if (xdp_result & FEC_ENET_XDP_REDIR)
>  		xdp_do_flush_map();
>  
> +	if (xdp_prog) {
> +		int i;
> +
> +		u64_stats_update_begin(&rxq->syncp);
> +		for (i = 0; i < XDP_STATS_TOTAL; i++)
> +			rxq->stats[i] += xdp_stats[i];
> +		u64_stats_update_end(&rxq->syncp);
> +	}
> +

This looks wrong. You are processing upto the napi budget, 64 frames,
in a loop. The memset to 0 happens inside the loop, but you do the
accumulation outside the loop?

This patch is getting pretty big. Please break it up, at least into
one patch for XDP stats and one for page pool stats.

    Andrew