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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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.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.
>