Re: [PATCH net-next 3/4] net: ethernet: mtk_eth_soc: introduce xdp ethtool counters

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

[...]
> 
> This is allocating on the stack and clearing a relatively large struct
> for every poll() call, which is not good.
> 
> Why can't you touch directly the eth->mac[i]->hw_stats.xdp_stats
> counters where needed?

I am currently relying on xdp_stats to flush xdp maps but I can rework a bit
the code to remove this dependency. I will fix it in v2.

Regards,
Lorenzo

> 
> >  	struct bpf_prog *prog = READ_ONCE(eth->prog);
> >  	struct dim_sample dim_sample = {};
> >  	struct mtk_rx_ring *ring;
> > @@ -1535,7 +1574,6 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
> >  	struct sk_buff *skb;
> >  	u8 *data, *new_data;
> >  	struct mtk_rx_dma_v2 *rxd, trxd;
> > -	bool xdp_do_redirect = false;
> >  	int done = 0, bytes = 0;
> >  
> >  	while (done < budget) {
> > @@ -1597,12 +1635,10 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
> >  					 false);
> >  			xdp_buff_clear_frags_flag(&xdp);
> >  
> > -			ret = mtk_xdp_run(ring, prog, &xdp, netdev);
> > -			if (ret != XDP_PASS) {
> > -				if (ret == XDP_REDIRECT)
> > -					xdp_do_redirect = true;
> > +			ret = mtk_xdp_run(ring, prog, &xdp, netdev,
> > +					  &xdp_stats[mac]);
> > +			if (ret != XDP_PASS)
> >  				goto skip_rx;
> > -			}
> >  
> >  			skb = build_skb(data, PAGE_SIZE);
> >  			if (unlikely(!skb)) {
> > @@ -1725,8 +1761,8 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
> >  			  &dim_sample);
> >  	net_dim(&eth->rx_dim, dim_sample);
> >  
> > -	if (prog && xdp_do_redirect)
> > -		xdp_do_flush_map();
> > +	if (prog)
> > +		mtk_xdp_rx_complete(eth, xdp_stats);
> >  
> >  	return done;
> >  }
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > index a1cea93300c1..629cdcdd632a 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > @@ -570,6 +570,16 @@ struct mtk_tx_dma_v2 {
> >  struct mtk_eth;
> >  struct mtk_mac;
> >  
> > +struct mtk_xdp_stats {
> > +	u64 rx_xdp_redirect;
> > +	u64 rx_xdp_pass;
> > +	u64 rx_xdp_drop;
> > +	u64 rx_xdp_tx;
> > +	u64 rx_xdp_tx_errors;
> > +	u64 tx_xdp_xmit;
> > +	u64 tx_xdp_xmit_errors;
> > +};
> > +
> >  /* struct mtk_hw_stats - the structure that holds the traffic statistics.
> >   * @stats_lock:		make sure that stats operations are atomic
> >   * @reg_offset:		the status register offset of the SoC
> > @@ -593,6 +603,8 @@ struct mtk_hw_stats {
> >  	u64 rx_checksum_errors;
> >  	u64 rx_flow_control_packets;
> >  
> > +	struct mtk_xdp_stats	xdp_stats;
> > +
> >  	spinlock_t		stats_lock;
> >  	u32			reg_offset;
> >  	struct u64_stats_sync	syncp;
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]