netdev - Re: [PATCH net-next] sfc: handle VI shortage on ef100 by readjusting the channels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40d9d9b9-0500-2c68-c047-20b1a090c0bf@gmail.com>
Date: Thu, 25 May 2023 15:51:03 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@....com>,
 netdev@...r.kernel.org, linux-net-drivers@....com
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
 edumazet@...gle.com, habetsm.xilinx@...il.com
Subject: Re: [PATCH net-next] sfc: handle VI shortage on ef100 by readjusting
 the channels

On 24/05/2023 10:36, Pieter Jansen van Vuuren wrote:
> When fewer VIs are allocated than what is allowed we can readjust
> the channels by calling efx_mcdi_alloc_vis() again.
> 
> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@....com>
> Reviewed-by: Martin Habets <habetsm.xilinx@...il.com>

Reviewed-by: Edward Cree <ecree.xilinx@...il.com>
though see below for one nit (fix in a follow-up?)

> ---
>  drivers/net/ethernet/sfc/ef100_netdev.c | 51 ++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index d916877b5a9a..c201e001f3b8 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -40,19 +40,26 @@ static int ef100_alloc_vis(struct efx_nic *efx, unsigned int *allocated_vis)
>  	unsigned int tx_vis = efx->n_tx_channels + efx->n_extra_tx_channels;
>  	unsigned int rx_vis = efx->n_rx_channels;
>  	unsigned int min_vis, max_vis;
> +	int rc;
>  
>  	EFX_WARN_ON_PARANOID(efx->tx_queues_per_channel != 1);
>  
>  	tx_vis += efx->n_xdp_channels * efx->xdp_tx_per_channel;
>  
>  	max_vis = max(rx_vis, tx_vis);
> -	/* Currently don't handle resource starvation and only accept
> -	 * our maximum needs and no less.
> +	/* We require at least a single complete TX channel worth of queues. */
> +	min_vis = efx->tx_queues_per_channel;
> +
> +	rc = efx_mcdi_alloc_vis(efx, min_vis, max_vis,
> +				NULL, allocated_vis);

I'd like a check here like
    if (rc == -EAGAIN)
            rc = -ESOMETHINGELSE;
 just to avoid confusion if the MC or MCDI machinery returns EAGAIN
 for whatever reason.
Or perhaps better still, don't overload EAGAIN like this and instead
 have this function return 1 in the "we succeeded but didn't get
 max_vis" case (would need a comment above the function, documenting
 this), since that's not a value that can happen in-band.

-ed