netdev - Re: [PATCH net-next v2 5/5] enic: Adjust used MSI-X wq/rq/cq/interrupt resources in a more robust way

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] [day] [month] [year] [list]

Message-ID: <20241031181909.4f12b240@kernel.org>
Date: Thu, 31 Oct 2024 18:19:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Nelson Escobar via B4 Relay <devnull+neescoba.cisco.com@...nel.org>
Cc: neescoba@...co.com, John Daley <johndale@...co.com>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Christian Benvenuti
 <benve@...co.com>, Satish Kharat <satishkh@...co.com>, Andrew Lunn
 <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Simon Horman
 <horms@...nel.org>
Subject: Re: [PATCH net-next v2 5/5] enic: Adjust used MSI-X
 wq/rq/cq/interrupt resources in a more robust way

On Thu, 24 Oct 2024 18:19:47 -0700 Nelson Escobar via B4 Relay wrote:
> To accomplish this do the following:
> - Make enic_set_intr_mode() only set up interrupt related stuff.
> - Move resource adjustment out of enic_set_intr_mode() into its own
>   function, and basing the resources used on the most constrained
>   resource.
> - Move the kdump resources limitations into the new function too.

Please try to split the pure code moves / refactors to separate
commits, this change is quite hard to review.

> +	case VNIC_DEV_INTR_MODE_MSIX:
> +		/* Adjust the number of wqs/rqs/cqs/interrupts that will be
> +		 * used based on which resource is the most constrained
> +		 */
> +		wq_avail = min(enic->wq_avail, ENIC_WQ_MAX);
> +		rq_avail = min(enic->rq_avail, ENIC_RQ_MAX);
> +		max_queues = min(enic->cq_avail,
> +				 enic->intr_avail - ENIC_MSIX_RESERVED_INTR);
> +		if (wq_avail + rq_avail <= max_queues) {
> +			/* we have enough cq and interrupt resources to cover
> +			 *  the number of wqs and rqs
> +			 */
> +			enic->rq_count = rq_avail;
> +			enic->wq_count = wq_avail;
> +		} else {
> +			/* recalculate wq/rq count */
> +			if (rq_avail < wq_avail) {
> +				enic->rq_count = min(rq_avail, max_queues / 2);
> +				enic->wq_count = max_queues - enic->rq_count;
> +			} else {
> +				enic->wq_count = min(wq_avail, max_queues / 2);
> +				enic->rq_count = max_queues - enic->wq_count;
> +			}
> +		}

I don't see netif_get_num_default_rss_queues() being used and you're
now moving into the "serious queue count" territory. Please cap the
default number of allocated Rx rings to what the helper returns.
-- 
pw-bot: cr