[PATCH net-next] nfp: improve link modes reading process

[PATCH net-next] nfp: improve link modes reading process

[Louis Peens]

From: Yinjun Zhang <yinjun.zhang@corigine.com>

Avoid reading link modes from management firmware every time when
`ethtool_get_link_ksettings` is called, only communicate with
management firmware when necessary like we do for eth_table info.

This change can ease the situation that when large number of vlan
sub-interfaces are created and their information is requested by
some monitoring process like PCP [1] through ethool ioctl frequently.

[1] https://pcp.io

Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com>
Acked-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 32 +++++------
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h  |  7 ++-
 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 54 +++++++++----------
 3 files changed, 45 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index dfedb52b7e70..e75cbb287625 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -436,49 +436,41 @@ static void nfp_add_media_link_mode(struct nfp_port *port,
 				    struct nfp_eth_table_port *eth_port,
 				    struct ethtool_link_ksettings *cmd)
 {
-	u64 supported_modes[2], advertised_modes[2];
-	struct nfp_eth_media_buf ethm = {
-		.eth_index = eth_port->eth_index,
-	};
-	struct nfp_cpp *cpp = port->app->cpp;
-
-	if (nfp_eth_read_media(cpp, &ethm)) {
-		bitmap_fill(port->speed_bitmap, NFP_SUP_SPEED_NUMBER);
-		return;
-	}
-
 	bitmap_zero(port->speed_bitmap, NFP_SUP_SPEED_NUMBER);
 
-	for (u32 i = 0; i < 2; i++) {
-		supported_modes[i] = le64_to_cpu(ethm.supported_modes[i]);
-		advertised_modes[i] = le64_to_cpu(ethm.advertised_modes[i]);
-	}
-
 	for (u32 i = 0; i < NFP_MEDIA_LINK_MODES_NUMBER; i++) {
 		if (i < 64) {
-			if (supported_modes[0] & BIT_ULL(i)) {
+			if (eth_port->link_modes_supp[0] & BIT_ULL(i)) {
 				__set_bit(nfp_eth_media_table[i].ethtool_link_mode,
 					  cmd->link_modes.supported);
 				__set_bit(nfp_eth_media_table[i].speed,
 					  port->speed_bitmap);
 			}
 
-			if (advertised_modes[0] & BIT_ULL(i))
+			if (eth_port->link_modes_ad[0] & BIT_ULL(i))
 				__set_bit(nfp_eth_media_table[i].ethtool_link_mode,
 					  cmd->link_modes.advertising);
 		} else {
-			if (supported_modes[1] & BIT_ULL(i - 64)) {
+			if (eth_port->link_modes_supp[1] & BIT_ULL(i - 64)) {
 				__set_bit(nfp_eth_media_table[i].ethtool_link_mode,
 					  cmd->link_modes.supported);
 				__set_bit(nfp_eth_media_table[i].speed,
 					  port->speed_bitmap);
 			}
 
-			if (advertised_modes[1] & BIT_ULL(i - 64))
+			if (eth_port->link_modes_ad[1] & BIT_ULL(i - 64))
 				__set_bit(nfp_eth_media_table[i].ethtool_link_mode,
 					  cmd->link_modes.advertising);
 		}
 	}
+
+	/* We take all speeds as supported when it fails to read
+	 * link modes due to old management firmware that doesn't
+	 * support link modes reading or error occurring, so that
+	 * speed change of this port is allowed.
+	 */
+	if (bitmap_empty(port->speed_bitmap, NFP_SUP_SPEED_NUMBER))
+		bitmap_fill(port->speed_bitmap, NFP_SUP_SPEED_NUMBER);
 }
 
 /**
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
index 781edc451bd4..6e044ac04917 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
@@ -196,6 +196,9 @@ enum nfp_ethtool_link_mode_list {
  *			subports)
  * @ports.is_split:	is interface part of a split port
  * @ports.fec_modes_supported:	bitmap of FEC modes supported
+ *
+ * @ports.link_modes_supp:	bitmap of link modes supported
+ * @ports.link_modes_ad:	bitmap of link modes advertised
  */
 struct nfp_eth_table {
 	unsigned int count;
@@ -235,6 +238,9 @@ struct nfp_eth_table {
 		bool is_split;
 
 		unsigned int fec_modes_supported;
+
+		u64 link_modes_supp[2];
+		u64 link_modes_ad[2];
 	} ports[];
 };
 
@@ -313,7 +319,6 @@ struct nfp_eth_media_buf {
 };
 
 int nfp_nsp_read_media(struct nfp_nsp *state, void *buf, unsigned int size);
-int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm);
 
 #define NFP_NSP_VERSION_BUFSZ	1024 /* reasonable size, not in the ABI */
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
index 570ac1bb2122..9d62085d772a 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
@@ -227,6 +227,30 @@ nfp_eth_calc_port_type(struct nfp_cpp *cpp, struct nfp_eth_table_port *entry)
 		entry->port_type = PORT_DA;
 }
 
