Re: [dpdk-dev] [PATCH v3 19/22] net/ena: make ethdev references smp safe
From: Ferruh Yigit <hidden>
Date: 2021-05-07 16:49:32
On 5/6/2021 3:25 PM, Michal Krawczyk wrote:
From: Stanislaw Kardach <redacted> rte_pci_device and rte_eth_dev are process-local structures. Therefore ena_adapter::pdev and ena_adapter::rte_dev cannot be used universally. Switch this to extracting those structures via rte_eth_devices indexing and remove pdev since it's not used outside of init. Signed-off-by: Stanislaw Kardach <redacted> Reviewed-by: Michal Krawczyk <redacted> Reviewed-by: Igor Chauskin <redacted> Reviewed-by: Shay Agroskin <redacted>
<...>
quoted hunk ↗ jump to hunk
@@ -1634,7 +1633,7 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer, void *arg) { struct ena_adapter *adapter = arg; - struct rte_eth_dev *dev = adapter->rte_dev; + struct rte_eth_dev *dev = &rte_eth_devices[adapter->port_id];
I think better to try to avoid using 'rte_eth_devices' global variable as much as possible, although it may not be possible always, but for this case it seems it can be done. Why not just pass "struct rte_eth_dev" as 'arg', instead of "struct ena_adapter"?
quoted hunk ↗ jump to hunk
check_for_missing_keep_alive(adapter); check_for_admin_com_state(adapter);@@ -1819,11 +1818,10 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) memset(adapter, 0, sizeof(struct ena_adapter)); ena_dev = &adapter->ena_dev; - adapter->rte_eth_dev_data = eth_dev->data; - adapter->rte_dev = eth_dev; + adapter->edev_data = eth_dev->data; + adapter->port_id = eth_dev->data->port_id; pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); - adapter->pdev = pci_dev; PMD_INIT_LOG(INFO, "Initializing %x:%x:%x.%d", pci_dev->addr.domain,@@ -1843,7 +1841,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) } ena_dev->reg_bar = adapter->regs; - ena_dev->dmadev = adapter->pdev; + /* This is a dummy pointer for ena_com functions. */ + ena_dev->dmadev = adapter; adapter->id_number = adapters_found;@@ -1857,7 +1856,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) } /* device specific initialization routine */ - rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state); + rc = ena_device_init(ena_dev, pci_dev, &get_feat_ctx, &wd_state); if (rc) { PMD_INIT_LOG(CRIT, "Failed to init ENA device"); goto err;@@ -2716,7 +2715,7 @@ static int ena_xstats_get_names(struct rte_eth_dev *dev, struct rte_eth_xstat_name *xstats_names, unsigned int n) { - unsigned int xstats_count = ena_xstats_calc_num(dev); + unsigned int xstats_count = ena_xstats_calc_num(dev->data); unsigned int stat, i, count = 0; if (n < xstats_count || !xstats_names)@@ -2765,7 +2764,7 @@ static int ena_xstats_get(struct rte_eth_dev *dev, unsigned int n) { struct ena_adapter *adapter = dev->data->dev_private; - unsigned int xstats_count = ena_xstats_calc_num(dev); + unsigned int xstats_count = ena_xstats_calc_num(dev->data); unsigned int stat, i, count = 0; int stat_offset; void *stats_begin;@@ -2997,7 +2996,7 @@ static void ena_update_on_link_change(void *adapter_data, adapter = adapter_data; aenq_link_desc = (struct ena_admin_aenq_link_change_desc *)aenq_e; - eth_dev = adapter->rte_dev; + eth_dev = &rte_eth_devices[adapter->port_id];
Similarly, instead of passing "struct ena_adapter" as "void *adapter_data", it can be possible to pass "struct rte_eth_dev", as far as I can that chain has more layers but still possible. Meanhile, you can use "void *process_private;" field of "struct rte_eth_dev" to keep process private eth_dev data, if you want.