diff mbox

[net-next,05/10] be2net: don't enable PAUSE by default

Message ID 1430722042-26449-6-git-send-email-sathya.perla@emulex.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Sathya Perla May 4, 2015, 6:47 a.m. UTC
From: Kalesh AP <kalesh.purayil@emulex.com>

The driver so far, on load, always tried to enable RX/TX general pause.
This setting always fails on certain profiles (like FCoE) where PFC is
needed. Such failures cannot be effectively communicated to the stack or
user. To fix this unpredictability in state, this patch changes the driver
behavior by not configuring pause at load time.
The user can configure it via ethtool and if the setting is disallowed,
an appropriate error is returned.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_ethtool.c |  5 ++++-
 drivers/net/ethernet/emulex/benet/be_main.c    | 12 ------------
 2 files changed, 4 insertions(+), 13 deletions(-)

Comments

David Miller May 4, 2015, 3:59 p.m. UTC | #1
From: Sathya Perla <sathya.perla@emulex.com>
Date: Mon,  4 May 2015 02:47:17 -0400

> From: Kalesh AP <kalesh.purayil@emulex.com>
> 
> The driver so far, on load, always tried to enable RX/TX general pause.
> This setting always fails on certain profiles (like FCoE) where PFC is
> needed. Such failures cannot be effectively communicated to the stack or
> user. To fix this unpredictability in state, this patch changes the driver
> behavior by not configuring pause at load time.
> The user can configure it via ethtool and if the setting is disallowed,
> an appropriate error is returned.
> 
> Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Not enabling pause by default is going to be _STRONGLY_ not expected
by the user and will cause unwanted packet drops in high traffic
environments.

It's not the user's fault that your stuff is architected in such a way
such that handling this properly isn't easy.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sathya Perla May 5, 2015, 5:35 a.m. UTC | #2
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> 
> From: Sathya Perla <sathya.perla@emulex.com>
> Date: Mon,  4 May 2015 02:47:17 -0400
> 
> > From: Kalesh AP <kalesh.purayil@emulex.com>
> >
> > The driver so far, on load, always tried to enable RX/TX general pause.
> > This setting always fails on certain profiles (like FCoE) where PFC is
> > needed. Such failures cannot be effectively communicated to the stack or
> > user. To fix this unpredictability in state, this patch changes the driver
> > behavior by not configuring pause at load time.
> > The user can configure it via ethtool and if the setting is disallowed,
> > an appropriate error is returned.
...
> 
> Not enabling pause by default is going to be _STRONGLY_ not expected
> by the user and will cause unwanted packet drops in high traffic
> environments.
> 
> It's not the user's fault that your stuff is architected in such a way
> such that handling this properly isn't easy.

Ok, I'll follow up with our FW team about this and in the meantime
drop this patch from this patch-set. Thanks!

-Sathya
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 61321de..8b50021 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -717,7 +717,10 @@  be_set_pauseparam(struct net_device *netdev, struct ethtool_pauseparam *ecmd)
 	status = be_cmd_set_flow_control(adapter, ecmd->tx_pause,
 					 ecmd->rx_pause);
 	if (status) {
-		dev_warn(&adapter->pdev->dev, "Pause param set failed\n");
+		dev_warn(&adapter->pdev->dev,
+			 "Failed to set Ethernet flow control parameters\n");
+		if (base_status(status) == MCC_STATUS_FEATURE_NOT_SUPPORTED)
+			return -EOPNOTSUPP;
 		return be_cmd_status(status);
 	}
 
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index b25c509..66749d0 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4248,15 +4248,6 @@  static int be_setup(struct be_adapter *adapter)
 
 	be_set_rx_mode(adapter->netdev);
 
-	status = be_cmd_set_flow_control(adapter, adapter->tx_fc,
-					 adapter->rx_fc);
-	if (status)
-		be_cmd_get_flow_control(adapter, &adapter->tx_fc,
-					&adapter->rx_fc);
-
-	dev_info(&adapter->pdev->dev, "HW Flow control - TX:%d RX:%d\n",
-		 adapter->tx_fc, adapter->rx_fc);
-
 	if (be_physfn(adapter))
 		be_cmd_set_logical_link_config(adapter,
 					       IFLA_VF_LINK_STATE_AUTO, 0);
@@ -5441,9 +5432,6 @@  static int be_drv_init(struct be_adapter *adapter)
 	INIT_DELAYED_WORK(&adapter->be_err_detection_work,
 			  be_err_detection_task);
 
-	adapter->rx_fc = true;
-	adapter->tx_fc = true;
-
 	/* Must be a power of 2 or else MODULO will BUG_ON */
 	adapter->be_get_temp_freq = 64;