Patchwork [net-26,1/5] cxgb4vf: Virtual Interfaces are always up ...

login
register
mail settings
Submitter Casey Leedom
Date Feb. 12, 2011, 1 a.m.
Message ID <1297472423-15672-2-git-send-email-leedom@chelsio.com>
Download mbox | patch
Permalink /patch/82882/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Casey Leedom - Feb. 12, 2011, 1 a.m.
Implement new default mode of always reporting the Virtual Interface link as
being "up".  This allows different Virtual Interfaces on the same port to
continue to communicate with each other even when the physical port link is
down.  This new behavior is controlled via the module parameter
force_link_up (default 1).  The old behavior can be achieved by setting
force_link_up=0.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
 drivers/net/cxgb4vf/cxgb4vf_main.c |   30 +++++++++++++++++++++++++++---
 1 files changed, 27 insertions(+), 3 deletions(-)
David Miller - Feb. 12, 2011, 5:19 a.m.
From: Casey Leedom <leedom@chelsio.com>
Date: Fri, 11 Feb 2011 17:00:19 -0800

> Implement new default mode of always reporting the Virtual Interface link as
> being "up".  This allows different Virtual Interfaces on the same port to
> continue to communicate with each other even when the physical port link is
> down.  This new behavior is controlled via the module parameter
> force_link_up (default 1).  The old behavior can be achieved by setting
> force_link_up=0.
> 
> Signed-off-by: Casey Leedom <leedom@chelsio.com>

No driver specific module parameters!  Add something generic and common
so other drivers can use it too.

Otherwise every user has to learn a different way to control this
attribute, depending upon the device type, which is rediculious.

How many times do we have to tell driver authors this?
--
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
Casey Leedom - Feb. 14, 2011, 7:13 p.m.
| From: David Miller <davem@davemloft.net>
| Date: Friday, February 11, 2011 09:19 pm
| 
| From: Casey Leedom <leedom@chelsio.com>
| Date: Fri, 11 Feb 2011 17:00:19 -0800
| 
| > Implement new default mode of always reporting the Virtual Interface link
| > as being "up".  This allows different Virtual Interfaces on the same
| > port to continue to communicate with each other even when the physical
| > port link is down.  This new behavior is controlled via the module
| > parameter
| > force_link_up (default 1).  The old behavior can be achieved by setting
| > force_link_up=0.
| > 
| > Signed-off-by: Casey Leedom <leedom@chelsio.com>
| 
| No driver specific module parameters!  Add something generic and common
| so other drivers can use it too.
| 
| Otherwise every user has to learn a different way to control this
| attribute, depending upon the device type, which is rediculious.
| 
| How many times do we have to tell driver authors this?

  Sorry.  I wasn't aware of this rule.  My bad.  Is this writeen down somewhere 
under Documentation?  I'm not being snarky.  I really would like to know so I 
can read through the general ground rules and avoid making more mistakes in the 
future.

  As for a generic mechanism, what's the preferred way of doing this?  A new 
ethtool flag?  Sorry for being a doofus here, I'm happy to follow whatever the 
accepted standard is.  Thanks for your time and patience.

Casey
--
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
Casey Leedom - Feb. 14, 2011, 10:31 p.m.
| From: Casey Leedom <leedom@chelsio.com>
| Date: Monday, February 14, 2011 11:13 am
| 
| | No driver specific module parameters!  Add something generic and common
| | so other drivers can use it too.
| | 
| | Otherwise every user has to learn a different way to control this
| | attribute, depending upon the device type, which is rediculious.
| | 
| | How many times do we have to tell driver authors this?
| 
|   Sorry.  I wasn't aware of this rule.  My bad.  Is this writeen down
| somewhere under Documentation?  I'm not being snarky.  I really would like
| to know so I can read through the general ground rules and avoid making
| more mistakes in the future.
| 
|   As for a generic mechanism, what's the preferred way of doing this?  A
| new ethtool flag?  Sorry for being a doofus here, I'm happy to follow
| whatever the accepted standard is.  Thanks for your time and patience.

  Also, I assume then the the entire patch series is now rejected, right?  And 
