Re: [dpdk-dev] [PATCH 19.08 v3 2/2] net/pcap: enable infinitely rxing a pcap file

["Ferriter, Cian" <cian.ferriter@intel.com>]

Hi Ferruh,

I've responded inline but to summarize, I agree with both of your points.

There is one additional piece of rework that I spotted for this version. I have left out a small part of the commit message. Please see my response inline below for more details.

Since the rework is minor here, I'd be happy if you wanted to make the changes and apply the patches to save going through another version + review cycle.

Let me know if there is anything else.
Thanks,
Cian

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: 27 June 2019 18:56
> To: Ferriter, Cian <cian.ferriter@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 19.08 v3 2/2] net/pcap: enable infinitely rxing a pcap file
> 
> On 6/14/2019 3:43 PM, Cian Ferriter wrote:
> > It can be useful to use pcap files for some rudimental performance
> >

Part of the commit message got lost in my splitting of the patch into two patches between v2 and v3. The above line should read:
It can be useful to use pcap files for some rudimental performance
testing. This patch enables this functionality in the pcap driver.

> > At a high level, this works by creaing a ring of sufficient size to
> > store the packets in the pcap file passed to the application. When the
> > rx function for this mode is called, packets are dequeued from the
> > ring for use by the application and also enqueued back on to the ring
> > to be "received" again.
> >
> > A tx_drop mode is also added since transmitting to a tx_pcap file
> > isn't desirable at a high traffic rate.
> >
> > Jumbo frames are not supported in this mode. When filling the ring at
> > rx queue setup time, the presence of multi segment mbufs is checked for.
> > The PMD will exit on detection of these multi segment mbufs.
> >
> > Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
> 
> <...>
> 
> > @@ -106,6 +106,26 @@ Runtime Config Options
> >
> >     --vdev 'net_pcap0,iface=eth0,phy_mac=1'
> >
> > +- Use the RX PCAP file to infinitely receive packets
> > +
> > + In case ``rx_pcap=`` configuration is set, user may want to use the
> > + selected PCAP file for rudimental performance testing. This can be done
> with a ``devarg`` ``infinite_rx``, for example::
> > +
> > +   --vdev 'net_pcap0,rx_pcap=file_rx.pcap,infinite_rx=1,tx_drop=1'
> 
> 'tx_drop' seems remaining, it is no more valid.
> 

I missed this, it should be removed.

> <...>
> 
> > @@ -1337,6 +1551,9 @@ pmd_pcap_remove(struct rte_vdev_device
> *dev)
> >  			eth_dev->data->mac_addrs = NULL;
> >  	}
> >
> > +	if (internals->infinite_rx)
> > +		eth_dev_close(eth_dev);
> 
> Why 'internals->infinite_rx' check is required to call the 'eth_dev_close()'?
> Can we remove the check?

Good point, we don't need to check whether the device is in infinite_rx mode, since this is done in eth_dev_close anyway.