netdev - Re: [PATCH net-next 1/5] sfc: add new helper macros to iterate channels by type

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 11 May 2022 08:19:45 +0100
From:   Martin Habets <habetsm.xilinx@...il.com>
To:     Íñigo Huguet <ihuguet@...hat.com>
Cc:     ecree.xilinx@...il.com, ap420073@...il.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, dinang@...inx.com, pabloc@...inx.com
Subject: Re: [PATCH net-next 1/5] sfc: add new helper macros to iterate
 channels by type

Hi Íñigo,

On Tue, May 10, 2022 at 10:44:39AM +0200, Íñigo Huguet wrote:
> Sometimes in the driver it's needed to iterate a subset of the channels
> depending on whether it is an rx, tx or xdp channel. Now it's done
> iterating over all channels and checking if it's of the desired type,
> leading to too much nested and a bit complex to understand code.
> 
> Add new iterator macros to allow iterating only over a single type of
> channel.

We have similar code we'll be upstreaming soon, once we've managed to
split off Siena. The crucial part of that seems to have been done
today.

> Signed-off-by: Íñigo Huguet <ihuguet@...hat.com>
> ---
>  drivers/net/ethernet/sfc/net_driver.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 318db906a154..7f665ba6a082 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -1501,6 +1501,27 @@ efx_get_channel(struct efx_nic *efx, unsigned index)
>  	     _channel = (_channel->channel + 1 < (_efx)->n_channels) ?	\
>  		     (_efx)->channel[_channel->channel + 1] : NULL)
>  
> +#define efx_for_each_rx_channel(_channel, _efx)				    \
> +	for (_channel = (_efx)->channel[0];				    \
> +	     _channel;							    \
> +	     _channel = (_channel->channel + 1 < (_efx)->n_rx_channels) ?   \
> +		     (_efx)->channel[_channel->channel + 1] : NULL)
> +#define efx_for_each_tx_channel(_channel, _efx)				    \
> +	for (_channel = (_efx)->channel[efx->tx_channel_offset];	    \
> +	     _channel;							    \
> +	     _channel = (_channel->channel + 1 <			    \
> +		     (_efx)->tx_channel_offset + (_efx)->n_tx_channels) ?   \
> +		     (_efx)->channel[_channel->channel + 1] : NULL)

We've chosen a different naming conventions here, and we're also removing
the channel array.
Also not every channel has RX queues and not every channel has TX queues.

Sounds like it's time we have another call.
Martin

> +#define efx_for_each_xdp_channel(_channel, _efx)			    \
> +	for (_channel = ((_efx)->n_xdp_channels > 0) ?			    \
> +		     (_efx)->channel[efx->xdp_channel_offset] : NULL;	    \
> +	     _channel;							    \
> +	     _channel = (_channel->channel + 1 <			    \
> +		     (_efx)->xdp_channel_offset + (_efx)->n_xdp_channels) ? \
> +		     (_efx)->channel[_channel->channel + 1] : NULL)
> +
>  /* Iterate over all used channels in reverse */
>  #define efx_for_each_channel_rev(_channel, _efx)			\
>  	for (_channel = (_efx)->channel[(_efx)->n_channels - 1];	\
> -- 
> 2.34.1