diff mbox series

[net-next,4/5] octeontx2-af: replace generic error codes

Message ID 20230817112357.25874-5-hkelam@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series octeontx2-af: misc MAC block changes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1353 this patch: 1353
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 96 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Hariprasad Kelam Aug. 17, 2023, 11:23 a.m. UTC 
currently, if any netdev is not mapped to the MAC block(cgx/rpm)
requests MAC feature, AF driver returns a generic error like -EPERM.
This patch replaces generic error codes with driver-specific error
codes for better debugging

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Jakub Kicinski Aug. 19, 2023, 2:50 a.m. UTC | #1 
On Thu, 17 Aug 2023 16:53:56 +0530 Hariprasad Kelam wrote:
> currently, if any netdev is not mapped to the MAC block(cgx/rpm)
> requests MAC feature, AF driver returns a generic error like -EPERM.
> This patch replaces generic error codes with driver-specific error
> codes for better debugging

The custom error codes are not liked upstream, they make much harder
for people who don't work on the driver to refactor it.

If you want debugging isn't it better to add a tracepoint to the
checks?
Hariprasad Kelam Aug. 20, 2023, 4:12 a.m. UTC | #2 
> ----------------------------------------------------------------------
> On Thu, 17 Aug 2023 16:53:56 +0530 Hariprasad Kelam wrote:
> > currently, if any netdev is not mapped to the MAC block(cgx/rpm)
> > requests MAC feature, AF driver returns a generic error like -EPERM.
> > This patch replaces generic error codes with driver-specific error
> > codes for better debugging
> 
> The custom error codes are not liked upstream, they make much harder for
> people who don't work on the driver to refactor it.
> 
> If you want debugging isn't it better to add a tracepoint to the checks?

Hari>>  These error codes are added in AF mailbox handlers, user space tools like ethool ,tc won't see these since these are between pf netdev and AF. During netdev driver probe/open calls, it requests AF driver to configure different hardware blocks MAC/network etc. If there is any error instead of getting EPERM, we will get block specific error codes like LMAC_AF_ERR_INVALID_PARAM, NIX_AF_ERR_PARAM etc.

Thanks,
Hariprasad k
> --
> pw-bot: cr
Hariprasad Kelam Aug. 24, 2023, 5:41 a.m. UTC | #3 
> -----Original Message-----
> From: Hariprasad Kelam
> Sent: Sunday, August 20, 2023 9:42 AM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> davem@davemloft.net; Sunil Kovvuri Goutham <sgoutham@marvell.com>;
> Geethasowjanya Akula <gakula@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Linu Cherian <lcherian@marvell.com>; Subbaraya
> Sundeep Bhatta <sbhatta@marvell.com>; Naveen Mamindlapalli
> <naveenm@marvell.com>; edumazet@google.com; pabeni@redhat.com
> Subject: Re: [net-next Patch 4/5] octeontx2-af: replace generic error codes
> 
> 
> 
> > ----------------------------------------------------------------------
> > On Thu, 17 Aug 2023 16:53:56 +0530 Hariprasad Kelam wrote:
> > > currently, if any netdev is not mapped to the MAC block(cgx/rpm)
> > > requests MAC feature, AF driver returns a generic error like -EPERM.
> > > This patch replaces generic error codes with driver-specific error
> > > codes for better debugging
> >
> > The custom error codes are not liked upstream, they make much harder
> > for people who don't work on the driver to refactor it.
> >
> > If you want debugging isn't it better to add a tracepoint to the checks?
> 
> Hari>>  These error codes are added in AF mailbox handlers, user space tools
> like ethool ,tc won't see these since these are between pf netdev and AF.
> During netdev driver probe/open calls, it requests AF driver to configure
> different hardware blocks MAC/network etc. If there is any error instead of
> getting EPERM, we will get block specific error codes like
> LMAC_AF_ERR_INVALID_PARAM, NIX_AF_ERR_PARAM etc.

Jakub,
Any comments here?

> 
> Thanks,
> Hariprasad k
> > --
> > pw-bot: cr
Jakub Kicinski Aug. 24, 2023, 3:19 p.m. UTC | #4 
On Thu, 24 Aug 2023 05:41:01 +0000 Hariprasad Kelam wrote:
> > > The custom error codes are not liked upstream, they make much harder
> > > for people who don't work on the driver to refactor it.
> > >
> > > If you want debugging isn't it better to add a tracepoint to the checks?  
> >   
> > Hari>>  These error codes are added in AF mailbox handlers, user space tools  
> > like ethool ,tc won't see these since these are between pf netdev and AF.
> > During netdev driver probe/open calls, it requests AF driver to configure
> > different hardware blocks MAC/network etc. If there is any error instead of
> > getting EPERM, we will get block specific error codes like
> > LMAC_AF_ERR_INVALID_PARAM, NIX_AF_ERR_PARAM etc.  
> 
> Jakub,
> Any comments here?

Please learn how to use email correctly.

Hari>>

