Date:   Mon, 30 Jan 2023 11:32:22 +0100
From:   Jiri Pirko <jiri@resnulli.us>
To:     Ratheesh Kannoth <rkannoth@marvell.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, leon@kernel.org, sgoutham@marvell.com
Subject: Re: [net PATCH v1] octeontx2-af: Fix devlink unregister
Message-ID: <Y9ecthytH/d+xV5v@nanopsycho>
References: <20230130060443.763564-1-rkannoth@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230130060443.763564-1-rkannoth@marvell.com>
Precedence: bulk

Mon, Jan 30, 2023 at 07:04:43AM CET, rkannoth@marvell.com wrote:
>Exact match devlink entry is only for CN10K-B.
>Unregistration devlink should subtract this
>entry before invoking devlink unregistration
>
>Fixes: 87e4ea29b030 ("octeontx2-af: Debugsfs support for exact match.")
>Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
>---
> .../marvell/octeontx2/af/rvu_devlink.c        | 37 ++++++++++++++-----
> 1 file changed, 28 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>index bda1a6fa2ec4..4950eb13a79f 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>@@ -1500,6 +1500,9 @@ static const struct devlink_param rvu_af_dl_params[] = {
> 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> 			     rvu_af_dl_dwrr_mtu_get, rvu_af_dl_dwrr_mtu_set,
> 			     rvu_af_dl_dwrr_mtu_validate),
>+};
>+
>+static const struct devlink_param rvu_af_dl_param_exact_match[] = {
> 	DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NPC_EXACT_FEATURE_DISABLE,
> 			     "npc_exact_feature_disable", DEVLINK_PARAM_TYPE_STRING,
> 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>@@ -1556,7 +1559,6 @@ int rvu_register_dl(struct rvu *rvu)
> {
> 	struct rvu_devlink *rvu_dl;
> 	struct devlink *dl;
>-	size_t size;
> 	int err;
> 
> 	dl = devlink_alloc(&rvu_devlink_ops, sizeof(struct rvu_devlink),
>@@ -1578,18 +1580,29 @@ int rvu_register_dl(struct rvu *rvu)
> 		goto err_dl_health;
> 	}
> 
>-	/* Register exact match devlink only for CN10K-B */
>-	size = ARRAY_SIZE(rvu_af_dl_params);
>-	if (!rvu_npc_exact_has_match_table(rvu))
>-		size -= 1;
>-
>-	err = devlink_params_register(dl, rvu_af_dl_params, size);
>+	err = devlink_params_register(dl, rvu_af_dl_params, ARRAY_SIZE(rvu_af_dl_params));
> 	if (err) {
> 		dev_err(rvu->dev,
> 			"devlink params register failed with error %d", err);
> 		goto err_dl_health;
> 	}
> 
>+	/* Register exact match devlink only for CN10K-B */
>+	if (!rvu_npc_exact_has_match_table(rvu))
>+		goto done;
>+
>+	err = devlink_params_register(dl, rvu_af_dl_param_exact_match,
>+				      ARRAY_SIZE(rvu_af_dl_param_exact_match));
>+	if (!err)
>+		goto done;

Could you please follow the common error path pattern?
	if (err)
		goto err_something;

	return 0;


>+
>+	dev_err(rvu->dev,
>+		"devlink exact match params register failed with error %d", err);
>+
>+	devlink_params_unregister(dl, rvu_af_dl_params, ARRAY_SIZE(rvu_af_dl_params));
>+	goto err_dl_health;
>+
>+done:
> 	devlink_register(dl);
> 	return 0;
> 
>@@ -1605,8 +1618,14 @@ void rvu_unregister_dl(struct rvu *rvu)
> 	struct devlink *dl = rvu_dl->dl;
> 
> 	devlink_unregister(dl);
>-	devlink_params_unregister(dl, rvu_af_dl_params,
>-				  ARRAY_SIZE(rvu_af_dl_params));
>+
>+	devlink_params_unregister(dl, rvu_af_dl_params, ARRAY_SIZE(rvu_af_dl_params));
>+
>+	/* Unregister exact match devlink only for CN10K-B */
>+	if (rvu_npc_exact_has_match_table(rvu))
>+		devlink_params_unregister(dl, rvu_af_dl_param_exact_match,
>+					  ARRAY_SIZE(rvu_af_dl_param_exact_match));
>+
> 	rvu_health_reporters_destroy(rvu);
> 	devlink_free(dl);
> }
>-- 
>2.25.1
>