+static void
+nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_nsp *nsp, struct nfp_eth_table_port *entry)
+{
+	struct nfp_eth_media_buf ethm = {
+		.eth_index = entry->eth_index,
+	};
+	unsigned int i;
+	int ret;
+
+	if (!nfp_nsp_has_read_media(nsp))
+		return;
+
+	ret = nfp_nsp_read_media(nsp, &ethm, sizeof(ethm));
+	if (ret) {
+		nfp_err(cpp, "Reading media link modes failed: %d\n", ret);
+		return;
+	}
+
+	for (i = 0; i < 2; i++) {
+		entry->link_modes_supp[i] = le64_to_cpu(ethm.supported_modes[i]);
+		entry->link_modes_ad[i] = le64_to_cpu(ethm.advertised_modes[i]);
+	}
+}
+
 /**
  * nfp_eth_read_ports() - retrieve port information
  * @cpp:	NFP CPP handle
@@ -293,8 +317,10 @@ __nfp_eth_read_ports(struct nfp_cpp *cpp, struct nfp_nsp *nsp)
 					       &table->ports[j++]);
 
 	nfp_eth_calc_port_geometry(cpp, table);
-	for (i = 0; i < table->count; i++)
+	for (i = 0; i < table->count; i++) {
 		nfp_eth_calc_port_type(cpp, &table->ports[i]);
+		nfp_eth_read_media(cpp, nsp, &table->ports[i]);
+	}
 
 	kfree(entries);
 
@@ -647,29 +673,3 @@ int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes)
 	return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
 				      lanes, NSP_ETH_CTRL_SET_LANES);
 }
-
-int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm)
-{
-	struct nfp_nsp *nsp;
-	int ret;
-
-	nsp = nfp_nsp_open(cpp);
-	if (IS_ERR(nsp)) {
-		nfp_err(cpp, "Failed to access the NSP: %pe\n", nsp);
-		return PTR_ERR(nsp);
-	}
-
-	if (!nfp_nsp_has_read_media(nsp)) {
-		nfp_warn(cpp, "Reading media link modes not supported. Please update flash\n");
-		ret = -EOPNOTSUPP;
-		goto exit_close_nsp;
-	}
-
-	ret = nfp_nsp_read_media(nsp, ethm, sizeof(*ethm));
-	if (ret)
-		nfp_err(cpp, "Reading media link modes failed: %pe\n", ERR_PTR(ret));
-
-exit_close_nsp:
-	nfp_nsp_close(nsp);
-	return ret;
-}
-- 
2.34.1

Re: [PATCH net-next] nfp: improve link modes reading process

[Leon Romanovsky]

On Tue, May 09, 2023 at 09:58:17AM +0200, Louis Peens wrote:
> From: Yinjun Zhang <yinjun.zhang@corigine.com>
> 
> Avoid reading link modes from management firmware every time when
> `ethtool_get_link_ksettings` is called, only communicate with
> management firmware when necessary like we do for eth_table info.
> 
> This change can ease the situation that when large number of vlan
> sub-interfaces are created and their information is requested by
> some monitoring process like PCP [1] through ethool ioctl frequently.
> 
> [1] https://pcp.io
> 
> Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com>
> Acked-by: Simon Horman <simon.horman@corigine.com>
> Signed-off-by: Louis Peens <louis.peens@corigine.com>
> ---
>  .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 32 +++++------
>  .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h  |  7 ++-
>  .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 54 +++++++++----------
>  3 files changed, 45 insertions(+), 48 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net-next] nfp: improve link modes reading process

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  9 May 2023 09:58:17 +0200 you wrote:
> From: Yinjun Zhang <yinjun.zhang@corigine.com>
> 
> Avoid reading link modes from management firmware every time when
> `ethtool_get_link_ksettings` is called, only communicate with
> management firmware when necessary like we do for eth_table info.
> 
> This change can ease the situation that when large number of vlan
> sub-interfaces are created and their information is requested by
> some monitoring process like PCP [1] through ethool ioctl frequently.
> 
> [...]

Here is the summary with links:
  - [net-next] nfp: improve link modes reading process
    https://git.kernel.org/netdev/net-next/c/a731a43e8669

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html