netdev - Re: [PATCH net-next 3/4] net: ethernet: mtk_eth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 12 Jul 2022 18:17:17 +0200
From:   Lorenzo Bianconi <lorenzo@...nel.org>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     netdev@...r.kernel.org, nbd@....name, john@...ozen.org,
        sean.wang@...iatek.com, Mark-MC.Lee@...iatek.com,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        matthias.bgg@...il.com, linux-mediatek@...ts.infradead.org,
        ilias.apalodimas@...aro.org, lorenzo.bianconi@...hat.com,
        jbrouer@...hat.com
Subject: Re: [PATCH net-next 3/4] net: ethernet: mtk_eth_soc: introduce xdp
 ethtool counters

[...]
> 
> 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;
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)