netdev - Re: [net-next PATCH v3 08/15] eth: fbnic: Implement Tx queue alloc/start/stop/free

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: <20240703201502.GS598357@kernel.org>
Date: Wed, 3 Jul 2024 21:15:02 +0100
From: Simon Horman <horms@...nel.org>
To: Alexander Duyck <alexander.duyck@...il.com>
Cc: netdev@...r.kernel.org, Alexander Duyck <alexanderduyck@...com>,
	kuba@...nel.org, davem@...emloft.net, pabeni@...hat.com,
	edumazet@...gle.com, kernel-team@...a.com
Subject: Re: [net-next PATCH v3 08/15] eth: fbnic: Implement Tx queue
 alloc/start/stop/free

On Tue, Jul 02, 2024 at 08:00:11AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@...com>
> 
> Implement basic management operations for Tx queues.
> Allocate memory for submission and completion rings.
> Learn how to start the queues, stop them, and wait for HW
> to be idle.
> 
> We call HW rings "descriptor rings" (stored in ring->desc),
> and SW context rings "buffer rings" (stored in ring->*_buf union).
> 
> This is the first patch which actually touches CSRs so add CSR
> helpers.
> 
> No actual datapath / packet handling here, yet.
> 
> Signed-off-by: Alexander Duyck <alexanderduyck@...com>

...

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c

...

> +void fbnic_fill(struct fbnic_net *fbn)

Hi Alexander,

Although it is done as part of a later patch in the series,
to avoid W=1 builds complaining, it would be best to add
a declaration of fbnic_fill to this patch.

> +{
> +	struct fbnic_napi_vector *nv;
> +
> +	list_for_each_entry(nv, &fbn->napis, napis) {
> +		int i;
> +
> +		/* Configure NAPI mapping for Tx */
> +		for (i = 0; i < nv->txt_count; i++) {
> +			struct fbnic_q_triad *qt = &nv->qt[i];
> +
> +			/* Nothing to do if Tx queue is disabled */
> +			if (qt->sub0.flags & FBNIC_RING_F_DISABLED)
> +				continue;
> +
> +			/* Associate Tx queue with NAPI */
> +			netif_queue_set_napi(nv->napi.dev, qt->sub0.q_idx,
> +					     NETDEV_QUEUE_TYPE_TX, &nv->napi);
> +		}

It is fixed in a subsequent patch of this series,
but a '}' should go here.

> +}

-- 
pw-bot: changes-requested