diff mbox

[1/1] bonding: delay up state without speed and duplex in 802.3ad mode

Message ID 1450413386-8867-2-git-send-email-zyjzyj2000@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Zhu Yanjun Dec. 18, 2015, 4:36 a.m. UTC
From: yzhu1 <yzhu1@windriver.com>

In 802.3ad mode, the speed and duplex is needed. But in some NICs,
there is a time span between NIC up state and getting speed and duplex.
As such, sometimes a slave in 802.3ad mode is in up state without
speed and duplex. This will make bonding in 802.3ad mode can not
work well.

To make bonding driver robust and compatible with more NICs, it is
necessary to delay the up state without speed and duplex in 802.3ad
mode.

Signed-off-by: yzhu1 <yzhu1@windriver.com>
---
 drivers/net/bonding/bond_main.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Jay Vosburgh Dec. 18, 2015, 4:54 a.m. UTC | #1
<zyjzyj2000@gmail.com> wrote:

>From: yzhu1 <yzhu1@windriver.com>
>
>In 802.3ad mode, the speed and duplex is needed. But in some NICs,
>there is a time span between NIC up state and getting speed and duplex.
>As such, sometimes a slave in 802.3ad mode is in up state without
>speed and duplex. This will make bonding in 802.3ad mode can not
>work well.
>
>To make bonding driver robust and compatible with more NICs, it is
>necessary to delay the up state without speed and duplex in 802.3ad
>mode.

	You misunderstood my comment.  What I meant is that the device
driver for the network device should change to either delay carrier up
or issue a second notifier when speed and duplex are available.  If the
driver doesn't handle duplex and speed notification properly, it will
likely have trouble with more than just bonding.

	You also didn't mention the identity of the network device that
requires this special handling.  Is the driver part of the linux kernel?

	-J

>Signed-off-by: yzhu1 <yzhu1@windriver.com>
>---
> drivers/net/bonding/bond_main.c |   34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 9e0f8a7..a1d8708 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -419,6 +419,35 @@ const char *bond_slave_link_status(s8 link)
> 	}
> }
> 
>+/* This function is to check the speed and duplex of a NIC.
>+ * Since the speed and duplex of a slave device are very
>+ * important to the bonding in the 802.3ad mode. As such,
>+ * it is necessary to check the speed and duplex of a slave
>+ * device in 802.3ad mode.
>+ *
>+ * speed != SPEED_UNKNOWN and duplex == DUPLEX_FULL  :  1
>+ *                                           others  :  0
>+ */
>+static int __check_speed_duplex(struct net_device *netdev)
>+{
>+	struct ethtool_cmd ecmd;
>+	u32 slave_speed = SPEED_UNKNOWN;
>+	int res;
>+
>+	res = __ethtool_get_settings(netdev, &ecmd);
>+	if (res < 0)
>+		return 0;
>+
>+	slave_speed = ethtool_cmd_speed(&ecmd);
>+	if (slave_speed == 0 || slave_speed == ((__u32) -1))
>+		return 0;
>+
>+	if (DUPLEX_FULL != ecmd.duplex)
>+		return 0;
>+
>+	return 1;
>+}
>+
> /* if <dev> supports MII link status reporting, check its link status.
>  *
>  * We either do MII/ETHTOOL ioctls, or check netif_carrier_ok(),
>@@ -445,6 +474,11 @@ static int bond_check_dev_link(struct bonding *bond,
> 	if (!reporting && !netif_running(slave_dev))
> 		return 0;
> 
>+	/* Check the speed and duplex of the slave device in 802.3ad mode. */
>+	if ((BOND_MODE(bond) == BOND_MODE_8023AD) &&
>+	   !__check_speed_duplex(slave_dev))
>+		return 0;
>+
> 	if (bond->params.use_carrier)
> 		return netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0;
> 
>-- 
>1.7.9.5
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com
--
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
Sergei Shtylyov Dec. 18, 2015, 1:37 p.m. UTC | #2
Hello.

On 12/18/2015 7:36 AM, zyjzyj2000@gmail.com wrote:

> From: yzhu1 <yzhu1@windriver.com>
>
> In 802.3ad mode, the speed and duplex is needed. But in some NICs,
> there is a time span between NIC up state and getting speed and duplex.
> As such, sometimes a slave in 802.3ad mode is in up state without
> speed and duplex. This will make bonding in 802.3ad mode can not
> work well.
>
> To make bonding driver robust and compatible with more NICs, it is
> necessary to delay the up state without speed and duplex in 802.3ad
> mode.
>
> Signed-off-by: yzhu1 <yzhu1@windriver.com>
> ---
>   drivers/net/bonding/bond_main.c |   34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 9e0f8a7..a1d8708 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -419,6 +419,35 @@ const char *bond_slave_link_status(s8 link)
>   	}
>   }
>
> +/* This function is to check the speed and duplex of a NIC.
> + * Since the speed and duplex of a slave device are very
> + * important to the bonding in the 802.3ad mode. As such,
> + * it is necessary to check the speed and duplex of a slave
> + * device in 802.3ad mode.
> + *
> + * speed != SPEED_UNKNOWN and duplex == DUPLEX_FULL  :  1
> + *                                           others  :  0
> + */
> +static int __check_speed_duplex(struct net_device *netdev)
> +{
> +	struct ethtool_cmd ecmd;
> +	u32 slave_speed = SPEED_UNKNOWN;
> +	int res;
> +
> +	res = __ethtool_get_settings(netdev, &ecmd);
> +	if (res < 0)
> +		return 0;
> +
> +	slave_speed = ethtool_cmd_speed(&ecmd);
> +	if (slave_speed == 0 || slave_speed == ((__u32) -1))
> +		return 0;
> +
> +	if (DUPLEX_FULL != ecmd.duplex)

    Please place the immediate operand to the right of the != operator.

[...]

MBR, Sergei

--
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/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9e0f8a7..a1d8708 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -419,6 +419,35 @@  const char *bond_slave_link_status(s8 link)
 	}
 }
 
+/* This function is to check the speed and duplex of a NIC.
+ * Since the speed and duplex of a slave device are very
+ * important to the bonding in the 802.3ad mode. As such,
+ * it is necessary to check the speed and duplex of a slave
+ * device in 802.3ad mode.
+ *
+ * speed != SPEED_UNKNOWN and duplex == DUPLEX_FULL  :  1
+ *                                           others  :  0
+ */
+static int __check_speed_duplex(struct net_device *netdev)
+{
+	struct ethtool_cmd ecmd;
+	u32 slave_speed = SPEED_UNKNOWN;
+	int res;
+
+	res = __ethtool_get_settings(netdev, &ecmd);
+	if (res < 0)
+		return 0;
+
+	slave_speed = ethtool_cmd_speed(&ecmd);
+	if (slave_speed == 0 || slave_speed == ((__u32) -1))
+		return 0;
+
+	if (DUPLEX_FULL != ecmd.duplex)
+		return 0;
+
+	return 1;
+}
+
 /* if <dev> supports MII link status reporting, check its link status.
  *
  * We either do MII/ETHTOOL ioctls, or check netif_carrier_ok(),
@@ -445,6 +474,11 @@  static int bond_check_dev_link(struct bonding *bond,
 	if (!reporting && !netif_running(slave_dev))
 		return 0;
 
+	/* Check the speed and duplex of the slave device in 802.3ad mode. */
+	if ((BOND_MODE(bond) == BOND_MODE_8023AD) &&
+	   !__check_speed_duplex(slave_dev))
+		return 0;
+
 	if (bond->params.use_carrier)
 		return netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0;