diff mbox series

[net-next,PatchV2] octeontx2-af: map management port always to first PF

Message ID 20240410132538.20158-1-hkelam@marvell.com (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series [net-next,PatchV2] octeontx2-af: map management port always to first PF | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
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: 8 this patch: 8
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 953 this patch: 953
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: 973 this patch: 973
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 111 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-11--00-00 (tests: 959)

Commit Message

Hariprasad Kelam April 10, 2024, 1:25 p.m. UTC
The user can enable or disable any MAC block or a few ports of the
block. The management port's interface name varies depending on the
setup of the user if its not mapped to the first pf.

The management port mapping is now configured to always connect to the
first PF. This patch implements this change.

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
v2 * Refactor code to avoid code duplication.

 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  5 +-
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 84 +++++++++++++------
 2 files changed, 63 insertions(+), 26 deletions(-)

--
2.17.1

Comments

Jacob Keller April 10, 2024, 10:30 p.m. UTC | #1
On 4/10/2024 6:25 AM, Hariprasad Kelam wrote:
> The user can enable or disable any MAC block or a few ports of the
> block. The management port's interface name varies depending on the
> setup of the user if its not mapped to the first pf.
> 
> The management port mapping is now configured to always connect to the
> first PF. This patch implements this change.
> 

nit: it's generally preferred to avoid "this patch.." language in commit
messages, as it is clear from the context.

That being said, its not worth a re-roll just to fix this.

Reviewed-by: Jacob Keller <jacob.keller@intel.com>
Jakub Kicinski April 12, 2024, 2:55 a.m. UTC | #2
On Wed, 10 Apr 2024 18:55:38 +0530 Hariprasad Kelam wrote:
> The user can enable or disable any MAC block or a few ports of the
> block. The management port's interface name varies depending on the
> setup of the user if its not mapped to the first pf.

There is no concept of management port in Linux networking.
I may be missing the point, but I'm unable to review this in 
the context of the upstream Linux kernel.
Sunil Kovvuri Goutham April 12, 2024, 12:07 p.m. UTC | #3
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, April 12, 2024 8:26 AM
> To: Hariprasad Kelam <hkelam@marvell.com>
> 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
> <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 PatchV2] octeontx2-af: map management
> port always to first PF
> 
> ----------------------------------------------------------------------
> On Wed, 10 Apr 2024 18:55:38 +0530 Hariprasad Kelam wrote:
> > The user can enable or disable any MAC block or a few ports of the
> > block. The management port's interface name varies depending on the
> > setup of the user if its not mapped to the first pf.
> 
> There is no concept of management port in Linux networking.
> I may be missing the point, but I'm unable to review this in the context of the
> upstream Linux kernel.

Agree, that there is no concept of management port in Linux.
From Octeon hardware pov, there are multiple MACs and each MAC (internally called RPM) is capable of supporting multiple interfaces (called LMACs).
Let's say there are two RPMs on the board and RPM0 is configured to 2x50G and RPM1 is configured as 4x10G.
When kernel boots with this config, let's say the interface names are eth0, eth1.. eth5.
If user is using 'eth3' for NFS, DHCP, SSH etc (ie for device management purposes) and then if user changes RPM0
config to 4x10G, then in the subsequent boot the same RPM1:LMAC0 could be named as 'eth5' now.
Customers have reported that their scripts are not working in these scenarios and they want some predictable naming.

What this patch does is that, RPM:LMAC0 which customer is using for management port is always mapped to 
same PCI device, so that interface naming remains unchanged irrespective of different RPM configurations.

Thanks,
Sunil.
Jakub Kicinski April 13, 2024, 1:44 a.m. UTC | #4
On Fri, 12 Apr 2024 12:07:40 +0000 Sunil Kovvuri Goutham wrote:
> Agree, that there is no concept of management port in Linux.
> From Octeon hardware pov, there are multiple MACs and each MAC (internally called RPM) is capable of supporting multiple interfaces (called LMACs).
> Let's say there are two RPMs on the board and RPM0 is configured to 2x50G and RPM1 is configured as 4x10G.
> When kernel boots with this config, let's say the interface names are eth0, eth1.. eth5.
> If user is using 'eth3' for NFS, DHCP, SSH etc (ie for device management purposes) and then if user changes RPM0
> config to 4x10G, then in the subsequent boot the same RPM1:LMAC0 could be named as 'eth5' now.
> Customers have reported that their scripts are not working in these scenarios and they want some predictable naming.
> 
> What this patch does is that, RPM:LMAC0 which customer is using for management port is always mapped to 
> same PCI device, so that interface naming remains unchanged irrespective of different RPM configurations.

You should try to associate a devlink port with the netdev, that will
give it appropriate predictable naming. (Failing that, just implement
ndo_get_phys_port_name directly, but really devlink is much preferred).
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 4a77f6fe2622..88cced83bf23 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -639,7 +639,10 @@  struct cgx_lmac_fwdata_s {
 	/* Only applicable if SFP/QSFP slot is present */
 	struct sfp_eeprom_s sfp_eeprom;
 	struct phy_s phy;
-#define LMAC_FWDATA_RESERVED_MEM 1021
+	u32 lmac_type;
+	u32 portm_idx;
+	u64 mgmt_port:1;
+#define LMAC_FWDATA_RESERVED_MEM 1019
 	u64 reserved[LMAC_FWDATA_RESERVED_MEM];
 };

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 266ecbc1b97a..8cc17d7e368d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -118,15 +118,67 @@  static void rvu_map_cgx_nix_block(struct rvu *rvu, int pf,
 		pfvf->nix_blkaddr = BLKADDR_NIX1;
 }

