On 6/28/2019 1:32 PM, Ferriter, Cian wrote: > 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.
+1, I will update them while merging. > > 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.ferri...@intel.com>; Richardson, Bruce >> <bruce.richard...@intel.com>; Mcnamara, John >> <john.mcnam...@intel.com>; Kovacevic, Marko >> <marko.kovace...@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.ferri...@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. >
