<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <div class="moz-cite-prefix">在 2023/2/16 0:09, Ferruh Yigit 写道: </div> <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap="">On 1/12/2023 2:44 AM, lihuisong (C) wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> 在 2023/1/11 20:51, Ferruh Yigit 写道: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">On 12/6/2022 9:26 AM, Huisong Li wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">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. </pre> </blockquote> <pre class="moz-quote-pre" wrap="">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. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> For example, rte_eth_dev_info_get is called in this event callback to get driver name. </pre> </blockquote> </blockquote> Hi Ferruh, Sorry for the delay. I missed this email. Thanks for your review. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> Why 'rte_eth_dev_info_get()' is called in the event called at first place? </pre> </blockquote> There is no limitation on rte_eth_dev_info_get() in event callback. The upper layer is entirely possible to use. After all, it is more convenient for app to use this pointer to get driver information in a probing callback. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> 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.</pre> </blockquote> Yes <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> - Stop forwarding implicitly in even callback in case event callback run while forwarding is on.</pre> </blockquote> But this is a modification for tesptmd to make it work well. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> All looks to me hack/complexity to make a specific case work, which is make secondary *testmp* application work with attached/detached device.</pre> </blockquote> It is not just a testpmd application case, but, I think, still exists in actual case. Because eal lib supports hotplugging device on primary and secondary process and the communication each other when attach or detach device. The reason why no one has ever put forward this question, I think it may be attributed to the fact that the scene is not or rarely tested. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> 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.</pre> </blockquote> I'm just raising this issue, and we can discuss how to deal with it together.😁 <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> 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?</pre> </blockquote> Actually, the probing event callback called in patch 4/5 is the result of the MP socket communication (please see hogplug_mp.c). <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> 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</pre> </blockquote> Please note the log "Now total ports is 0", which indicates the port number is updated if we detached device in this process. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> 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 </pre> </blockquote> But the port number in this process does not be updated after finishing the destroy event. That's the problem. <blockquote type="cite" cite="mid:5d06f478-6142-651b-d54b-0a3c75e9e7a0@amd.com"> <pre class="moz-quote-pre" wrap=""> testpmd> show port summary all Number of available ports: 0 Port MAC Address Name Driver Status Link testpmd> `` . </pre> </blockquote> </body> </html>