netdev - Re: [PATCH net-next v3 1/6] net: ethernet: ti: am65-cpsw: Introduce multi queue Rx

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]

Message-ID: <20240705181509.1437b12e@kernel.org>
Date: Fri, 5 Jul 2024 18:15:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Roger Quadros <rogerq@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Siddharth Vadapalli
 <s-vadapalli@...com>, Julien Panis <jpanis@...libre.com>, Simon Horman
 <horms@...nel.org>, Andrew Lunn <andrew@...n.ch>, srk@...com,
 vigneshr@...com, danishanwar@...com, pekka Varis <p-varis@...com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-omap@...r.kernel.org
Subject: Re: [PATCH net-next v3 1/6] net: ethernet: ti: am65-cpsw: Introduce
 multi queue Rx

On Wed, 03 Jul 2024 16:51:32 +0300 Roger Quadros wrote:
>  
> -	if (queue >= AM65_CPSW_MAX_TX_QUEUES)
> +	if (queue >= AM65_CPSW_MAX_TX_QUEUES &&
> +	    queue >= AM65_CPSW_MAX_RX_QUEUES)
>  		return -EINVAL;

both MAXes are 8, the else conditions below are dead code
Same for set

> -	tx_chn = &common->tx_chns[queue];
> +	if (queue < AM65_CPSW_MAX_TX_QUEUES) {
> +		tx_chn = &common->tx_chns[queue];
> +		coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000;
> +	} else {
> +		coal->tx_coalesce_usecs = ~0;
> +	}
>  
> -	coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000;
> +	if (queue < AM65_CPSW_MAX_RX_QUEUES) {
> +		rx_flow = &common->rx_chns.flows[queue];
> +		coal->rx_coalesce_usecs = rx_flow->rx_pace_timeout / 1000;
> +	} else {
> +		coal->rx_coalesce_usecs = ~0;
> +	}

+	for (flow_idx = 0; flow_idx < common->rx_ch_num_flows; flow_idx++) {
+		flow = &rx_chn->flows[flow_idx];
+		for (i = 0; i < AM65_CPSW_MAX_RX_DESC; i++) {
+			page = page_pool_dev_alloc_pages(flow->page_pool);
+			if (!page) {
+				dev_err(common->dev, "cannot allocate page in flow %d\n",
+					flow_idx);
+				ret = -ENOMEM;
+				if (i)
+					goto fail_rx;

-			return ret;
-		}
-		rx_chn->pages[i] = page;
+				return ret;

the direct returns now that it's a double-nested loop seem questionable,
don't you have to goto fail_rx?