Makes the entire line rendered as a quote in many email clients,
because the gt than sign is a quote marker.

You should also wrap your lines.

If you want to return the block which failed to the caller you can 
do that, but not instead of the error code.
Hariprasad Kelam Aug. 25, 2023, 7:20 a.m. UTC | #5 
> On Thu, 24 Aug 2023 05:41:01 +0000 Hariprasad Kelam wrote:
> > > > The custom error codes are not liked upstream, they make much
> > > > harder for people who don't work on the driver to refactor it.
> > > >
> > > > If you want debugging isn't it better to add a tracepoint to the checks?
> > >
> > > Hari>>  These error codes are added in AF mailbox handlers, user
> > > Hari>> space tools
> > > like ethool ,tc won't see these since these are between pf netdev and
> AF.
> > > During netdev driver probe/open calls, it requests AF driver to
> > > configure different hardware blocks MAC/network etc. If there is any
> > > error instead of getting EPERM, we will get block specific error
> > > codes like LMAC_AF_ERR_INVALID_PARAM, NIX_AF_ERR_PARAM etc.
> >
> > Jakub,
> > Any comments here?
> 
> Please learn how to use email correctly.
> 
> Hari>>
> 
> Makes the entire line rendered as a quote in many email clients, because the
> gt than sign is a quote marker.

   ACK
> 
> You should also wrap your lines.
> 
> If you want to return the block which failed to the caller you can do that, but
> not instead of the error code.

Ok, will remove this patch from the series.

Thanks,
Hariprasad k
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 4e3aec7bdbee..c96a94303a54 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -600,7 +600,7 @@  int rvu_mbox_handler_cgx_mac_addr_set(struct rvu *rvu,
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	if (rvu_npc_exact_has_match_table(rvu))
 		return rvu_npc_exact_mac_addr_set(rvu, req, rsp);
@@ -621,7 +621,7 @@  int rvu_mbox_handler_cgx_mac_addr_add(struct rvu *rvu,
 	int rc = 0;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	if (rvu_npc_exact_has_match_table(rvu))
 		return rvu_npc_exact_mac_addr_add(rvu, req, rsp);
@@ -644,7 +644,7 @@  int rvu_mbox_handler_cgx_mac_addr_del(struct rvu *rvu,
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	if (rvu_npc_exact_has_match_table(rvu))
 		return rvu_npc_exact_mac_addr_del(rvu, req, rsp);
@@ -690,7 +690,7 @@  int rvu_mbox_handler_cgx_mac_addr_get(struct rvu *rvu,
 	u64 cfg;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
 
@@ -709,7 +709,7 @@  int rvu_mbox_handler_cgx_promisc_enable(struct rvu *rvu, struct msg_req *req,
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	/* Disable drop on non hit rule */
 	if (rvu_npc_exact_has_match_table(rvu))
@@ -728,7 +728,7 @@  int rvu_mbox_handler_cgx_promisc_disable(struct rvu *rvu, struct msg_req *req,
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	/* Disable drop on non hit rule */
 	if (rvu_npc_exact_has_match_table(rvu))
@@ -798,7 +798,7 @@  static int rvu_cgx_config_linkevents(struct rvu *rvu, u16 pcifunc, bool en)
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
 
@@ -836,7 +836,7 @@  int rvu_mbox_handler_cgx_get_linkinfo(struct rvu *rvu, struct msg_req *req,
 	pf = rvu_get_pf(req->hdr.pcifunc);
 
 	if (!is_pf_cgxmapped(rvu, pf))
-		return -ENODEV;
+		return LMAC_AF_ERR_PF_NOT_MAPPED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
 
@@ -897,7 +897,7 @@  static int rvu_cgx_config_intlbk(struct rvu *rvu, u16 pcifunc, bool en)
 	u8 cgx_id, lmac_id;
 
 	if (!is_cgx_config_permitted(rvu, pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PF_NOT_MAPPED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
 	mac_ops = get_mac_ops(rvu_cgx_pdata(cgx_id, rvu));
@@ -1100,7 +1100,7 @@  int rvu_mbox_handler_cgx_set_fec_param(struct rvu *rvu,
 	u8 cgx_id, lmac_id;
 
 	if (!is_pf_cgxmapped(rvu, pf))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	if (req->fec == OTX2_FEC_OFF)
 		req->fec = OTX2_FEC_NONE;
@@ -1119,7 +1119,7 @@  int rvu_mbox_handler_cgx_get_aux_link_info(struct rvu *rvu, struct msg_req *req,
 		return LMAC_AF_ERR_FIRMWARE_DATA_NOT_MAPPED;
 
 	if (!is_pf_cgxmapped(rvu, pf))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
 
@@ -1144,7 +1144,7 @@  int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
 	void *cgxd;
 
 	if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
-		return -EPERM;
+		return LMAC_AF_ERR_PERM_DENIED;
 
 	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_idx, &lmac);
 	cgxd = rvu_cgx_pdata(cgx_idx, rvu);