On Fri, Jul 11, 2014 at 03:29:17PM +0000, Venkatesan, Venky wrote: > > -----Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of John W. > > Linville > > Sent: Friday, July 11, 2014 7:49 AM > > To: Stephen Hemminger > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] librte_pmd_packet: add PMD for > > AF_PACKET- based virtual devices > > > > On Fri, Jul 11, 2014 at 06:11:47AM -0700, Stephen Hemminger wrote: > > > On Thu, 10 Jul 2014 16:32:49 -0400 "John W. Linville" > > > <linville at tuxdriver.com> wrote: > > > > > > > This is a Linux-specific virtual PMD driver backed by an > > > > AF_PACKET
<snip> > > > > +struct pkt_rx_queue { > > > > + int sockfd; > > > > + > > > > + struct iovec *rd; > > > > + uint8_t *map; > > > > + unsigned int framecount; > > > > + unsigned int framenum; > > > > + > > > > + struct rte_mempool *mb_pool; > > > > + > > > > + volatile unsigned long rx_pkts; > > > > + volatile unsigned long err_pkts; > > > > > > Use of volatile will generate slow code, don't think it is > > > necessary, especially when only one CPU can use a queue at a time. > > > > That is a good point, worth checking out. FWIW, those lines are > > boilerplate originally copied from the pcap PMD. :-) > > > > > Yes, I agree it's worth checking out if there is a performance > > impact, but if we assume that the stats for RX/TX are possibly going > > to be read by another core, they really should be volatile for > > correctness > > Accessing the rx_queue structure directly for stats is unlikely to happen > from a second core; we should probably change the PCAP PMD as well (thanks > for pointing that out John). > "Unlikely" doesn't sound completely safe... :-) LOL. :-). This is an internal data structure and the DPDK docs specifically mention that they are not multi-process safe/accessible. The unlikely was for people that don't read the docs ... ;)