netdev - Re: [PATCH net-next v2 2/5] bnxt_en: Refactor bnxt_get

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: <e12379d6-8999-400b-a659-a7e03ea4326c@intel.com>
Date: Wed, 20 Aug 2025 11:08:42 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Michael Chan <michael.chan@...adcom.com>
CC: <netdev@...r.kernel.org>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <andrew+netdev@...n.ch>, <pavan.chebbi@...adcom.com>,
	<andrew.gospodarek@...adcom.com>, Shruti Parab <shruti.parab@...adcom.com>,
	<davem@...emloft.net>
Subject: Re: [PATCH net-next v2 2/5] bnxt_en: Refactor bnxt_get_regs()

On 8/19/25 18:39, Michael Chan wrote:
> From: Shruti Parab <shruti.parab@...adcom.com>
> 
> Separate the code that sends the FW message to retrieve pcie stats into
> a new helper function.  The caller of the helper will call hwrm_req_hold()
> beforehand and so the caller will call hwrm_req_drop() in all cases
> afterwards.  This helper will be useful when adding the support for the
> larger struct pcie_ctx_hw_stats_v2.
> 
> Signed-off-by: Shruti Parab <shruti.parab@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
> v2: Update Changelog
> 
> v1: https://lore.kernel.org/netdev/20250818004940.5663-3-michael.chan@broadcom.com/
> 
> Cc: Przemek Kitszel <przemyslaw.kitszel@...el.com>

thanks, now it looks good
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

> ---
>   .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 43 +++++++++++--------
>   1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 68a4ee9f69b1..2eb7c09a116f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -2074,6 +2074,25 @@ static int bnxt_get_regs_len(struct net_device *dev)
>   	return reg_len;
>   }
>   
> +static void *
> +__bnxt_hwrm_pcie_qstats(struct bnxt *bp, struct hwrm_pcie_qstats_input *req)
> +{
> +	struct pcie_ctx_hw_stats_v2 *hw_pcie_stats;
> +	dma_addr_t hw_pcie_stats_addr;
> +	int rc;
> +
> +	hw_pcie_stats = hwrm_req_dma_slice(bp, req, sizeof(*hw_pcie_stats),
> +					   &hw_pcie_stats_addr);
> +	if (!hw_pcie_stats)
> +		return NULL;
> +
> +	req->pcie_stat_size = cpu_to_le16(sizeof(*hw_pcie_stats));
> +	req->pcie_stat_host_addr = cpu_to_le64(hw_pcie_stats_addr);
> +	rc = hwrm_req_send(bp, req);
> +
> +	return rc ? NULL : hw_pcie_stats;
> +}
> +
>   #define BNXT_PCIE_32B_ENTRY(start, end)			\
>   	 { offsetof(struct pcie_ctx_hw_stats, start),	\
>   	   offsetof(struct pcie_ctx_hw_stats, end) }
> @@ -2088,11 +2107,9 @@ static const struct {
>   static void bnxt_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>   			  void *_p)
>   {
> -	struct pcie_ctx_hw_stats *hw_pcie_stats;
>   	struct hwrm_pcie_qstats_input *req;
>   	struct bnxt *bp = netdev_priv(dev);
> -	dma_addr_t hw_pcie_stats_addr;
> -	int rc;
> +	u8 *src;
>   
>   	regs->version = 0;
>   	if (!(bp->fw_dbg_cap & DBG_QCAPS_RESP_FLAGS_REG_ACCESS_RESTRICTED))
> @@ -2104,24 +2121,14 @@ static void bnxt_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>   	if (hwrm_req_init(bp, req, HWRM_PCIE_QSTATS))
>   		return;
>   
> -	hw_pcie_stats = hwrm_req_dma_slice(bp, req, sizeof(*hw_pcie_stats),
> -					   &hw_pcie_stats_addr);
> -	if (!hw_pcie_stats) {
> -		hwrm_req_drop(bp, req);
> -		return;
> -	}
> -
> -	regs->version = 1;
> -	hwrm_req_hold(bp, req); /* hold on to slice */
> -	req->pcie_stat_size = cpu_to_le16(sizeof(*hw_pcie_stats));
> -	req->pcie_stat_host_addr = cpu_to_le64(hw_pcie_stats_addr);
> -	rc = hwrm_req_send(bp, req);
> -	if (!rc) {
> +	hwrm_req_hold(bp, req);
> +	src = __bnxt_hwrm_pcie_qstats(bp, req);
> +	if (src) {
>   		u8 *dst = (u8 *)(_p + BNXT_PXP_REG_LEN);
> -		u8 *src = (u8 *)hw_pcie_stats;
>   		int i, j;
>   
> -		for (i = 0, j = 0; i < sizeof(*hw_pcie_stats); ) {
> +		regs->version = 1;
> +		for (i = 0, j = 0; i < sizeof(struct pcie_ctx_hw_stats); ) {
>   			if (i >= bnxt_pcie_32b_entries[j].start &&
>   			    i <= bnxt_pcie_32b_entries[j].end) {
>   				u32 *dst32 = (u32 *)(dst + i);