| Message ID | 1606939410-26718-2-git-send-email-akiyano@amazon.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | XDP Redirect implementation for ENA driver | expand |
Am 02.12.2020 um 21:03 schrieb akiyano@amazon.com: > From: Arthur Kiyanovski <akiyano@amazon.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") > Signed-off-by: Shay Agroskin <shayagr@amazon.com> > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.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: >
> -----Original Message----- > From: Heiner Kallweit <hkallweit1@gmail.com> > Sent: Wednesday, December 2, 2020 11:55 PM > To: Kiyanovski, Arthur <akiyano@amazon.com>; kuba@kernel.org; > netdev@vger.kernel.org > Cc: Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik > <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>; > Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>; > Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea > <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, > Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; > Herrenschmidt, Benjamin <benh@amazon.com>; Dagan, Noam > <ndagan@amazon.com>; Agroskin, Shay <shayagr@amazon.com>; Jubran, > Samih <sameehj@amazon.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@amazon.com: > > From: Arthur Kiyanovski <akiyano@amazon.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 > > Signed-off-by: Shay Agroskin <shayagr@amazon.com> > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.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: > >
Am 03.12.2020 um 15:38 schrieb Kiyanovski, Arthur: > > >> -----Original Message----- >> From: Heiner Kallweit <hkallweit1@gmail.com> >> Sent: Wednesday, December 2, 2020 11:55 PM >> To: Kiyanovski, Arthur <akiyano@amazon.com>; kuba@kernel.org; >> netdev@vger.kernel.org >> Cc: Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik >> <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>; >> Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>; >> Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea >> <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, >> Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; >> Herrenschmidt, Benjamin <benh@amazon.com>; Dagan, Noam >> <ndagan@amazon.com>; Agroskin, Shay <shayagr@amazon.com>; Jubran, >> Samih <sameehj@amazon.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@amazon.com: >>> From: Arthur Kiyanovski <akiyano@amazon.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@amazon.com> >>> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.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: >>> >
> -----Original Message----- > From: Heiner Kallweit <hkallweit1@gmail.com> > Sent: Thursday, December 3, 2020 10:56 PM > To: Kiyanovski, Arthur <akiyano@amazon.com>; kuba@kernel.org; > netdev@vger.kernel.org; davem@davemloft.net > Cc: Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik > <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>; > Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt <msw@amazon.com>; > Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea > <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, > Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; > Herrenschmidt, Benjamin <benh@amazon.com>; Dagan, Noam > <ndagan@amazon.com>; Agroskin, Shay <shayagr@amazon.com>; Jubran, > Samih <sameehj@amazon.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 03.12.2020 um 15:38 schrieb Kiyanovski, Arthur: > > > > > >> -----Original Message----- > >> From: Heiner Kallweit <hkallweit1@gmail.com> > >> Sent: Wednesday, December 2, 2020 11:55 PM > >> To: Kiyanovski, Arthur <akiyano@amazon.com>; kuba@kernel.org; > >> netdev@vger.kernel.org > >> Cc: Woodhouse, David <dwmw@amazon.co.uk>; Machulsky, Zorik > >> <zorik@amazon.com>; Matushevsky, Alexander <matua@amazon.com>; > >> Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt > <msw@amazon.com>; > >> Liguori, Anthony <aliguori@amazon.com>; Bshara, Nafea > >> <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal, > >> Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>; > >> Herrenschmidt, Benjamin <benh@amazon.com>; Dagan, Noam > >> <ndagan@amazon.com>; Agroskin, Shay <shayagr@amazon.com>; > Jubran, > >> Samih <sameehj@amazon.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@amazon.com: > >>> From: Arthur Kiyanovski <akiyano@amazon.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). Good idea. Added this explanation to the next version of this patchset. Thanks! > >>> Signed-off-by: Shay Agroskin <shayagr@amazon.com> > >>> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.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: > >>> > >
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: