Re: [dpdk-dev] [PATCH v2] af_packet: make the device detachable

["Wojciech Żmuda" <woz@semihalf.com>]

Hi Bruce,

>The use of "deinitialization" sounds awkward

Thank you for your interest. I called it deinitialization in
opposition to an initialization of a device. As I'm not a native
English speaker, I trust your opinion and I'll try to rephrase this.

Hi Bernard,

>What parameters do you use with --vdev option in testpmd

I launch testpmd like this:
# ./testpmd -c f -n 4 --vdev 'eth_af_packet0,iface=eth2' -- -i
--port-topology=chained

Then I can see that af_packet starts:

PMD: Initializing pmd_af_packet for eth_af_packet0
PMD: eth_af_packet0: AF_PACKET MMAP parameters:
PMD: eth_af_packet0:    block size 4096
PMD: eth_af_packet0:    block count 256
PMD: eth_af_packet0:    frame size 2048
PMD: eth_af_packet0:    frame count 512
PMD: eth_af_packet0: creating AF_PACKET-backed ethdev on numa socket

When I get to the CLI, I do as follows:

Checking link statuses...
Port 0 Link Up - speed 10000 Mbps - full-duplex
Done
testpmd> port stop 0
Stopping ports...
Checking link statuses...
Port 0 Link Down
Done
testpmd> port close 0
Closing ports...
Done
testpmd> port detach 0
Detaching a port...
PMD: Closing AF_PACKET ethdev on numa socket 0
Port 'eth_af_packet0' is detached. Now total ports is 0
Done
testpmd>

In opposition, without the patch detach is impossible:

testpmd> port detach 0
Detaching a port...
EAL: Driver, cannot detach the device
testpmd>


Bernard, Bruce, I have a question, if I may. Do you know what is the
reason that rte_pmd_af_packet_devinit() is the only non-static device
initialization function among all the dpdk drivers? There's even a
comment in the rte_eth_af_packet.h:

/**
 * For use by the EAL only. Called as part of EAL init to set up any dummy NICs
 * configured on command line.
 */
int rte_pmd_af_packet_devinit(const char *name, const char *params);

Despite the comment above, I cannot see this function being called
directly anywhere. Is there any reason it is implemented this way? Or
should I change the definition to static, as it should be called via
proper API functions?

Thank you for your time,
Wojtek

2016-02-09 17:37 GMT+01:00 Bruce Richardson <bruce.richardson@intel.com>:
> On Tue, Feb 09, 2016 at 05:09:06PM +0100, Wojciech Zmuda wrote:
>> Implement rte_pmd_af_packet_devuninit() exposed through struct
>> rte_driver.uninit() and set dev_flags to RTE_ETH_DEV_DETACHABLE,
>> to allow af_packet device deinitialization with API function
>> rte_eth_dev_detach(). This fixes memory leak by freeing memory
>> allocated during initialization.
>> During device initialization copy device name to ethdev->data to make
>> it compatible with rte_eth_dev_allocated().
>>
>> Signed-off-by: Wojciech Zmuda <woz@semihalf.com>
>> ---
>> v2:
>> * Fixed typo and a comment.
>> * Added feature to the 2.3 release notes.
>> * Free memory allocated for rx and tx queues.
>>
>>  doc/guides/rel_notes/release_2_3.rst      |  4 ++++
>>  drivers/net/af_packet/rte_eth_af_packet.c | 37 ++++++++++++++++++++++++++++++-
>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/guides/rel_notes/release_2_3.rst b/doc/guides/rel_notes/release_2_3.rst
>> index 7945694..4694646 100644
>> --- a/doc/guides/rel_notes/release_2_3.rst
>> +++ b/doc/guides/rel_notes/release_2_3.rst
>> @@ -39,6 +39,10 @@ This section should contain new features added in this release. Sample format:
>>
>>    Enabled virtio 1.0 support for virtio pmd driver.
>>
>> +* **Added af_packet driver deinitialization function.**
>> +
>> +  Implemented rte_pmd_af_packet_devuninit() exposed through struct
>> +  rte_driver.uninit() to allow af_packet device deinitialization with API function.
>>
>
> The use of "deinitialization" sounds awkward, and the overall text maybe could be
> made less technical. Maybe talk about "allowing dynamic removal" of af_packet
> devices [or even hotplug of them]?
>
> /Bruce