lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Dec 2020 21:56:01 +0100 From: Heiner Kallweit <hkallweit1@...il.com> To: "Kiyanovski, Arthur" <akiyano@...zon.com>, "kuba@...nel.org" <kuba@...nel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net> Cc: "Woodhouse, David" <dwmw@...zon.co.uk>, "Machulsky, Zorik" <zorik@...zon.com>, "Matushevsky, Alexander" <matua@...zon.com>, "Bshara, Saeed" <saeedb@...zon.com>, "Wilson, Matt" <msw@...zon.com>, "Liguori, Anthony" <aliguori@...zon.com>, "Bshara, Nafea" <nafea@...zon.com>, "Tzalik, Guy" <gtzalik@...zon.com>, "Belgazal, Netanel" <netanel@...zon.com>, "Saidi, Ali" <alisaidi@...zon.com>, "Herrenschmidt, Benjamin" <benh@...zon.com>, "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay" <shayagr@...zon.com>, "Jubran, Samih" <sameehj@...zon.com> Subject: Re: [PATCH V3 net-next 1/9] net: ena: use constant value for net_device allocation Am 03.12.2020 um 15:38 schrieb Kiyanovski, Arthur: > > >> -----Original Message----- >> From: Heiner Kallweit <hkallweit1@...il.com> >> Sent: Wednesday, December 2, 2020 11:55 PM >> To: Kiyanovski, Arthur <akiyano@...zon.com>; kuba@...nel.org; >> netdev@...r.kernel.org >> Cc: Woodhouse, David <dwmw@...zon.co.uk>; Machulsky, Zorik >> <zorik@...zon.com>; Matushevsky, Alexander <matua@...zon.com>; >> Bshara, Saeed <saeedb@...zon.com>; Wilson, Matt <msw@...zon.com>; >> Liguori, Anthony <aliguori@...zon.com>; Bshara, Nafea >> <nafea@...zon.com>; Tzalik, Guy <gtzalik@...zon.com>; Belgazal, >> Netanel <netanel@...zon.com>; Saidi, Ali <alisaidi@...zon.com>; >> Herrenschmidt, Benjamin <benh@...zon.com>; Dagan, Noam >> <ndagan@...zon.com>; Agroskin, Shay <shayagr@...zon.com>; Jubran, >> Samih <sameehj@...zon.com> >> Subject: RE: [EXTERNAL] [PATCH V3 net-next 1/9] net: ena: use constant >> value for net_device allocation >> >> CAUTION: This email originated from outside of the organization. Do not click >> links or open attachments unless you can confirm the sender and know the >> content is safe. >> >> >> >> Am 02.12.2020 um 21:03 schrieb akiyano@...zon.com: >>> From: Arthur Kiyanovski <akiyano@...zon.com> >>> >>> The patch changes the maximum number of RX/TX queues it advertises to >>> the kernel (via alloc_etherdev_mq()) from a value received from the >>> device to a constant value which is the minimum between 128 and the >>> number of CPUs in the system. >>> >>> By allocating the net_device struct with a constant number of queues, >>> the driver is able to allocate it at a much earlier stage, before >>> calling any ena_com functions. This would allow to make all log prints >>> in ena_com to use netdev_* log functions instead or current pr_* ones. >>> >> >> Did you test this? Usually using netdev_* before the net_device is registered >> results in quite ugly messages. Therefore there's a number of patches doing >> the opposite, replacing netdev_* with dev_* before register_netdev(). See >> e.g. >> 22148df0d0bd ("r8169: don't use netif_info et al before net_device has been >> registered") > > Thanks for your comment. > Yes we did test it. > Please see the discussion which led to this patch in a previous thread here: > https://www.mail-archive.com/netdev@vger.kernel.org/msg353590.html > Ah, I see. After reading the mail thread your motivation is clear. You accept ugly messages when ena_com functions are called from probe() for the sake of better messages when the same ena_com functions are called later from other parts of the driver. Maybe an explanation of this tradeoff would have been good in the commit message (or a link to the mail thread). >>> Signed-off-by: Shay Agroskin <shayagr@...zon.com> >>> Signed-off-by: Arthur Kiyanovski <akiyano@...zon.com> >>> --- >>> drivers/net/ethernet/amazon/ena/ena_netdev.c | 46 >>> ++++++++++---------- >>> 1 file changed, 23 insertions(+), 23 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> index df1884d57d1a..985dea1870b5 100644 >>> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> @@ -29,6 +29,8 @@ MODULE_LICENSE("GPL"); >>> /* Time in jiffies before concluding the transmitter is hung. */ >>> #define TX_TIMEOUT (5 * HZ) >>> >>> +#define ENA_MAX_RINGS min_t(unsigned int, >> ENA_MAX_NUM_IO_QUEUES, >>> +num_possible_cpus()) >>> + >>> #define ENA_NAPI_BUDGET 64 >>> >>> #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_PROBE >> | >>> NETIF_MSG_IFUP | \ @@ -4176,18 +4178,34 @@ static int >> ena_probe(struct >>> pci_dev *pdev, const struct pci_device_id *ent) >>> >>> ena_dev->dmadev = &pdev->dev; >>> >>> + netdev = alloc_etherdev_mq(sizeof(struct ena_adapter), >> ENA_MAX_RINGS); >>> + if (!netdev) { >>> + dev_err(&pdev->dev, "alloc_etherdev_mq failed\n"); >>> + rc = -ENOMEM; >>> + goto err_free_region; >>> + } >>> + >>> + SET_NETDEV_DEV(netdev, &pdev->dev); >>> + adapter = netdev_priv(netdev); >>> + adapter->ena_dev = ena_dev; >>> + adapter->netdev = netdev; >>> + adapter->pdev = pdev; >>> + adapter->msg_enable = netif_msg_init(debug, >> DEFAULT_MSG_ENABLE); >>> + >>> + pci_set_drvdata(pdev, adapter); >>> + >>> rc = ena_device_init(ena_dev, pdev, &get_feat_ctx, &wd_state); >>> if (rc) { >>> dev_err(&pdev->dev, "ENA device init failed\n"); >>> if (rc == -ETIME) >>> rc = -EPROBE_DEFER; >>> - goto err_free_region; >>> + goto err_netdev_destroy; >>> } >>> >>> rc = ena_map_llq_mem_bar(pdev, ena_dev, bars); >>> if (rc) { >>> dev_err(&pdev->dev, "ENA llq bar mapping failed\n"); >>> - goto err_free_ena_dev; >>> + goto err_device_destroy; >>> } >>> >>> calc_queue_ctx.ena_dev = ena_dev; @@ -4207,26 +4225,8 @@ static >>> int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>> goto err_device_destroy; >>> } >>> >>> - /* dev zeroed in init_etherdev */ >>> - netdev = alloc_etherdev_mq(sizeof(struct ena_adapter), >> max_num_io_queues); >>> - if (!netdev) { >>> - dev_err(&pdev->dev, "alloc_etherdev_mq failed\n"); >>> - rc = -ENOMEM; >>> - goto err_device_destroy; >>> - } >>> - >>> - SET_NETDEV_DEV(netdev, &pdev->dev); >>> - >>> - adapter = netdev_priv(netdev); >>> - pci_set_drvdata(pdev, adapter); >>> - >>> - adapter->ena_dev = ena_dev; >>> - adapter->netdev = netdev; >>> - adapter->pdev = pdev; >>> - >>> ena_set_conf_feat_params(adapter, &get_feat_ctx); >>> >>> - adapter->msg_enable = netif_msg_init(debug, >> DEFAULT_MSG_ENABLE); >>> adapter->reset_reason = ENA_REGS_RESET_NORMAL; >>> >>> adapter->requested_tx_ring_size = calc_queue_ctx.tx_queue_size; >>> @@ -4257,7 +4257,7 @@ static int ena_probe(struct pci_dev *pdev, const >> struct pci_device_id *ent) >>> if (rc) { >>> dev_err(&pdev->dev, >>> "Failed to query interrupt moderation feature\n"); >>> - goto err_netdev_destroy; >>> + goto err_device_destroy; >>> } >>> ena_init_io_rings(adapter, >>> 0, >>> @@ -4335,11 +4335,11 @@ static int ena_probe(struct pci_dev *pdev, >> const struct pci_device_id *ent) >>> ena_disable_msix(adapter); >>> err_worker_destroy: >>> del_timer(&adapter->timer_service); >>> -err_netdev_destroy: >>> - free_netdev(netdev); >>> err_device_destroy: >>> ena_com_delete_host_info(ena_dev); >>> ena_com_admin_destroy(ena_dev); >>> +err_netdev_destroy: >>> + free_netdev(netdev); >>> err_free_region: >>> ena_release_bars(ena_dev, pdev); >>> err_free_ena_dev: >>> >
Powered by blists - more mailing lists