diff mbox series

[v1,net-next,10/11] net: ena: handle ena_calc_io_queue_size() possible errors

Message ID 20240129085531.15608-11-darinzon@amazon.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ENA driver changes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1064 this patch: 1064
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1081 this patch: 1081
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1081 this patch: 1081
netdev/checkpatch warning WARNING: line length of 84 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest pending net-next-2024-01-30--09-00

Commit Message

Arinzon, David Jan. 29, 2024, 8:55 a.m. UTC 
From: David Arinzon <darinzon@amazon.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@amazon.com>
Signed-off-by: David Arinzon <darinzon@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 24 +++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

Comments

Nelson, Shannon Jan. 30, 2024, 1:16 a.m. UTC | #1 
On 1/29/2024 12:55 AM, darinzon@amazon.com wrote:
> 
> From: David Arinzon <darinzon@amazon.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@amazon.com>
> Signed-off-by: David Arinzon <darinzon@amazon.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

> +       }
> +
>          /* 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
> 
>
Arinzon, David Jan. 30, 2024, 9:39 a.m. UTC | #2 
> On 1/29/2024 12:55 AM, darinzon@amazon.com wrote:
> >
> > From: David Arinzon <darinzon@amazon.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@amazon.com>
> > Signed-off-by: David Arinzon <darinzon@amazon.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
> >
> >
diff mbox series

Patch

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;
+	}
+
 	/* 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: