From: Ferruh Yigit <ferruh.yigit@xilinx.com>
To: Chaoyong He <chaoyong.he@corigine.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: oss-drivers <oss-drivers@corigine.com>,
Niklas Soderlund <niklas.soderlund@corigine.com>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v7 04/12] net/nfp: add initial flower firmware support
Date: Wed, 7 Sep 2022 14:24:05 +0100 [thread overview]
Message-ID: <ac24f7be-6ccf-bf81-ab07-f8284099ceb6@xilinx.com> (raw)
In-Reply-To: <SJ0PR13MB5545DFBABD56623DC13C1BB89E419@SJ0PR13MB5545.namprd13.prod.outlook.com>
On 9/7/2022 9:22 AM, Chaoyong He wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> Sent: Monday, September 5, 2022 11:40 PM
>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>> Cc: oss-drivers <oss-drivers@corigine.com>; Niklas Soderlund
>> <niklas.soderlund@corigine.com>; Heinrich Kuhn
>> <heinrich.kuhn@corigine.com>
>> Subject: Re: [PATCH v7 04/12] net/nfp: add initial flower firmware support
>>
>> On 8/12/2022 11:22 AM, Chaoyong He wrote:
>>> Adds the basic probing infrastructure to support the flower firmware.
>>> This firmware is geared towards offloading OVS and can generally be
>>> found in /lib/firmware/netronome/flower. It is also used by the NFP
>>> kernel driver when OVS offload with TC is desired.
>>
>> '/lib/firmware/netronome/flower' FW is loaded (automatically ?) by kernel
>> driver, right?
>> Is there anything related to the DPDK with the name/path of the FW?
>>
>>
>> And I wonder if these kind of information should be part of driver
>> documentation?
>>
>> <...>
>>
>>> @@ -965,6 +968,16 @@
>>> goto hwqueues_cleanup;
>>> }
>>> break;
>>> + case NFP_APP_FLOWER_NIC:
>>> + PMD_INIT_LOG(INFO, "Initializing Flower");
>>> + pci_dev->device.driver = &pci_drv->driver;
>>
>> Why this assignment is required? Driver shouldn't need to update this
>> bus related data struct.
>
> Following the framework's logic:
> function 'rte_pci_probe_one_driver()' in file drivers/bus/pci/pci_common.c
>
> ```
> ...
> ret = dr->probe(dr, dev); // here call our nfp_pf_pci_probe()
> if (ret) {
> ...
> } else {
> dev->device.driver = &dr->driver;
> }
>
> ```
>
> Here the framework will do the assignment.
>
> But in our logic, if we won't add this assignment, we will run into a coredump:
> ```
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 rte_eth_dev_info_get (port_id=0, dev_info=0x7ffe9c3d3f70) at ../lib/ethdev/rte_ethdev.c:3138
> 3138 dev_info->driver_name = dev->device->driver->name;
> (gdb) bt
> #0 rte_eth_dev_info_get (port_id=0, dev_info=0x7ffe9c3d3f70) at ../lib/ethdev/rte_ethdev.c:3138
> #1 0x00007fa95128565f in rte_eth_dev_configure (port_id=0, nb_rx_q=1, nb_tx_q=1,
> dev_conf=0x7fa94ce338e0 <port_conf>) at ../lib/ethdev/rte_ethdev.c:1110
> #2 0x00007fa94cdf74b7 in nfp_flower_init_ctrl_vnic (hw=0x2001e8530)
> at ../drivers/net/nfp/flower/nfp_flower.c:1035
> #3 0x00007fa94cdf7b32 in nfp_init_app_fw_flower (pf_dev=0x2001e8c80)
> at ../drivers/net/nfp/flower/nfp_flower.c:1219
> #4 0x00007fa94ce30e33 in nfp_pf_init (pci_dev=0xcd22c0, pci_drv=0x7fa94ce44460 <rte_nfp_net_pf_pmd>)
> at ../drivers/net/nfp/nfp_ethdev.c:976
> #5 0x00007fa94ce31429 in nfp_pf_pci_probe (pci_drv=0x7fa94ce44460 <rte_nfp_net_pf_pmd>, dev=0xcd22c0)
> at ../drivers/net/nfp/nfp_ethdev.c:1152
> #6 0x00007fa94e8f8f34 in rte_pci_probe_one_driver (dr=0x7fa94ce44460 <rte_nfp_net_pf_pmd>, dev=0xcd22c0)
> at ../drivers/bus/pci/pci_common.c:269
> #7 0x00007fa94e8f91c0 in pci_probe_all_drivers (dev=0xcd22c0) at ../drivers/bus/pci/pci_common.c:353
> #8 0x00007fa94e8f9244 in pci_probe () at ../drivers/bus/pci/pci_common.c:380
> #9 0x00007fa951163c0e in rte_bus_probe () at ../lib/eal/common/eal_common_bus.c:72
> #10 0x00007fa951193903 in rte_eal_init (argc=10, argv=0xcc5150) at ../lib/eal/linux/eal.c:1279
> #11 0x0000000000601531 in dpdk_init__ (ovs_other_config=0xccefd0) at lib/dpdk.c:466
> #12 0x0000000000601913 in dpdk_init (ovs_other_config=0xccefd0) at lib/dpdk.c:545
> #13 0x00000000004129ed in bridge_run () at vswitchd/bridge.c:3252
> #14 0x0000000000418364 in main (argc=11, argv=0x7ffe9c3d49c8) at vswitchd/ovs-vswitchd.c:129
>
> ```
>
> Our nfp card use `control message` to exchange message between PMD and firmware when we use flower firmware.
> The control message is in the form of pkt and we use a `ctrl vNIC` ehtdev as the agent to send and receive these pkts.
> e.g., if we want to create representor port, the PMD must send the corresponding control message to firmware.
> To be able to send and receive pkt, we must do some configure steps to this ethdev firstly.
> But the framework has not do the assignment step at this moment.
> And this is where the problem comes from.
>
I am not sure about PMD calling 'rte_eth_dev_configure()', this API is
intended for application.
And even worse, not sure if it is allowed to call
'rte_eth_dev_configure()' before driver probe completed successfully.
It looks to me assigning 'pci_dev->device.driver' within the PMD is hack
to allow it run 'rte_eth_dev_configure()' before probe() completed.
Do you really need to call ''rte_eth_dev_configure()' to prepare ethdev
for the 'control message' exchange? What exactly needs to be configured
for this?
> Logically, if our probe process is successful, assignment again won't import any problem.
> If our probe process failed, our logic will undo the assignment and the framework will go as its original logic.
> So maybe we can keep the logic now?
> Or there exist another way more standard?
>
next prev parent reply other threads:[~2022-09-07 13:24 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-12 10:22 [PATCH v7 00/12] preparation for the rte_flow offload of nfp PMD Chaoyong He
2022-08-12 10:22 ` [PATCH v7 01/12] net/nfp: move app specific attributes to own struct Chaoyong He
2022-09-05 15:38 ` Ferruh Yigit
2022-09-06 9:20 ` Chaoyong He
2022-09-06 9:35 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 02/12] net/nfp: simplify initialization and remove dead code Chaoyong He
2022-08-12 10:22 ` [PATCH v7 03/12] net/nfp: move app specific init logic to own function Chaoyong He
2022-09-05 15:38 ` Ferruh Yigit
2022-09-06 8:29 ` Chaoyong He
2022-09-06 9:47 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 04/12] net/nfp: add initial flower firmware support Chaoyong He
2022-09-05 15:39 ` Ferruh Yigit
2022-09-07 8:22 ` Chaoyong He
2022-09-07 13:24 ` Ferruh Yigit [this message]
2022-08-12 10:22 ` [PATCH v7 05/12] net/nfp: add flower PF setup and mempool init logic Chaoyong He
2022-09-05 15:42 ` Ferruh Yigit
2022-09-06 8:45 ` Chaoyong He
2022-09-06 10:18 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 06/12] net/nfp: add flower PF related routines Chaoyong He
2022-08-12 10:22 ` [PATCH v7 07/12] net/nfp: add flower ctrl VNIC related logics Chaoyong He
2022-08-12 10:22 ` [PATCH v7 08/12] net/nfp: move common rxtx function for flower use Chaoyong He
2022-08-12 10:22 ` [PATCH v7 09/12] net/nfp: add flower ctrl VNIC rxtx logic Chaoyong He
2022-09-05 15:42 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 10/12] net/nfp: add flower representor framework Chaoyong He
2022-08-12 10:22 ` [PATCH v7 11/12] net/nfp: move rxtx function to header file Chaoyong He
2022-08-12 10:22 ` [PATCH v7 12/12] net/nfp: add flower PF rxtx logic Chaoyong He
2022-09-01 22:10 ` [PATCH v7 00/12] preparation for the rte_flow offload of nfp PMD Niklas Söderlund
2022-09-05 15:40 ` Ferruh Yigit
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=ac24f7be-6ccf-bf81-ab07-f8284099ceb6@xilinx.com \
--to=ferruh.yigit@xilinx.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=niklas.soderlund@corigine.com \
--cc=oss-drivers@corigine.com \
/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).