* [PATCH] ionic: Fix rx queue allocation in 'ionic_lif_alloc()'
@ 2020-01-26 10:25 ` Christophe JAILLET
0 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2020-01-26 10:25 UTC (permalink / raw)
To: snelson, drivers, jakub.kicinski, davem
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
The 'struct ionic' has a 'nrxqs_per_lif' field. So use it instead of
using two times the value of 'ntxqs_per_lif'.
Note that with the current implementation, this patch is a no-op because
both fields are set to the same value in 'ionic_lifs_size()' which
is called before reaching 'ionic_lif_alloc()'.
However, it is more future-proof.
Fixes: 1a58e196467f ("ionic: Add basic lif support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Another alternative could be to use 'alloc_etherdev_mq()' if really using
the same value for both fields is what is expected.
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 60fd14df49d7..96d3b3e993ad 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1663,7 +1663,7 @@ static struct ionic_lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index
int err;
netdev = alloc_etherdev_mqs(sizeof(*lif),
- ionic->ntxqs_per_lif, ionic->ntxqs_per_lif);
+ ionic->ntxqs_per_lif, ionic->nrxqs_per_lif);
if (!netdev) {
dev_err(dev, "Cannot allocate netdev, aborting\n");
return ERR_PTR(-ENOMEM);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] ionic: Fix rx queue allocation in 'ionic_lif_alloc()'
@ 2020-01-26 10:25 ` Christophe JAILLET
0 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2020-01-26 10:25 UTC (permalink / raw)
To: snelson, drivers, jakub.kicinski, davem
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
The 'struct ionic' has a 'nrxqs_per_lif' field. So use it instead of
using two times the value of 'ntxqs_per_lif'.
Note that with the current implementation, this patch is a no-op because
both fields are set to the same value in 'ionic_lifs_size()' which
is called before reaching 'ionic_lif_alloc()'.
However, it is more future-proof.
Fixes: 1a58e196467f ("ionic: Add basic lif support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Another alternative could be to use 'alloc_etherdev_mq()' if really using
the same value for both fields is what is expected.
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 60fd14df49d7..96d3b3e993ad 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1663,7 +1663,7 @@ static struct ionic_lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index
int err;
netdev = alloc_etherdev_mqs(sizeof(*lif),
- ionic->ntxqs_per_lif, ionic->ntxqs_per_lif);
+ ionic->ntxqs_per_lif, ionic->nrxqs_per_lif);
if (!netdev) {
dev_err(dev, "Cannot allocate netdev, aborting\n");
return ERR_PTR(-ENOMEM);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ionic: Fix rx queue allocation in 'ionic_lif_alloc()'
2020-01-26 10:25 ` Christophe JAILLET
@ 2020-01-26 18:36 ` Shannon Nelson
-1 siblings, 0 replies; 4+ messages in thread
From: Shannon Nelson @ 2020-01-26 18:36 UTC (permalink / raw)
To: Christophe JAILLET, drivers, jakub.kicinski, davem
Cc: netdev, linux-kernel, kernel-janitors
On 1/26/20 2:25 AM, Christophe JAILLET wrote:
> The 'struct ionic' has a 'nrxqs_per_lif' field. So use it instead of
> using two times the value of 'ntxqs_per_lif'.
>
> Note that with the current implementation, this patch is a no-op because
> both fields are set to the same value in 'ionic_lifs_size()' which
> is called before reaching 'ionic_lif_alloc()'.
>
> However, it is more future-proof.
>
> Fixes: 1a58e196467f ("ionic: Add basic lif support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Another alternative could be to use 'alloc_etherdev_mq()' if really using
> the same value for both fields is what is expected.
> ---
> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 60fd14df49d7..96d3b3e993ad 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -1663,7 +1663,7 @@ static struct ionic_lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index
> int err;
>
> netdev = alloc_etherdev_mqs(sizeof(*lif),
> - ionic->ntxqs_per_lif, ionic->ntxqs_per_lif);
> + ionic->ntxqs_per_lif, ionic->nrxqs_per_lif);
> if (!netdev) {
> dev_err(dev, "Cannot allocate netdev, aborting\n");
> return ERR_PTR(-ENOMEM);
NAK
Even though the NIC configuration seems to allow for different values,
the rest of the driver assumes that the values are equal and things will
likely break if they aren't. This is why the ethtool service
ionic_set_channels() only allows the user to change the Combined value,
and not the individual rx or tx values.
If you really want to tweak this, a better change might be to use the
min of ntxqs and nrxqs, and maybe add a warning log message that they
are configured with different values.
sln
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ionic: Fix rx queue allocation in 'ionic_lif_alloc()'
@ 2020-01-26 18:36 ` Shannon Nelson
0 siblings, 0 replies; 4+ messages in thread
From: Shannon Nelson @ 2020-01-26 18:36 UTC (permalink / raw)
To: Christophe JAILLET, drivers, jakub.kicinski, davem
Cc: netdev, linux-kernel, kernel-janitors
On 1/26/20 2:25 AM, Christophe JAILLET wrote:
> The 'struct ionic' has a 'nrxqs_per_lif' field. So use it instead of
> using two times the value of 'ntxqs_per_lif'.
>
> Note that with the current implementation, this patch is a no-op because
> both fields are set to the same value in 'ionic_lifs_size()' which
> is called before reaching 'ionic_lif_alloc()'.
>
> However, it is more future-proof.
>
> Fixes: 1a58e196467f ("ionic: Add basic lif support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Another alternative could be to use 'alloc_etherdev_mq()' if really using
> the same value for both fields is what is expected.
> ---
> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 60fd14df49d7..96d3b3e993ad 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -1663,7 +1663,7 @@ static struct ionic_lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index
> int err;
>
> netdev = alloc_etherdev_mqs(sizeof(*lif),
> - ionic->ntxqs_per_lif, ionic->ntxqs_per_lif);
> + ionic->ntxqs_per_lif, ionic->nrxqs_per_lif);
> if (!netdev) {
> dev_err(dev, "Cannot allocate netdev, aborting\n");
> return ERR_PTR(-ENOMEM);
NAK
Even though the NIC configuration seems to allow for different values,
the rest of the driver assumes that the values are equal and things will
likely break if they aren't. This is why the ethtool service
ionic_set_channels() only allows the user to change the Combined value,
and not the individual rx or tx values.
If you really want to tweak this, a better change might be to use the
min of ntxqs and nrxqs, and maybe add a warning log message that they
are configured with different values.
sln
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-26 18:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-26 10:25 [PATCH] ionic: Fix rx queue allocation in 'ionic_lif_alloc()' Christophe JAILLET
2020-01-26 10:25 ` Christophe JAILLET
2020-01-26 18:36 ` Shannon Nelson
2020-01-26 18:36 ` Shannon Nelson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.