netdev - RE: [PATCH v1 net-next 10/11] net: ena: handle ena_calc_io_queue

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2baee949f83d47a896a2752e7667a99a@amazon.com>
Date: Tue, 30 Jan 2024 09:39:19 +0000
From: "Arinzon, David" <darinzon@...zon.com>
To: "Nelson, Shannon" <shannon.nelson@....com>, David Miller
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
	"netdev@...r.kernel.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>,
	"Belgazal, Netanel" <netanel@...zon.com>, "Saidi, Ali" <alisaidi@...zon.com>,
	"Herrenschmidt, Benjamin" <benh@...zon.com>, "Kiyanovski, Arthur"
	<akiyano@...zon.com>, "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay"
	<shayagr@...zon.com>, "Itzko, Shahar" <itzko@...zon.com>, "Abboud, Osama"
	<osamaabb@...zon.com>, "Ostrovsky, Evgeny" <evostrov@...zon.com>, "Tabachnik,
 Ofir" <ofirt@...zon.com>, "Koler, Nati" <nkolder@...zon.com>
Subject: RE: [PATCH v1 net-next 10/11] net: ena: handle ena_calc_io_queue_size()
 possible errors

> On 1/29/2024 12:55 AM, darinzon@...zon.com wrote:
> >
> > From: David Arinzon <darinzon@...zon.com>
> >
> > Fail queue size calculation when the device returns maximum TX/RX
> > queue sizes that are smaller than the allowed minimum.
> >
> > Signed-off-by: Osama Abboud <osamaabb@...zon.com>
> > Signed-off-by: David Arinzon <darinzon@...zon.com>
> > ---
> >   drivers/net/ethernet/amazon/ena/ena_netdev.c | 24
> +++++++++++++++++---
> >   1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > index 8d99904..ca56dff 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> > @@ -2899,8 +2899,8 @@ static const struct net_device_ops
> ena_netdev_ops = {
> >          .ndo_xdp_xmit           = ena_xdp_xmit,
> >   };
> >
> > -static void ena_calc_io_queue_size(struct ena_adapter *adapter,
> > -                                  struct ena_com_dev_get_features_ctx *get_feat_ctx)
> > +static int ena_calc_io_queue_size(struct ena_adapter *adapter,
> > +                                 struct ena_com_dev_get_features_ctx
> > +*get_feat_ctx)
> >   {
> >          struct ena_admin_feature_llq_desc *llq = &get_feat_ctx->llq;
> >          struct ena_com_dev *ena_dev = adapter->ena_dev; @@ -2959,6
> > +2959,18 @@ static void ena_calc_io_queue_size(struct ena_adapter
> *adapter,
> >          max_tx_queue_size =
> rounddown_pow_of_two(max_tx_queue_size);
> >          max_rx_queue_size =
> rounddown_pow_of_two(max_rx_queue_size);
> >
> > +       if (max_tx_queue_size < ENA_MIN_RING_SIZE) {
> > +               netdev_err(adapter->netdev, "Device max TX queue size: %d <
> minimum: %d\n",
> > +                          max_tx_queue_size, ENA_MIN_RING_SIZE);
> > +               return -EFAULT;
> > +       }
> > +
> > +       if (max_rx_queue_size < ENA_MIN_RING_SIZE) {
> > +               netdev_err(adapter->netdev, "Device max RX queue size: %d <
> minimum: %d\n",
> > +                          max_rx_queue_size, ENA_MIN_RING_SIZE);
> > +               return -EFAULT;
> 
> Maybe EINVAL for these two?
> 
> sln
> 

I agree with you, EINVAL represents this error much better than EFAULT.
Will change in the next patchset.

Thanks!
David

> > +       }
> > +
> >          /* When forcing large headers, we multiply the entry size by 2, and
> therefore divide
> >           * the queue size by 2, leaving the amount of memory used by the
> queues unchanged.
> >           */
> > @@ -2989,6 +3001,8 @@ static void ena_calc_io_queue_size(struct
> ena_adapter *adapter,
> >          adapter->max_rx_ring_size = max_rx_queue_size;
> >          adapter->requested_tx_ring_size = tx_queue_size;
> >          adapter->requested_rx_ring_size = rx_queue_size;
> > +
> > +       return 0;
> >   }
> >
> >   static int ena_device_validate_params(struct ena_adapter *adapter,
> > @@ -3190,11 +3204,15 @@ static int ena_device_init(struct ena_adapter
> *adapter, struct pci_dev *pdev,
> >                  goto err_admin_init;
> >          }
> >
> > -       ena_calc_io_queue_size(adapter, get_feat_ctx);
> > +       rc = ena_calc_io_queue_size(adapter, get_feat_ctx);
> > +       if (unlikely(rc))
> > +               goto err_admin_init;
> >
> >          return 0;
> >
> >   err_admin_init:
> > +       ena_com_abort_admin_commands(ena_dev);
> > +       ena_com_wait_for_abort_completion(ena_dev);
> >          ena_com_delete_host_info(ena_dev);
> >          ena_com_admin_destroy(ena_dev);
> >   err_mmio_read_less:
> > --
> > 2.40.1
> >
> >