Re: [PATCH net-next v7 1/2] net: libwx: add support for set_ringparam in wx_ethtool_ops_vf

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 7/10/26 03:59, Mengyuan Lou wrote:
> Add support for the set_ringparam in wx_ethtool_ops_vf,
> which is used to set ring sizes for ngbevf and txgbevf.
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
> ---
>   .../net/ethernet/wangxun/libwx/wx_ethtool.c   | 61 +++++++++++++++++++
>   drivers/net/ethernet/wangxun/libwx/wx_lib.c   |  9 +--
>   drivers/net/ethernet/wangxun/libwx/wx_lib.h   |  4 +-
>   .../net/ethernet/wangxun/libwx/wx_vf_common.c |  4 +-
>   .../net/ethernet/wangxun/libwx/wx_vf_common.h |  2 +
>   5 files changed, 72 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> index 5df971aca9e3..eae038df6875 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> @@ -9,6 +9,7 @@
>   #include "wx_ethtool.h"
>   #include "wx_hw.h"
>   #include "wx_lib.h"
> +#include "wx_vf_common.h"
>   
>   struct wx_stats {
>   	char stat_string[ETH_GSTRING_LEN];
> @@ -775,6 +776,65 @@ static int wx_get_link_ksettings_vf(struct net_device *netdev,
>   	return 0;
>   }
>   
> +static int wx_set_ringparam_vf(struct net_device *netdev,
> +			       struct ethtool_ringparam *ring,
> +			       struct kernel_ethtool_ringparam *kernel_ring,
> +			       struct netlink_ext_ack *extack)
> +{
> +	struct wx *wx = netdev_priv(netdev);
> +	u32 new_rx_count, new_tx_count;
> +	struct wx_ring *temp_ring;
> +	int i, err = 0;
> +
> +	new_tx_count = clamp_t(u32, ring->tx_pending, WX_MIN_TXD, WX_MAX_TXD);
> +	new_tx_count = ALIGN(new_tx_count, WX_REQ_TX_DESCRIPTOR_MULTIPLE);
> +
> +	new_rx_count = clamp_t(u32, ring->rx_pending, WX_MIN_RXD, WX_MAX_RXD);
> +	new_rx_count = ALIGN(new_rx_count, WX_REQ_RX_DESCRIPTOR_MULTIPLE);
> +
> +	if (new_tx_count == wx->tx_ring_count &&
> +	    new_rx_count == wx->rx_ring_count)
> +		return 0;
> +
> +	mutex_lock(&wx->reset_lock);
> +	set_bit(WX_STATE_RESETTING, wx->state);
> +
> +	if (!netif_running(wx->netdev)) {
> +		for (i = 0; i < wx->num_tx_queues; i++)
> +			wx->tx_ring[i]->count = new_tx_count;
> +		for (i = 0; i < wx->num_rx_queues; i++)
> +			wx->rx_ring[i]->count = new_rx_count;
> +		wx->tx_ring_count = new_tx_count;
> +		wx->rx_ring_count = new_rx_count;
> +
> +		goto clear_reset;
> +	}
> +
> +	/* allocate temporary buffer to store rings in */
> +	i = max_t(int, wx->num_tx_queues, wx->num_rx_queues);
> +	temp_ring = kvmalloc_objs(struct wx_ring, i);
> +	if (!temp_ring) {
> +		err = -ENOMEM;
> +		goto clear_reset;
> +	}

would be much better to move tempbuf allocation into the helper,
and just do it at the beginning there

in the unlikely event of -ENOMEM, you will just call the "up"
in the unroll path

> +
> +	wxvf_down(wx);
> +	/* wx_set_ring() may partially apply changes before
> +	 * returning an error. The error indicates that not all
> +	 * requested ring parameters could be configured.
> +	 */
> +	err = wx_set_ring(wx, new_tx_count, new_rx_count, temp_ring);
> +	if (err)
> +		wx_err(wx, "failed to set ring parameters: %d", err);
> +	wx_configure_vf(wx);
> +	wxvf_up_complete(wx);
> +	kvfree(temp_ring);
> +clear_reset:
> +	clear_bit(WX_STATE_RESETTING, wx->state);
> +	mutex_unlock(&wx->reset_lock);
> +	return err;
> +}