Re: [PATCH V4 1/5] drivers/bus: restore driver assignment at front of probing

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

On 1/12/2023 2:44 AM, lihuisong (C) wrote:
> 
> 在 2023/1/11 20:51, Ferruh Yigit 写道:
>> On 12/6/2022 9:26 AM, Huisong Li wrote:
>>> The driver assignment was moved back at the end of the device probing
>>> because there is no something to use rte_driver during the phase of
>>> probing. See commit 391797f04208 ("drivers/bus: move driver assignment
>>> to end of probing")
>>>
>>> However, it is necessary for probing callback to reference rte_driver
>>> before probing. For example, probing callback may call some APIs which
>>> access the rte_pci_driver::driver by the device::driver pointer to get
>>> driver information. In this case, a segment fault will occur in probing
>>> callback if there is not this assignment.
>>>
>> Probing callback gets driver as parameter, so callback function can
>> access it via 'drv->driver', is there a specific usecase that
>> 'dev->device->driver' needs to be accessed explicitly?
>>
>> I assume this is related to coming patches that setting up device in
>> testpmd event callback, but can you please clarify exact need.
>
> For example, rte_eth_dev_info_get is called in this event callback to get
> driver name.
>

Why 'rte_eth_dev_info_get()' is called in the event called at first place?


This set updates multiple things to extend 'RTE_ETH_EVENT_NEW' event
callback function support, like:

- Assign device driver *before* probing completed, so that even callback
can run 'rte_eth_dev_info_get()'

- Add a new ethdev state so that port can be recognized as valid port in
the even callback.

- Stop forwarding implicitly in even callback in case event callback run
while forwarding is on.


All looks to me hack/complexity to make a specific case work, which is
make secondary *testmp* application work with attached/detached device.

And finally patch 4/5 adds port setup to testpmd event callback for this.


I understand the intention, but I disagree with bus and ethdev level
changes for this.



Event callback may not be only way to share port attach/detach
information between primary and secondary, there is a MP socket and
'rte_mp_handle' thread to handle communication between primary and
secondary process, this should be able to use carrying device
information, as far as I remember this is why it is introduced at first
place.

Did you consider using MP socket for your use case?



Following is a sample usage:

Primary:
started as:
sudo ./build/app/dpdk-testpmd --no-pci --proc-type=auto -l 0-1
--log-level=*:debug -- -i --num-procs=2 --proc-id=0

``
testpmd> show port summary all
Number of available ports: 0
Port MAC Address       Name         Driver         Status   Link

testpmd> port attach net_null0
Attaching a new port...
dpaa: rte_dpaa_bus_parse(): Parse device name (net_null0 )
fslmc: rte_fslmc_parse(): Parsing dev=(net_null0 )
fslmc: rte_fslmc_parse(): Unknown or unsupported device (net_null0 )
vdev_probe_all_drivers(): Search driver to probe device net_null0
rte_pmd_null_probe(): Initializing pmd_null for net_null0
rte_pmd_null_probe(): Configure pmd_null: packet size is 64, packet copy
is disabled
eth_dev_null_create(): Creating null ethdev on numa socket 0
EAL: request: eal_dev_mp_request
EAL: msg: bus_vdev_mp
vdev_action(): send vdev, net_null0
EAL: sendmsg: bus_vdev_mp
EAL: reply: bus_vdev_mp
EAL: msg: eal_dev_mp_request
Port 0 is attached. Now total ports is 1
Done

testpmd> show port summary all
Number of available ports: 1
Port MAC Address       Name         Driver         Status   Link
0    DE:E5:79:00:A9:68 net_null0    net_null       down     10 Gbps

testpmd> port detach 0
Port was not closed
Removing a device...
EAL: request: eal_dev_mp_request
EAL: msg: eal_dev_mp_request
eth_dev_close(): Closing null ethdev on NUMA socket 0
Port 0 is closed
Device is detached
Now total ports is 0
Done

testpmd> show port summary all
Number of available ports: 0
Port MAC Address       Name         Driver         Status   Link
testpmd>

``

Secondary:
started as:
sudo ./build/app/dpdk-testpmd --no-pci --proc-type=auto -l 2-3
--log-level=*:debug -- -i --num-procs=2 --proc-id=1

``
testpmd> show port summary all
Number of available ports: 0
Port MAC Address       Name         Driver         Status   Link

testpmd> EAL: msg: eal_dev_mp_request
dpaa: rte_dpaa_bus_parse(): Parse device name (net_null0 )
fslmc: rte_fslmc_parse(): Parsing dev=(net_null0 )
fslmc: rte_fslmc_parse(): Unknown or unsupported device (net_null0 )
EAL: request: bus_vdev_mp
EAL: msg: bus_vdev_mp
vdev_action(): receive vdev, net_null0
EAL: msg: bus_vdev_mp
vdev_scan(): Received 1 vdevs
vdev_probe_all_drivers(): Search driver to probe device net_null0
rte_pmd_null_probe(): Initializing pmd_null for net_null0
EAL: reply: eal_dev_mp_request

testpmd> show port summary all
Number of available ports: 1
Port MAC Address       Name         Driver         Status   Link
0    DE:E5:79:00:A9:68 net_null0    net_null       down     10 Gbps

testpmd> EAL: msg: eal_dev_mp_request
dpaa: rte_dpaa_bus_parse(): Parse device name (net_null0 )
fslmc: rte_fslmc_parse(): Parsing dev=(net_null0 )
fslmc: rte_fslmc_parse(): Unknown or unsupported device (net_null0 )
eth_dev_close(): Closing null ethdev on NUMA socket 4294967295
Port 0 is closed
EAL: reply: eal_dev_mp_request

testpmd> show port summary all
Number of available ports: 0
Port MAC Address       Name         Driver         Status   Link
testpmd>
``