netdev - RE: [Intel-wired-lan] [PATCH iwl-net v3 4/6] idpf: replace flow scheduling buffer ring with buffer pool

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]

Message-ID: <DM4PR11MB65026B231AAA91BD5E8DF6ACD422A@DM4PR11MB6502.namprd11.prod.outlook.com>
Date: Tue, 5 Aug 2025 22:40:19 +0000
From: "Hay, Joshua A" <joshua.a.hay@...el.com>
To: "Lobakin, Aleksander" <aleksander.lobakin@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Luigi Rizzo
	<lrizzo@...gle.com>, Brian Vazquez <brianvv@...gle.com>, "Chittim, Madhu"
	<madhu.chittim@...el.com>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net v3 4/6] idpf: replace flow
 scheduling buffer ring with buffer pool

> > -static bool idpf_tx_clean_buf_ring(struct idpf_tx_queue *txq, u16
> compl_tag,
> > -				   struct libeth_sq_napi_stats *cleaned,
> > -				   int budget)
> > +static bool idpf_tx_clean_bufs(struct idpf_tx_queue *txq, u32 buf_id,
> > +			       struct libeth_sq_napi_stats *cleaned,
> > +			       int budget)
> >  {
> > -	u16 idx = compl_tag & txq->compl_tag_bufid_m;
> >  	struct idpf_tx_buf *tx_buf = NULL;
> >  	struct libeth_cq_pp cp = {
> >  		.dev	= txq->dev,
> >  		.ss	= cleaned,
> >  		.napi	= budget,
> >  	};
> > -	u16 ntc, orig_idx = idx;
> > -
> > -	tx_buf = &txq->tx_buf[idx];
> > -
> > -	if (unlikely(tx_buf->type <= LIBETH_SQE_CTX ||
> > -		     idpf_tx_buf_compl_tag(tx_buf) != compl_tag))
> > -		return false;
> >
> > +	tx_buf = &txq->tx_buf[buf_id];
> >  	if (tx_buf->type == LIBETH_SQE_SKB) {
> >  		if (skb_shinfo(tx_buf->skb)->tx_flags & SKBTX_IN_PROGRESS)
> >  			idpf_tx_read_tstamp(txq, tx_buf->skb);
> >
> >  		libeth_tx_complete(tx_buf, &cp);
> > +		idpf_post_buf_refill(txq->refillq, buf_id);
> >  	}
> >
> > -	idpf_tx_clean_buf_ring_bump_ntc(txq, idx, tx_buf);
> > +	while (idpf_tx_buf_next(tx_buf) != IDPF_TXBUF_NULL) {
> > +		u16 buf_id = idpf_tx_buf_next(tx_buf);
> 
> This line adds a new -Wshadow warning. This function has an argument
> named 'buf_id' and here you declare a variable with the same name.
> While -Wshadow gets enabled only with W=2, it would be nice if you don't
> introduce it anyway.
> If I understand the code correctly, you can just remove that `u16` since
> you don't use the former buf_id at this point anyway.

Ah, good catch, will fix. Correct, the u16 can just be removed.

> 
> >
> > -	while (idpf_tx_buf_compl_tag(tx_buf) == compl_tag) {
> > +		tx_buf = &txq->tx_buf[buf_id];
> >  		libeth_tx_complete(tx_buf, &cp);
> > -		idpf_tx_clean_buf_ring_bump_ntc(txq, idx, tx_buf);
> > +		idpf_post_buf_refill(txq->refillq, buf_id);
> >  	}
> 
> Thanks,
> Olek

Thanks,
Josh