From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "lihuisong (C)" <lihuisong@huawei.com>,
dev@dpdk.org, andrew.rybchenko@oktetlabs.ru
Cc: thomas@monjalon.net, liudongdong3@huawei.com,
huangdaode@huawei.com, fengchengwen@huawei.com
Subject: Re: [PATCH V4 1/5] drivers/bus: restore driver assignment at front of probing
Date: Tue, 6 Jun 2023 17:12:33 +0100 [thread overview]
Message-ID: <bfca44b9-1bf4-67c6-245a-0aca768b65b1@amd.com> (raw)
In-Reply-To: <80f82803-0ea0-adf6-4dd6-ccafc6643fbd@huawei.com>
On 2/28/2023 2:21 AM, lihuisong (C) wrote:
>
> 在 2023/2/16 0:09, Ferruh Yigit 写道:
>> 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.
>>>
> Hi Ferruh,
>
> Sorry for the delay. I missed this email.
> Thanks for your review.
>> Why 'rte_eth_dev_info_get()' is called in the event called at first place?
> 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.
>
I think there is a logical limit in calling 'rte_eth_dev_info_get()' in
NEW event callback, we may document this if it is missing.
The NEW event callback is to let application do proper tasks before port
addition finalized in the ethdev layer, but you are trying to call
'rte_eth_dev_info_get()' in the callback when it is not ready yet.
>> 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.
> Yes
>> - Stop forwarding implicitly in even callback in case event callback run
>> while forwarding is on.
> But this is a modification for tesptmd to make it work well.
>> All looks to me hack/complexity to make a specific case work, which is
>> make secondary *testmp* application work with attached/detached device.
> 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.
>> 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.
> I'm just raising this issue, and we can discuss how to deal with it
> together.😁
>>
>> 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?
> Actually, the probing event callback called in patch 4/5 is the result
> of the MP socket communication (please see hogplug_mp.c).
>>
>> 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
> Please note the log *"Now total ports is 0"*,
> which indicates the port number is updated if we detached device in this
> process.
>
Yes, as expected.
>> 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
> But the port number in this process does not be updated after finishing
> the destroy event.
> That's the problem.
>
Please see following command output,
it says "Number of available ports: 0",
so that shows port number is updated after destroy event, isn't it?
>> testpmd> show port summary all
>> Number of available ports: 0
>> Port MAC Address Name Driver Status Link
>> testpmd>
>> ``
>>
>>
>> .
next prev parent reply other threads:[~2023-06-06 16:12 UTC|newest]
Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20220825024425.10534-1-lihuisong@huawei.com>
2022-09-15 12:45 ` [PATCH V2 0/6] [PATCH 0/6] app/testpmd: support attach and detach port for MP Huisong Li
2022-09-15 12:45 ` [PATCH V2 1/6] bus/pci: fix a segfault when call callback Huisong Li
2022-10-10 19:49 ` Thomas Monjalon
2022-10-25 3:25 ` lihuisong (C)
2022-09-15 12:45 ` [PATCH V2 2/6] bus/vdev: " Huisong Li
2022-09-15 12:45 ` [PATCH V2 3/6] ethdev: fix push new event Huisong Li
2022-09-27 10:49 ` Thomas Monjalon
2022-10-08 4:09 ` lihuisong (C)
2022-10-25 3:26 ` lihuisong (C)
2022-09-15 12:45 ` [PATCH V2 4/6] app/testpmd: check the validity of the port Huisong Li
2022-09-22 5:07 ` Singh, Aman Deep
2022-09-15 12:45 ` [PATCH V2 5/6] app/testpmd: support attach and detach port for MP Huisong Li
2022-09-15 12:45 ` [PATCH V2 6/6] app/testpmd: stop packet forwarding in new and destroy event Huisong Li
2022-12-06 6:45 ` [PATCH V3 0/5] app/testpmd: support mulitple process attach and detach port Huisong Li
2022-12-06 6:45 ` [PATCH V3 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2022-12-06 6:45 ` [PATCH V3 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2022-12-06 6:45 ` [PATCH V3 3/5] app/testpmd: check the validity of the port Huisong Li
2022-12-06 6:45 ` [PATCH V3 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2022-12-06 6:45 ` [PATCH V3 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2022-12-06 9:26 ` [PATCH V4 0/5] app/testpmd: support mulitple process attach and detach port Huisong Li
2022-12-06 9:26 ` [PATCH V4 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-01-11 12:51 ` Ferruh Yigit
2023-01-12 2:44 ` lihuisong (C)
2023-02-15 16:09 ` Ferruh Yigit
2023-02-28 2:21 ` lihuisong (C)
2023-06-06 16:12 ` Ferruh Yigit [this message]
2023-06-07 10:11 ` lihuisong (C)
2023-06-15 2:21 ` lihuisong (C)
2022-12-06 9:26 ` [PATCH V4 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-01-11 12:51 ` Ferruh Yigit
2023-01-12 4:12 ` lihuisong (C)
2022-12-06 9:26 ` [PATCH V4 3/5] app/testpmd: check the validity of the port Huisong Li
2022-12-06 9:26 ` [PATCH V4 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-01-11 12:51 ` Ferruh Yigit
2023-01-12 4:14 ` lihuisong (C)
2022-12-06 9:26 ` [PATCH V4 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-01-11 12:52 ` Ferruh Yigit
2023-01-12 4:16 ` lihuisong (C)
2023-01-09 12:38 ` [PATCH V4 0/5] app/testpmd: support mulitple process attach and detach port lihuisong (C)
2023-01-10 16:51 ` Ferruh Yigit
2023-01-11 0:53 ` lihuisong (C)
2023-01-11 10:27 ` Ferruh Yigit
2023-01-11 10:46 ` Ferruh Yigit
2023-01-12 2:26 ` lihuisong (C)
2023-01-18 14:12 ` Thomas Monjalon
2023-01-19 10:31 ` lihuisong (C)
2023-01-19 14:35 ` Thomas Monjalon
2023-01-28 1:39 ` lihuisong (C)
2023-01-31 3:33 ` [PATCH V5 0/5] app/testpmd: support multiple " Huisong Li
2023-01-31 3:33 ` [PATCH V5 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-01-31 3:33 ` [PATCH V5 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-05-22 11:04 ` fengchengwen
2023-05-27 1:58 ` lihuisong (C)
2023-01-31 3:33 ` [PATCH V5 3/5] app/testpmd: check the validity of the port Huisong Li
2023-01-31 3:33 ` [PATCH V5 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-01-31 3:33 ` [PATCH V5 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-05-16 11:27 ` [PATCH V5 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2023-05-23 0:46 ` fengchengwen
2023-05-27 2:11 ` [PATCH V6 " Huisong Li
2023-05-27 2:11 ` [PATCH V6 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-05-27 2:11 ` [PATCH V6 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-05-27 2:11 ` [PATCH V6 3/5] app/testpmd: check the validity of the port Huisong Li
2023-05-27 2:11 ` [PATCH V6 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-05-27 2:11 ` [PATCH V6 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-06-06 16:26 ` [PATCH V6 0/5] app/testpmd: support multiple process attach and detach port Ferruh Yigit
2023-06-07 10:14 ` lihuisong (C)
2023-07-14 7:21 ` lihuisong (C)
2023-08-02 3:15 ` [PATCH RESEND v6 " Huisong Li
2023-08-02 3:15 ` [PATCH RESEND v6 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-08-02 3:15 ` [PATCH RESEND v6 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-08-02 3:15 ` [PATCH RESEND v6 3/5] app/testpmd: check the validity of the port Huisong Li
2023-08-02 3:15 ` [PATCH RESEND v6 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-08-02 3:16 ` [PATCH RESEND v6 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-10-09 10:34 ` [PATCH RESEND v6 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2023-10-30 12:17 ` lihuisong (C)
2023-12-08 2:25 ` lihuisong (C)
2024-01-30 6:36 ` [PATCH v7 " Huisong Li
2024-01-30 6:36 ` [PATCH v7 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2024-01-30 6:36 ` [PATCH v7 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2024-01-30 6:36 ` [PATCH v7 3/5] app/testpmd: check the validity of the port Huisong Li
2024-01-30 6:36 ` [PATCH v7 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2024-01-30 6:36 ` [PATCH v7 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2024-03-08 10:38 ` [PATCH v7 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2024-04-23 11:17 ` lihuisong (C)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bfca44b9-1bf4-67c6-245a-0aca768b65b1@amd.com \
--to=ferruh.yigit@amd.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=huangdaode@huawei.com \
--cc=lihuisong@huawei.com \
--cc=liudongdong3@huawei.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).