Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 10/05/2018 5:36 PM, Tariq Toukan wrote: On 10/05/2018 5:18 PM, Dan Carpenter wrote: On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: If an error occurs, 'mlx4_en_destroy_netdev()' is called. It then calls 'mlx4_en_free_resources()' which does the needed resources cleanup. So, doing some explicit kfree in the error handling path would lead to some double kfree. Patch make sense but what's bothering me is that mlx4_en_free_resources loops on the entire array, assuming !priv->tx_ring[t] means entry is allocated but the existing code does not assume that, see [1]. So i looked to see where tx_ring array is zeroed and didn't find it. Am i missing something here. It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter We do zero (twice) on init, that's right. But I think Yuval's comment is valid in case of the driver went into configuration change, or down/up, that reallocates the rings. I'm double checking this. Well, the flows in which we need to nullify the tx_rings pointer (if any, I still need to investigate this) is not related to this init function. Here we're safe. Anyway, a V2 is already submitted, please use it for your next comments. I think patch is OK.
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 10/05/2018 5:36 PM, Tariq Toukan wrote: On 10/05/2018 5:18 PM, Dan Carpenter wrote: On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: If an error occurs, 'mlx4_en_destroy_netdev()' is called. It then calls 'mlx4_en_free_resources()' which does the needed resources cleanup. So, doing some explicit kfree in the error handling path would lead to some double kfree. Patch make sense but what's bothering me is that mlx4_en_free_resources loops on the entire array, assuming !priv->tx_ring[t] means entry is allocated but the existing code does not assume that, see [1]. So i looked to see where tx_ring array is zeroed and didn't find it. Am i missing something here. It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter We do zero (twice) on init, that's right. But I think Yuval's comment is valid in case of the driver went into configuration change, or down/up, that reallocates the rings. I'm double checking this. Well, the flows in which we need to nullify the tx_rings pointer (if any, I still need to investigate this) is not related to this init function. Here we're safe. Anyway, a V2 is already submitted, please use it for your next comments. I think patch is OK.
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 10/05/2018 5:18 PM, Dan Carpenter wrote: On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: If an error occurs, 'mlx4_en_destroy_netdev()' is called. It then calls 'mlx4_en_free_resources()' which does the needed resources cleanup. So, doing some explicit kfree in the error handling path would lead to some double kfree. Patch make sense but what's bothering me is that mlx4_en_free_resources loops on the entire array, assuming !priv->tx_ring[t] means entry is allocated but the existing code does not assume that, see [1]. So i looked to see where tx_ring array is zeroed and didn't find it. Am i missing something here. It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter We do zero (twice) on init, that's right. But I think Yuval's comment is valid in case of the driver went into configuration change, or down/up, that reallocates the rings. I'm double checking this.
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 10/05/2018 5:18 PM, Dan Carpenter wrote: On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: If an error occurs, 'mlx4_en_destroy_netdev()' is called. It then calls 'mlx4_en_free_resources()' which does the needed resources cleanup. So, doing some explicit kfree in the error handling path would lead to some double kfree. Patch make sense but what's bothering me is that mlx4_en_free_resources loops on the entire array, assuming !priv->tx_ring[t] means entry is allocated but the existing code does not assume that, see [1]. So i looked to see where tx_ring array is zeroed and didn't find it. Am i missing something here. It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter We do zero (twice) on init, that's right. But I think Yuval's comment is valid in case of the driver went into configuration change, or down/up, that reallocates the rings. I'm double checking this.
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
Le 10/05/2018 à 15:38, Yuval Shaia a écrit :
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.
So, doing some explicit kfree in the error handling path would lead to
some double kfree.
Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.
Am i missing something here.
My understanding is that the array is zeoed at line 3289, when the whole
priv struct is memset(0)'ed (also done in alloc_etherdev_mqs but leaving
an explicit memset help to remind that the struct is zeroed)
If speed matters here (and I don't think so), the memset could be saved
(the mlx4_en_priv struct is quite big after all), but at least a comment
should remind that it is initialized within alloc_etherdev_mqs.
CJ
Simplify code to avoid such a case.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
- goto err_free_tx;
+ goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
- kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
return 0;
-err_free_tx:
- while (t--) {
[1]
- kfree(priv->tx_ring[t]);
- kfree(priv->tx_cq[t]);
- }
out:
mlx4_en_destroy_netdev(dev);
return err;
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
Le 10/05/2018 à 15:38, Yuval Shaia a écrit :
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.
So, doing some explicit kfree in the error handling path would lead to
some double kfree.
Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.
Am i missing something here.
My understanding is that the array is zeoed at line 3289, when the whole
priv struct is memset(0)'ed (also done in alloc_etherdev_mqs but leaving
an explicit memset help to remind that the struct is zeroed)
If speed matters here (and I don't think so), the memset could be saved
(the mlx4_en_priv struct is quite big after all), but at least a comment
should remind that it is initialized within alloc_etherdev_mqs.
CJ
Simplify code to avoid such a case.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
- goto err_free_tx;
+ goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
- kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
return 0;
-err_free_tx:
- while (t--) {
[1]
- kfree(priv->tx_ring[t]);
- kfree(priv->tx_cq[t]);
- }
out:
mlx4_en_destroy_netdev(dev);
return err;
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: > On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: > > If an error occurs, 'mlx4_en_destroy_netdev()' is called. > > It then calls 'mlx4_en_free_resources()' which does the needed resources > > cleanup. > > > > So, doing some explicit kfree in the error handling path would lead to > > some double kfree. > > Patch make sense but what's bothering me is that mlx4_en_free_resources > loops on the entire array, assuming !priv->tx_ring[t] means entry is > allocated but the existing code does not assume that, see [1]. So i looked > to see where tx_ring array is zeroed and didn't find it. > > Am i missing something here. > It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: > On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: > > If an error occurs, 'mlx4_en_destroy_netdev()' is called. > > It then calls 'mlx4_en_free_resources()' which does the needed resources > > cleanup. > > > > So, doing some explicit kfree in the error handling path would lead to > > some double kfree. > > Patch make sense but what's bothering me is that mlx4_en_free_resources > loops on the entire array, assuming !priv->tx_ring[t] means entry is > allocated but the existing code does not assume that, see [1]. So i looked > to see where tx_ring array is zeroed and didn't find it. > > Am i missing something here. > It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
> If an error occurs, 'mlx4_en_destroy_netdev()' is called.
> It then calls 'mlx4_en_free_resources()' which does the needed resources
> cleanup.
>
> So, doing some explicit kfree in the error handling path would lead to
> some double kfree.
Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.
Am i missing something here.
>
> Simplify code to avoid such a case.
>
> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
> Signed-off-by: Christophe JAILLET
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index e0adac4a9a19..9670b33fc9b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
> port,
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_ring[t]) {
> err = -ENOMEM;
> - goto err_free_tx;
> + goto out;
> }
> priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
>MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_cq[t]) {
> - kfree(priv->tx_ring[t]);
> err = -ENOMEM;
> goto out;
> }
> @@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
> port,
>
> return 0;
>
> -err_free_tx:
> - while (t--) {
[1]
> - kfree(priv->tx_ring[t]);
> - kfree(priv->tx_cq[t]);
> - }
> out:
> mlx4_en_destroy_netdev(dev);
> return err;
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
> If an error occurs, 'mlx4_en_destroy_netdev()' is called.
> It then calls 'mlx4_en_free_resources()' which does the needed resources
> cleanup.
>
> So, doing some explicit kfree in the error handling path would lead to
> some double kfree.
Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.
Am i missing something here.
>
> Simplify code to avoid such a case.
>
> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
> Signed-off-by: Christophe JAILLET
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index e0adac4a9a19..9670b33fc9b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
> port,
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_ring[t]) {
> err = -ENOMEM;
> - goto err_free_tx;
> + goto out;
> }
> priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
>MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_cq[t]) {
> - kfree(priv->tx_ring[t]);
> err = -ENOMEM;
> goto out;
> }
> @@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
> port,
>
> return 0;
>
> -err_free_tx:
> - while (t--) {
[1]
> - kfree(priv->tx_ring[t]);
> - kfree(priv->tx_cq[t]);
> - }
> out:
> mlx4_en_destroy_netdev(dev);
> return err;
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.
So, doing some explicit kfree in the error handling path would lead to
some double kfree.
Simplify code to avoid such a case.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
- goto err_free_tx;
+ goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
- kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
return 0;
-err_free_tx:
- while (t--) {
- kfree(priv->tx_ring[t]);
- kfree(priv->tx_cq[t]);
- }
out:
mlx4_en_destroy_netdev(dev);
return err;
--
2.17.0
[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.
So, doing some explicit kfree in the error handling path would lead to
some double kfree.
Simplify code to avoid such a case.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
- goto err_free_tx;
+ goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
- kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
return 0;
-err_free_tx:
- while (t--) {
- kfree(priv->tx_ring[t]);
- kfree(priv->tx_cq[t]);
- }
out:
mlx4_en_destroy_netdev(dev);
return err;
--
2.17.0
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 08/05/2018 12:34 PM, Christophe JAILLET wrote:
If the 2nd memory allocation of the loop fails, we must undo the
memory allocation done so far.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..bf078244e467 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3331,7 +3331,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
- goto out;
+ goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
Hi Christophe,
Thanks for re-sending this.
In your previous mail you referred to the call mlx4_en_destroy_netdev here:
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L232
While I was referring to the one below, which is always called on failures:
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_netdev.c#L3587
I still believe that the err_free_tx label and its while loop is redundant.
Regards,
Tariq
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On 08/05/2018 12:34 PM, Christophe JAILLET wrote:
If the 2nd memory allocation of the loop fails, we must undo the
memory allocation done so far.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..bf078244e467 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3331,7 +3331,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
- goto out;
+ goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
Hi Christophe,
Thanks for re-sending this.
In your previous mail you referred to the call mlx4_en_destroy_netdev here:
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L232
While I was referring to the one below, which is always called on failures:
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_netdev.c#L3587
I still believe that the err_free_tx label and its while loop is redundant.
Regards,
Tariq
[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
If the 2nd memory allocation of the loop fails, we must undo the
memory allocation done so far.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..bf078244e467 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3331,7 +3331,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
- goto out;
+ goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
--
2.17.0
[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
If the 2nd memory allocation of the loop fails, we must undo the
memory allocation done so far.
Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..bf078244e467 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3331,7 +3331,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
- goto out;
+ goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
--
2.17.0
