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

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:   Fri, 3 Mar 2023 15:50:39 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Shay Agroskin <shayagr@...zon.com>
Cc:     David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
        "Woodhouse, David" <dwmw@...zon.com>,
        "Machulsky, Zorik" <zorik@...zon.com>,
        "Matushevsky, Alexander" <matua@...zon.com>,
        Saeed Bshara <saeedb@...zon.com>,
        "Wilson, Matt" <msw@...zon.com>,
        "Liguori, Anthony" <aliguori@...zon.com>,
        "Bshara, Nafea" <nafea@...zon.com>,
        "Belgazal, Netanel" <netanel@...zon.com>,
        "Saidi, Ali" <alisaidi@...zon.com>,
        "Herrenschmidt, Benjamin" <benh@...zon.com>,
        "Kiyanovski, Arthur" <akiyano@...zon.com>,
        "Dagan, Noam" <ndagan@...zon.com>,
        "Arinzon, David" <darinzon@...zon.com>,
        "Itzko, Shahar" <itzko@...zon.com>,
        "Abboud, Osama" <osamaabb@...zon.com>
Subject: Re: [PATCH RFC v2 net-next 4/4] net: ena: Add support to changing
 tx_push_buf_len

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);