Re: [PATCH net-next 5/5] sfc: move tx_channel_offset calculation to interrupts probe

[Martin Habets <habetsm.xilinx@gmail.com>]

On Tue, May 10, 2022 at 10:44:43AM +0200, Íñigo Huguet wrote:
> All parameters related to what channels are used for RX, TX and/or XDP
> are calculated in efx_probe_interrupts or its called function
> efx_allocate_msix_channels.
> 
> tx_channel_offset was recalculated needlessly in efx_set_queues. Remove
> this from here since it's more coherent to calculate it only once, in
> the same place than the rest of channels parameters. If MSIX is not used,
> this value was not set in efx_probe_interrupts, so let's do it now.
> 
> The value calculated in efx_set_queues was wrong anyway, because with
> the addition of the support for XDP, additional channels had been added
> after the TX channels, and efx->n_channels - efx->n_tx_channels didn't
> point to the beginning of the TX channels any more.

Apart from the reformatting this fixes a bug in the existing code.
Please submit this bug fix part again as an individual patch.

Martin

> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> ---
>  drivers/net/ethernet/sfc/efx_channels.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index f6634faa1ec4..b9bbef07bb5e 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -220,14 +220,9 @@ static int efx_allocate_msix_channels(struct efx_nic *efx,
>  	n_channels -= efx->n_xdp_channels;
>  
>  	if (efx_separate_tx_channels) {
> -		efx->n_tx_channels =
> -			min(max(n_channels / 2, 1U),
> -			    efx->max_tx_channels);
> -		efx->tx_channel_offset =
> -			n_channels - efx->n_tx_channels;
> -		efx->n_rx_channels =
> -			max(n_channels -
> -			    efx->n_tx_channels, 1U);
> +		efx->n_tx_channels = min(max(n_channels / 2, 1U), efx->max_tx_channels);
> +		efx->tx_channel_offset = n_channels - efx->n_tx_channels;
> +		efx->n_rx_channels = max(n_channels - efx->n_tx_channels, 1U);
>  	} else {
>  		efx->n_tx_channels = min(n_channels, efx->max_tx_channels);
>  		efx->tx_channel_offset = 0;
> @@ -303,6 +298,7 @@ int efx_probe_interrupts(struct efx_nic *efx)
>  		efx->n_channels = 1;
>  		efx->n_rx_channels = 1;
>  		efx->n_tx_channels = 1;
> +		efx->tx_channel_offset = 0;
>  		efx->n_xdp_channels = 0;
>  		efx->xdp_channel_offset = efx->n_channels;
>  		rc = pci_enable_msi(efx->pci_dev);
> @@ -323,6 +319,7 @@ int efx_probe_interrupts(struct efx_nic *efx)
>  		efx->n_channels = 1 + (efx_separate_tx_channels ? 1 : 0);
>  		efx->n_rx_channels = 1;
>  		efx->n_tx_channels = 1;
> +		efx->tx_channel_offset = 1;
>  		efx->n_xdp_channels = 0;
>  		efx->xdp_channel_offset = efx->n_channels;
>  		efx->legacy_irq = efx->pci_dev->irq;
> @@ -952,10 +949,6 @@ int efx_set_queues(struct efx_nic *efx)
>  	unsigned int queue_num = 0;
>  	int rc;
>  
> -	efx->tx_channel_offset =
> -		efx_separate_tx_channels ?
> -		efx->n_channels - efx->n_tx_channels : 0;
> -
>  	/* We need to mark which channels really have RX and TX queues, and
>  	 * adjust the TX queue numbers if we have separate RX/TX only channels.
>  	 */
> -- 
> 2.34.1