diff mbox series

[net-next,v2] net: mana: Improve mana_set_channels() in low mem conditions

Message ID 1725248734-21760-1-git-send-email-shradhagupta@linux.microsoft.com (mailing list archive)
State Not Applicable
Headers show
Series [net-next,v2] net: mana: Improve mana_set_channels() in low mem conditions | expand

Commit Message

Shradha Gupta Sept. 2, 2024, 3:45 a.m. UTC
The mana_set_channels() function requires detaching the mana
driver and reattaching it with changed channel values.
During this operation if the system is low on memory, the reattach
might fail, causing the network device being down.
To avoid this we pre-allocate buffers at the beginning of set operation,
to prevent complete network loss

Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 Changes in v2
 * Pass num_queues as argument in mana_pre_alloc_rxbufs()
---
 drivers/net/ethernet/microsoft/mana/mana_en.c |  6 ++--
 .../ethernet/microsoft/mana/mana_ethtool.c    | 28 ++++++++++---------
 include/net/mana/mana.h                       |  2 +-
 3 files changed, 19 insertions(+), 17 deletions(-)

Comments

Gerhard Engleder Sept. 4, 2024, 7:05 p.m. UTC | #1
On 02.09.24 05:45, Shradha Gupta wrote:
> The mana_set_channels() function requires detaching the mana
> driver and reattaching it with changed channel values.
> During this operation if the system is low on memory, the reattach
> might fail, causing the network device being down.
> To avoid this we pre-allocate buffers at the beginning of set operation,
> to prevent complete network loss
> 
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> ---
>   Changes in v2
>   * Pass num_queues as argument in mana_pre_alloc_rxbufs()
> ---
>   drivers/net/ethernet/microsoft/mana/mana_en.c |  6 ++--
>   .../ethernet/microsoft/mana/mana_ethtool.c    | 28 ++++++++++---------
>   include/net/mana/mana.h                       |  2 +-
>   3 files changed, 19 insertions(+), 17 deletions(-)

Looks better now with the argument for the queue number.

Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Haiyang Zhang Sept. 4, 2024, 7:33 p.m. UTC | #2
> -----Original Message-----
> From: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Sent: Sunday, September 1, 2024 11:46 PM
> To: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org
> Cc: Shradha Gupta <shradhagupta@linux.microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Wei Liu
> <wei.liu@kernel.org>; Dexuan Cui <decui@microsoft.com>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Long Li
> <longli@microsoft.com>; Simon Horman <horms@kernel.org>; Konstantin
> Taranov <kotaranov@microsoft.com>; Souradeep Chakrabarti
> <schakrabarti@linux.microsoft.com>; Erick Archer
> <erick.archer@outlook.com>; Pavan Chebbi <pavan.chebbi@broadcom.com>;
> Ahmed Zaki <ahmed.zaki@intel.com>; Colin Ian King
> <colin.i.king@gmail.com>; Shradha Gupta <shradhagupta@microsoft.com>
> Subject: [PATCH net-next v2] net: mana: Improve mana_set_channels() in
> low mem conditions
> 
> The mana_set_channels() function requires detaching the mana
> driver and reattaching it with changed channel values.
> During this operation if the system is low on memory, the reattach
> might fail, causing the network device being down.
> To avoid this we pre-allocate buffers at the beginning of set operation,
> to prevent complete network loss
> 
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>

Thanks.
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
patchwork-bot+netdevbpf@kernel.org Sept. 4, 2024, 11:40 p.m. UTC | #3
Hello:

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

On Sun,  1 Sep 2024 20:45:34 -0700 you wrote:
> The mana_set_channels() function requires detaching the mana
> driver and reattaching it with changed channel values.
> During this operation if the system is low on memory, the reattach
> might fail, causing the network device being down.
> To avoid this we pre-allocate buffers at the beginning of set operation,
> to prevent complete network loss
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: mana: Improve mana_set_channels() in low mem conditions
    https://git.kernel.org/netdev/net-next/c/1705341485ff

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 3e865985340e..a174ca719aba 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -608,7 +608,7 @@  static void mana_get_rxbuf_cfg(int mtu, u32 *datasize, u32 *alloc_size,
 	*datasize = mtu + ETH_HLEN;
 }
 
-int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu)
+int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_queues)
 {
 	struct device *dev;
 	struct page *page;
@@ -622,7 +622,7 @@  int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu)
 
 	dev = mpc->ac->gdma_dev->gdma_context->dev;
 
-	num_rxb = mpc->num_queues * mpc->rx_queue_size;
+	num_rxb = num_queues * mpc->rx_queue_size;
 
 	WARN(mpc->rxbufs_pre, "mana rxbufs_pre exists\n");
 	mpc->rxbufs_pre = kmalloc_array(num_rxb, sizeof(void *), GFP_KERNEL);
@@ -682,7 +682,7 @@  static int mana_change_mtu(struct net_device *ndev, int new_mtu)
 	int err;
 
 	/* Pre-allocate buffers to prevent failure in mana_attach later */
-	err = mana_pre_alloc_rxbufs(mpc, new_mtu);
+	err = mana_pre_alloc_rxbufs(mpc, new_mtu, mpc->num_queues);
 	if (err) {
 		netdev_err(ndev, "Insufficient memory for new MTU\n");
 		return err;
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index d6a35fbda447..dc3864377538 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -345,27 +345,29 @@  static int mana_set_channels(struct net_device *ndev,
 	struct mana_port_context *apc = netdev_priv(ndev);
 	unsigned int new_count = channels->combined_count;
 	unsigned int old_count = apc->num_queues;
-	int err, err2;
+	int err;
+
+	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, new_count);
+	if (err) {
+		netdev_err(ndev, "Insufficient memory for new allocations");
+		return err;
+	}
 
 	err = mana_detach(ndev, false);
 	if (err) {
 		netdev_err(ndev, "mana_detach failed: %d\n", err);
-		return err;
+		goto out;
 	}
 
 	apc->num_queues = new_count;
 	err = mana_attach(ndev);
-	if (!err)
-		return 0;
-
-	netdev_err(ndev, "mana_attach failed: %d\n", err);
-
-	/* Try to roll it back to the old configuration. */
-	apc->num_queues = old_count;
-	err2 = mana_attach(ndev);
-	if (err2)
-		netdev_err(ndev, "mana re-attach failed: %d\n", err2);
+	if (err) {
+		apc->num_queues = old_count;
+		netdev_err(ndev, "mana_attach failed: %d\n", err);
+	}
 
+out:
+	mana_pre_dealloc_rxbufs(apc);
 	return err;
 }
 
@@ -414,7 +416,7 @@  static int mana_set_ringparam(struct net_device *ndev,
 
 	/* pre-allocating new buffers to prevent failures in mana_attach() later */
 	apc->rx_queue_size = new_rx;
-	err = mana_pre_alloc_rxbufs(apc, ndev->mtu);
+	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
 	apc->rx_queue_size = old_rx;
 	if (err) {
 		netdev_err(ndev, "Insufficient memory for new allocations\n");
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 1f869624811d..dfbf78d4e557 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -488,7 +488,7 @@  struct bpf_prog *mana_xdp_get(struct mana_port_context *apc);
 void mana_chn_setxdp(struct mana_port_context *apc, struct bpf_prog *prog);
 int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
 void mana_query_gf_stats(struct mana_port_context *apc);
-int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu);
+int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu, int num_queues);
 void mana_pre_dealloc_rxbufs(struct mana_port_context *apc);
 
 extern const struct ethtool_ops mana_ethtool_ops;