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.yi...@xilinx.com>
Sent: Monday, September 5, 2022 11:40 PM
To: Chaoyong He <chaoyong...@corigine.com>; dev@dpdk.org
Cc: oss-drivers <oss-driv...@corigine.com>; Niklas Soderlund
<niklas.soderl...@corigine.com>; Heinrich Kuhn
<heinrich.k...@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?


Reply via email to