Re: [PATCH RFC v2 net-next 4/4] net: ena: Add support to changing tx_push_buf_len

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 2 Mar 2023 22:30:45 +0200 Shay Agroskin wrote:
> @@ -496,11 +509,40 @@ static int ena_set_ringparam(struct net_device *netdev,
>  			ENA_MIN_RING_SIZE : ring->rx_pending;
>  	new_rx_size = rounddown_pow_of_two(new_rx_size);
>  
> -	if (new_tx_size == adapter->requested_tx_ring_size &&
> -	    new_rx_size == adapter->requested_rx_ring_size)
> +	changed |= new_tx_size != adapter->requested_tx_ring_size ||
> +		   new_rx_size != adapter->requested_rx_ring_size;
> +
> +	/* This value is ignored if LLQ is not supported */
> +	new_tx_push_buf_len = 0;
> +	if (adapter->ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST)
> +		goto no_llq_supported;

Are you rejecting the unsupported config in this case or just ignoring
it? You need to return an error if user tries to set something the
device does not support/allow.

BTW your use of gotos to skip code is against the kernel coding style.
gotos are only for complex cases and error handling, you're using them
to save indentation it seems. Factor the code out to a helper instead,
or some such.

> +	new_tx_push_buf_len = kernel_ring->tx_push_buf_len;
> +
> +	/* support for ENA_LLQ_LARGE_HEADER is tested in the 'get' command */
> +	if (new_tx_push_buf_len != ENA_LLQ_HEADER &&
> +	    new_tx_push_buf_len != ENA_LLQ_LARGE_HEADER) {
> +		bool large_llq_sup = adapter->large_llq_header_supported;
> +		char large_llq_size_str[40];
> +
> +		snprintf(large_llq_size_str, 40, ", %lu", ENA_LLQ_LARGE_HEADER);
> +
> +		NL_SET_ERR_MSG_FMT_MOD(extack,
> +				       "Only [%lu%s] tx push buff length values are supported",
> +				       ENA_LLQ_HEADER,
> +				       large_llq_sup ? large_llq_size_str : "");
> +
> +		return -EINVAL;
> +	}
> +
> +	changed |= new_tx_push_buf_len != adapter->ena_dev->tx_max_header_size;
> +
> +no_llq_supported:
> +	if (!changed)
>  		return 0;
>  
> -	return ena_update_queue_sizes(adapter, new_tx_size, new_rx_size);
> +	return ena_update_queue_params(adapter, new_tx_size, new_rx_size,
> +				       new_tx_push_buf_len);