-static int rvu_map_cgx_lmac_pf(struct rvu *rvu)
+static bool rvu_cgx_is_mgmt_port(struct rvu *rvu, int cgx_id, int lmac_id)
+{
+	struct cgx_lmac_fwdata_s *fwdata;
+
+	fwdata =  &rvu->fwdata->cgx_fw_data_usx[cgx_id][lmac_id];
+	return !!fwdata->mgmt_port;
+}
+
+static void __rvu_map_cgx_lmac_pf(struct rvu *rvu, unsigned int pf,
+				  int cgx, int lmac)
 {
 	struct npc_pkind *pkind = &rvu->hw->pkind;
-	int cgx_cnt_max = rvu->cgx_cnt_max;
-	int pf = PF_CGXMAP_BASE;
+	int numvfs, hwvfs;
+	int free_pkind;
+
+	rvu->pf2cgxlmac_map[pf] = cgxlmac_id_to_bmap(cgx, lmac);
+	rvu->cgxlmac2pf_map[CGX_OFFSET(cgx) + lmac] = 1 << pf;
+	free_pkind = rvu_alloc_rsrc(&pkind->rsrc);
+	pkind->pfchan_map[free_pkind] = ((pf) & 0x3F) << 16;
+	rvu_map_cgx_nix_block(rvu, pf, cgx, lmac);
+	rvu->cgx_mapped_pfs++;
+	rvu_get_pf_numvfs(rvu, pf, &numvfs, &hwvfs);
+	rvu->cgx_mapped_vfs += numvfs;
+}
+
+static void rvu_cgx_map_mgmt_port(struct rvu *rvu, int cgx_cnt_max,
+				  unsigned int *pf, bool req_map_mgmt)
+{
 	unsigned long lmac_bmap;
-	int size, free_pkind;
 	int cgx, lmac, iter;
-	int numvfs, hwvfs;
+
+	for (cgx = 0; cgx < cgx_cnt_max; cgx++) {
+		if (!rvu_cgx_pdata(cgx, rvu))
+			continue;
+		lmac_bmap = cgx_get_lmac_bmap(rvu_cgx_pdata(cgx, rvu));
+		for_each_set_bit(iter, &lmac_bmap, rvu->hw->lmac_per_cgx) {
+			if (iter >= MAX_LMAC_COUNT)
+				continue;
+			lmac = cgx_get_lmacid(rvu_cgx_pdata(cgx, rvu), iter);
+			/* Map management port always to first PF */
+			if (req_map_mgmt &&
+			    rvu_cgx_is_mgmt_port(rvu, cgx, lmac)) {
+				__rvu_map_cgx_lmac_pf(rvu, *pf, cgx, lmac);
+				(*pf)++;
+				return;
+			}
+			/* Non management port mapping */
+			if (!req_map_mgmt &&
+			    !rvu_cgx_is_mgmt_port(rvu, cgx, lmac)) {
+				__rvu_map_cgx_lmac_pf(rvu, *pf, cgx, lmac);
+				(*pf)++;
+			}
+		}
+	}
+}
+
+static int rvu_map_cgx_lmac_pf(struct rvu *rvu)
+{
+	int cgx_cnt_max = rvu->cgx_cnt_max;
+	unsigned int pf = PF_CGXMAP_BASE;
+	int size;

 	if (!cgx_cnt_max)
 		return 0;
@@ -155,26 +207,8 @@  static int rvu_map_cgx_lmac_pf(struct rvu *rvu)
 		return -ENOMEM;

 	rvu->cgx_mapped_pfs = 0;
-	for (cgx = 0; cgx < cgx_cnt_max; cgx++) {
-		if (!rvu_cgx_pdata(cgx, rvu))
-			continue;
-		lmac_bmap = cgx_get_lmac_bmap(rvu_cgx_pdata(cgx, rvu));
-		for_each_set_bit(iter, &lmac_bmap, rvu->hw->lmac_per_cgx) {
-			if (iter >= MAX_LMAC_COUNT)
-				continue;
-			lmac = cgx_get_lmacid(rvu_cgx_pdata(cgx, rvu),
-					      iter);
-			rvu->pf2cgxlmac_map[pf] = cgxlmac_id_to_bmap(cgx, lmac);
-			rvu->cgxlmac2pf_map[CGX_OFFSET(cgx) + lmac] = 1 << pf;
-			free_pkind = rvu_alloc_rsrc(&pkind->rsrc);
-			pkind->pfchan_map[free_pkind] = ((pf) & 0x3F) << 16;
-			rvu_map_cgx_nix_block(rvu, pf, cgx, lmac);
-			rvu->cgx_mapped_pfs++;
-			rvu_get_pf_numvfs(rvu, pf, &numvfs, &hwvfs);
-			rvu->cgx_mapped_vfs += numvfs;
-			pf++;
-		}
-	}
+	rvu_cgx_map_mgmt_port(rvu, cgx_cnt_max, &pf, true);
+	rvu_cgx_map_mgmt_port(rvu, cgx_cnt_max, &pf, false);
 	return 0;
 }