diff mbox series

[net-next,03/16] net/mlx5e: Encapsulate updating netdev queues into a function

Message ID 20200226011246.70129-4-saeedm@mellanox.com
State Accepted
Delegated to: David Miller
Headers show
Series [net-next,01/16] net/mlx5e: Define one flow for TXQ selection when TCs are configured | expand

Commit Message

Saeed Mahameed Feb. 26, 2020, 1:12 a.m. UTC 
From: Maxim Mikityanskiy <maximmi@mellanox.com>

As a preparation for one of the following commits, create a function to
encapsulate the code that notifies the kernel about the new amount of
RX and TX queues. The code will be called multiple times in the next
commit.

Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Jakub Kicinski Feb. 26, 2020, 1:41 a.m. UTC | #1 
On Tue, 25 Feb 2020 17:12:33 -0800 Saeed Mahameed wrote:
> From: Maxim Mikityanskiy <maximmi@mellanox.com>
> 
> As a preparation for one of the following commits, create a function to
> encapsulate the code that notifies the kernel about the new amount of
> RX and TX queues. The code will be called multiple times in the next
> commit.
> 
> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/en_main.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index a4d3e1b6ab20..85a86ff72aac 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -2869,6 +2869,17 @@ static void mlx5e_netdev_set_tcs(struct net_device *netdev)
>  		netdev_set_tc_queue(netdev, tc, nch, 0);
>  }
>  
> +static void mlx5e_update_netdev_queues(struct mlx5e_priv *priv)
> +{
> +	int num_txqs = priv->channels.num * priv->channels.params.num_tc;
> +	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
> +	struct net_device *netdev = priv->netdev;
> +
> +	mlx5e_netdev_set_tcs(netdev);
> +	netif_set_real_num_tx_queues(netdev, num_txqs);
> +	netif_set_real_num_rx_queues(netdev, num_rxqs);
> +}
> +
>  static void mlx5e_build_txq_maps(struct mlx5e_priv *priv)
>  {
>  	int i, ch;
> @@ -2890,13 +2901,7 @@ static void mlx5e_build_txq_maps(struct mlx5e_priv *priv)
>  
>  void mlx5e_activate_priv_channels(struct mlx5e_priv *priv)
>  {
> -	int num_txqs = priv->channels.num * priv->channels.params.num_tc;
> -	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
> -	struct net_device *netdev = priv->netdev;
> -
> -	mlx5e_netdev_set_tcs(netdev);
> -	netif_set_real_num_tx_queues(netdev, num_txqs);
> -	netif_set_real_num_rx_queues(netdev, num_rxqs);
> +	mlx5e_update_netdev_queues(priv);

Not sure where we stand on just moving bad code, but set_real_num_
_queues can fail, Dave just pointed this out to someone recently in
review.

>  
>  	mlx5e_build_txq_maps(priv);
>  	mlx5e_activate_channels(&priv->channels);
Saeed Mahameed Feb. 26, 2020, 7:38 a.m. UTC | #2 
On Tue, 2020-02-25 at 17:41 -0800, Jakub Kicinski wrote:
> On Tue, 25 Feb 2020 17:12:33 -0800 Saeed Mahameed wrote:
> > From: Maxim Mikityanskiy <maximmi@mellanox.com>
> > 
> > As a preparation for one of the following commits, create a
> > function to
> > encapsulate the code that notifies the kernel about the new amount
> > of
> > RX and TX queues. The code will be called multiple times in the
> > next
> > commit.
> > 
> > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> > Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> >  .../net/ethernet/mellanox/mlx5/core/en_main.c | 19 ++++++++++++---
> > ----
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index a4d3e1b6ab20..85a86ff72aac 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -2869,6 +2869,17 @@ static void mlx5e_netdev_set_tcs(struct
> > net_device *netdev)
> >  		netdev_set_tc_queue(netdev, tc, nch, 0);
> >  }
> >  
> > +static void mlx5e_update_netdev_queues(struct mlx5e_priv *priv)
> > +{
> > +	int num_txqs = priv->channels.num * priv-
> > >channels.params.num_tc;
> > +	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
> > +	struct net_device *netdev = priv->netdev;
> > +
> > +	mlx5e_netdev_set_tcs(netdev);
> > +	netif_set_real_num_tx_queues(netdev, num_txqs);
> > +	netif_set_real_num_rx_queues(netdev, num_rxqs);
> > +}
> > +
> >  static void mlx5e_build_txq_maps(struct mlx5e_priv *priv)
> >  {
> >  	int i, ch;
> > @@ -2890,13 +2901,7 @@ static void mlx5e_build_txq_maps(struct
> > mlx5e_priv *priv)
> >  
> >  void mlx5e_activate_priv_channels(struct mlx5e_priv *priv)
> >  {
> > -	int num_txqs = priv->channels.num * priv-
> > >channels.params.num_tc;
> > -	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
> > -	struct net_device *netdev = priv->netdev;
> > -
> > -	mlx5e_netdev_set_tcs(netdev);
> > -	netif_set_real_num_tx_queues(netdev, num_txqs);
> > -	netif_set_real_num_rx_queues(netdev, num_rxqs);
> > +	mlx5e_update_netdev_queues(priv);
> 
> Not sure where we stand on just moving bad code, but set_real_num_
> _queues can fail, Dave just pointed this out to someone recently in
> review.
> 

good point, but likewise not sure if this patch is the place to fix the
issue, the good thing is that now it is fixable since maxim added the
ability to allow failing mlx5_switch_channels() and roll back.

in patch 6 of this series;
net/mlx5e: Fix configuration of XPS cpumasks and netdev queues in
corner cases

you can see that Maxim is using the new function introduced in this
patch as a hook that is allowed to fail and bail out when switching
between channels:

err = mlx5e_safe_switch_channels(priv, &new_channels,
mlx5e_num_channels_changed);

so all we need is to not ignore the return value of
netif_set_real_num_tx_queues() and abort mlx5e_num_channels_changed()
in case of failure which will abort mlx5e_safe_switch_channels() and
old channels will keep working.

I guess this should be an incremental oneliner patch after this series.

Thanks,
Saeed.

> >  
> >  	mlx5e_build_txq_maps(priv);
> >  	mlx5e_activate_channels(&priv->channels);
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a4d3e1b6ab20..85a86ff72aac 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2869,6 +2869,17 @@  static void mlx5e_netdev_set_tcs(struct net_device *netdev)
 		netdev_set_tc_queue(netdev, tc, nch, 0);
 }
 
+static void mlx5e_update_netdev_queues(struct mlx5e_priv *priv)
+{
+	int num_txqs = priv->channels.num * priv->channels.params.num_tc;
+	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
+	struct net_device *netdev = priv->netdev;
+
+	mlx5e_netdev_set_tcs(netdev);
+	netif_set_real_num_tx_queues(netdev, num_txqs);
+	netif_set_real_num_rx_queues(netdev, num_rxqs);
+}
+
 static void mlx5e_build_txq_maps(struct mlx5e_priv *priv)
 {
 	int i, ch;
@@ -2890,13 +2901,7 @@  static void mlx5e_build_txq_maps(struct mlx5e_priv *priv)
 
 void mlx5e_activate_priv_channels(struct mlx5e_priv *priv)
 {
-	int num_txqs = priv->channels.num * priv->channels.params.num_tc;
-	int num_rxqs = priv->channels.num * priv->profile->rq_groups;
-	struct net_device *netdev = priv->netdev;
-
-	mlx5e_netdev_set_tcs(netdev);
-	netif_set_real_num_tx_queues(netdev, num_txqs);
-	netif_set_real_num_rx_queues(netdev, num_rxqs);
+	mlx5e_update_netdev_queues(priv);
 
 	mlx5e_build_txq_maps(priv);
 	mlx5e_activate_channels(&priv->channels);