netdev - Re: [PATCH net-next 2/6] bnxt_en: Refactor aux bus functions to be generic

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: <20250929190601.GC324804@unreal>
Date: Mon, 29 Sep 2025 22:06:01 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc: jgg@...pe.ca, michael.chan@...adcom.com, dave.jiang@...el.com,
	saeedm@...dia.com, Jonathan.Cameron@...wei.com, davem@...emloft.net,
	corbet@....net, edumazet@...gle.com, gospo@...adcom.com,
	kuba@...nel.org, netdev@...r.kernel.org, pabeni@...hat.com,
	andrew+netdev@...n.ch, selvin.xavier@...adcom.com
Subject: Re: [PATCH net-next 2/6] bnxt_en: Refactor aux bus functions to be
 generic

On Mon, Sep 22, 2025 at 02:08:47AM -0700, Pavan Chebbi wrote:
> Up until now there was only one auxiliary device that bnxt
> created and that was for RoCE driver. bnxt fwctl is also
> going to use an aux bus device that bnxt should create.
> This requires some nomenclature changes and refactoring of
> the existing bnxt aux dev functions.
> 
> Make aux bus init/uninit/add/del functions generic which will
> accept aux device type as a parameter. Change aux_dev_ids to
> aux_dev_rdma_ids to mean it is for RoCE driver.
> 
> Also rename the 'aux_priv' and 'edev' members of struct bp to
> 'aux_priv_rdma' and 'edev_rdma' respectively, to mean they belong
> to rdma.
> Rename bnxt_aux_device_release() as bnxt_rdma_aux_device_release()
> 
> Future patches will reuse these functions to add an aux bus device
> for fwctl.
> 
> Reviewed-by: Andy Gospodarek <gospo@...adcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  23 ++--
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   4 +-
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   2 +-
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 102 +++++++++---------
>  include/linux/bnxt/ulp.h                      |  13 ++-
>  5 files changed, 77 insertions(+), 67 deletions(-)

<...>

> index 992eec874345..665850753f90 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -27,11 +27,11 @@
>  #include "bnxt.h"
>  #include "bnxt_hwrm.h"
>  
> -static DEFINE_IDA(bnxt_aux_dev_ids);
> +static DEFINE_IDA(bnxt_rdma_aux_dev_ids);

I would argue that this complexity is not needed, so this and following
two patches are very questionable.

1. The desire is to generate IDs inside auxiliary_device_create() function
and not create IDA per-driver or like in this case per-auxdevice.
2. You are not expected to mix both function pointers and auxdevices
which pretends to be separate devices with separate drivers. Core code
shouldn't call to auxdevice to avoid circular dependency. Auxdevice is
expected to call to core device instead.

Thanks