netdev - Re: [net-next PATCH RFC] mlx4: RX prefetch loop

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160711230509.GA45195@ast-mbp.thefacebook.com>
Date:	Mon, 11 Jul 2016 16:05:11 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Jesper Dangaard Brouer <brouer@...hat.com>
Cc:	netdev@...r.kernel.org, kafai@...com, daniel@...earbox.net,
	tom@...bertland.com, bblanco@...mgrid.com,
	john.fastabend@...il.com, gerlitz.or@...il.com,
	hannes@...essinduktion.org, rana.shahot@...il.com, tgraf@...g.ch,
	"David S. Miller" <davem@...emloft.net>, as754m@....com,
	saeedm@...lanox.com, amira@...lanox.com, tzahio@...lanox.com,
	Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [net-next PATCH RFC] mlx4: RX prefetch loop

On Mon, Jul 11, 2016 at 01:09:22PM +0200, Jesper Dangaard Brouer wrote:
> > -	/* Process all completed CQEs */
> > +	/* Extract and prefetch completed CQEs */
> >  	while (XNOR(cqe->owner_sr_opcode & MLX4_CQE_OWNER_MASK,
> >  		    cq->mcq.cons_index & cq->size)) {
> > +		void *data;
> >  
> >  		frags = ring->rx_info + (index << priv->log_rx_info);
> >  		rx_desc = ring->buf + (index << ring->log_stride);
> > +		prefetch(rx_desc);
> >  
> >  		/*
> >  		 * make sure we read the CQE after we read the ownership bit
> >  		 */
> >  		dma_rmb();
> >  
> > +		cqe_array[cqe_idx++] = cqe;
> > +
> > +		/* Base error handling here, free handled in next loop */
> > +		if (unlikely((cqe->owner_sr_opcode & MLX4_CQE_OPCODE_MASK) ==
> > +			     MLX4_CQE_OPCODE_ERROR))
> > +			goto skip;
> > +
> > +		data = page_address(frags[0].page) + frags[0].page_offset;
> > +		prefetch(data);

that's probably not correct in all cases, since doing prefetch on the address
that is going to be evicted soon may hurt performance.
We need to dma_sync_single_for_cpu() before doing a prefetch or
somehow figure out that dma_sync is a nop, so we can omit it altogether
and do whatever prefetches we like.
Also unconditionally doing batch of 8 may also hurt depending on what
is happening either with the stack, bpf afterwards or even cpu version.
Doing single prefetch of Nth packet is probably ok most of the time,
but asking cpu to prefetch 8 packets at once is unnecessary especially
since single prefetch gives the same performance.