Re: [PATCH 1/1] net/mana: add 32 bit short doorbell

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 9/9/2023 1:23 PM, Wei Hu wrote:
> Add 32 bit short doorbell support. Ring short doorbell when running
> in 32 bit applicactions.
> 

Hi Wei,

Is this performance improvement for 32 bit, or is short doorbell support
required for 32 bit support?


This patch is using RTE_ARCH_32 compile time macro to enable short
doorbell support, so need to decide to support 32 bit or 64 bit in
compile time.

Also I guess 32 bit driver can run on 64 bit arch, what will be the
result in that case?

My point is, instead of using compile time flag, what do you think to
detect execution platform on runtime and use preferred doorbell
according platform?

I can see short descriptor support touches multiple functions, can the
support be abstracted to let to use it based on runtime detection?

> Cc: stable@dpdk.org
> 

Similar comment as previous patch, this patch is not a fix but adding
new support, not sure about backporting it.

> Signed-off-by: Wei Hu <weh@microsoft.com>
>

<...>

> @@ -97,6 +110,7 @@ mana_alloc_and_post_rx_wqe(struct mana_rxq *rxq)
>  		/* update queue for tracking pending packets */
>  		desc->pkt = mbuf;
>  		desc->wqe_size_in_bu = wqe_size_in_bu;
> +		rxq->wqe_cnt_to_short_db += wqe_size_in_bu;
>

This variable always used within RTE_ARCH_32 block, but set here without
RTE_ARCH_32 ifdef, is this intentional?