that I should resubmit the patch series without the unacceptable module 
parameter, right?  I'm just trying to figure out what I need to do next.  
Thanks.

Casey
--
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
David Miller - Feb. 14, 2011, 10:34 p.m.
From: Casey Leedom <leedom@chelsio.com>
Date: Mon, 14 Feb 2011 14:31:32 -0800

> | From: Casey Leedom <leedom@chelsio.com>
> | Date: Monday, February 14, 2011 11:13 am
> | 
> | | No driver specific module parameters!  Add something generic and common
> | | so other drivers can use it too.
> | | 
> | | Otherwise every user has to learn a different way to control this
> | | attribute, depending upon the device type, which is rediculious.
> | | 
> | | How many times do we have to tell driver authors this?
> | 
> |   Sorry.  I wasn't aware of this rule.  My bad.  Is this writeen down
> | somewhere under Documentation?  I'm not being snarky.  I really would like
> | to know so I can read through the general ground rules and avoid making
> | more mistakes in the future.
> | 
> |   As for a generic mechanism, what's the preferred way of doing this?  A
> | new ethtool flag?  Sorry for being a doofus here, I'm happy to follow
> | whatever the accepted standard is.  Thanks for your time and patience.
> 
>   Also, I assume then the the entire patch series is now rejected, right?  And 
> that I should resubmit the patch series without the unacceptable module 
> parameter, right?  I'm just trying to figure out what I need to do next.  

You need to resubmit the whole series, because changing an earlier
patch causes the subsequent ones to have, at a minimum, offsets which
GIT apply will reject.
--
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

Patch

diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index 56166ae..08c2b29 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -96,6 +96,18 @@  module_param(msi, int, 0644);
 MODULE_PARM_DESC(msi, "whether to use MSI-X or MSI");
 
 /*
+ * The Virtual Interfaces are connected to an internal switch on the chip
+ * which allows VIs attached to the same port to talk to each other even when
+ * the port link is down.  As a result, we generally want to always report a
+ * VI's link as being "up".
+ */
+static int force_link_up = 1;
+
+module_param(force_link_up, int, 0644);
+MODULE_PARM_DESC(force_link_up, "always report link up");
+
+
+/*
  * Fundamental constants.
  * ======================
  */
@@ -151,7 +163,8 @@  void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
 		return;
 
 	/*
-	 * Tell the OS that the link status has changed and print a short
+	 * Tell the OS that the link status has changed (if we're not
+	 * operating in the forced link "up" mode) and print a short
 	 * informative message on the console about the event.
 	 */
 	if (link_ok) {
@@ -159,7 +172,8 @@  void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
 		const char *fc;
 		const struct port_info *pi = netdev_priv(dev);
 
-		netif_carrier_on(dev);
+		if (!force_link_up)
+			netif_carrier_on(dev);
 
 		switch (pi->link_cfg.speed) {
 		case SPEED_10000:
@@ -200,7 +214,9 @@  void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
 		printk(KERN_INFO "%s: link up, %s, full-duplex, %s PAUSE\n",
 		       dev->name, s, fc);
 	} else {
-		netif_carrier_off(dev);
+		if (!force_link_up)
+			netif_carrier_off(dev);
+
 		printk(KERN_INFO "%s: link down\n", dev->name);
 	}
 }
@@ -254,6 +270,14 @@  static int link_start(struct net_device *dev)
 	 */
 	if (ret == 0)
 		ret = t4vf_enable_vi(pi->adapter, pi->viid, true, true);
+
+	/*
+	 * If we didn't experience any error and we're always reporting the
+	 * link as being "up", tell the OS that the link is up.
+	 */
+	if (ret == 0 && force_link_up)
+		netif_carrier_on(dev);
+
 	return ret;
 }