| Message ID | 1450339417-31254-1-git-send-email-zyjzyj2000@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
<zyjzyj2000@gmail.com> wrote: >From: Zhu Yanjun <zyjzyj2000@gmail.com> > >In 802.3ad mode, the speed and duplex is needed. But in some NIC, >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 be compatible with more NICs, it is >necessary to restrict the up state in 802.3ad mode. What device is this? It seems a bit odd that an Ethernet device can be carrier up but not have the duplex and speed available. Also, what are the option settings for bonding? Specifically, is "use_carrier" set to 0? The default setting is 1. In general, though, bonding expects a speed or duplex change to be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would propagate to the 802.3ad logic. If the device here is going carrier up prior to having speed or duplex available, then maybe it should call netdev_state_change() when the duplex and speed are available, or delay calling netif_carrier_on(). >Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >--- > drivers/net/bonding/bond_main.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 9e0f8a7..0a80fb3 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1991,6 +1991,25 @@ static int bond_miimon_inspect(struct bonding *bond) > > link_state = bond_check_dev_link(bond, slave->dev, 0); > >+ /* Since some NIC has time span between netif_running and >+ * getting speed and duples. That is, after a NIC is up (netif_running), >+ * there is a time span before this NIC is negotiated with speed and duplex. >+ * During this time span, the slave in 802.3ad is configured without speed >+ * and duplex. This 802.3ad bonding will not work because it needs slave's speed >+ * and duplex to generate key field. >+ * As such, we restrict up in 802.3ad mode to: netif_running && peed != SPEED_UNKNOWN && >+ * duplex != DUPLEX_UNKNOWN >+ */ >+ if ((BMSR_LSTATUS == link_state) && >+ (BOND_MODE(bond) == BOND_MODE_8023AD)) { >+ bond_update_speed_duplex(slave); >+ if ((slave->speed == SPEED_UNKNOWN) || >+ (slave->duplex == DUPLEX_UNKNOWN)) { >+ link_state = 0; >+ netdev_info(bond->dev, "In 802.3ad mode, it is not enough to up without speed and duplex"); >+ } >+ } Also, as a functional note on this patch, the above looks like it will spam the log repeatedly every miimon interval for as long as the "carrier up but no speed/duplex" situation persists. -J > switch (slave->link) { > case BOND_LINK_UP: > if (link_state) >-- >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
Hi, Jay Thanks for your reply. Yes. The NIC is a bit odd. We have to be compatible with it. I followed your advice to delay calling netif_carrier_on(). Changes: Delay calling netif_carrier_on(). Best Regards! Zhu Yanjun -- 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
On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: > <zyjzyj2000@gmail.com> wrote: > >In 802.3ad mode, the speed and duplex is needed. But in some NIC, > >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 be compatible with more NICs, it is > >necessary to restrict the up state in 802.3ad mode. > > What device is this? It seems a bit odd that an Ethernet device > can be carrier up but not have the duplex and speed available. ... > In general, though, bonding expects a speed or duplex change to > be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would > propagate to the 802.3ad logic. > > If the device here is going carrier up prior to having speed or > duplex available, then maybe it should call netdev_state_change() when > the duplex and speed are available, or delay calling netif_carrier_on(). I have encountered this problem (NIC having carrier on before being able to detect speed/duplex and driver not notifying when speed/duplex becomes available) with netxen cards earlier. But it was eventually fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event handling.") so this example rather supports what you said. Michal Kubecek -- 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
On 12/28/2015 04:43 PM, Michal Kubecek wrote: > On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >> <zyjzyj2000@gmail.com> wrote: >>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>> 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 be compatible with more NICs, it is >>> necessary to restrict the up state in 802.3ad mode. >> What device is this? It seems a bit odd that an Ethernet device >> can be carrier up but not have the duplex and speed available. > ... >> In general, though, bonding expects a speed or duplex change to >> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >> propagate to the 802.3ad logic. >> >> If the device here is going carrier up prior to having speed or >> duplex available, then maybe it should call netdev_state_change() when >> the duplex and speed are available, or delay calling netif_carrier_on(). > I have encountered this problem (NIC having carrier on before being able > to detect speed/duplex and driver not notifying when speed/duplex > becomes available) with netxen cards earlier. But it was eventually > fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event > handling.") so this example rather supports what you said. > > Michal Kubecek Thanks a lot. I checked the commit 9d01412ae76f ("netxen: Fix link event handling."). The symptoms are the same with mine. The root cause is different. In my problem, the root cause is that LINKS register can not provide link_up and link_speed at the same time. There is a time span between link_up and link_speed. My solution is to force to synchronize link_up and link_speed in ixgbe X540 NIC. Best Regards! Zhu Yanjun -- 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
>-----Original Message----- >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On >Behalf Of zhuyj >Sent: Monday, December 28, 2015 1:19 AM >To: Michal Kubecek; Jay Vosburgh >Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org; >Shteinbock, Boris (Wind River) >Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode > >On 12/28/2015 04:43 PM, Michal Kubecek wrote: >> On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >>> <zyjzyj2000@gmail.com> wrote: >>>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>>> 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 be compatible with more NICs, it is >>>> necessary to restrict the up state in 802.3ad mode. >>> What device is this? It seems a bit odd that an Ethernet device >>> can be carrier up but not have the duplex and speed available. >> ... >>> In general, though, bonding expects a speed or duplex change to >>> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >>> propagate to the 802.3ad logic. >>> >>> If the device here is going carrier up prior to having speed or >>> duplex available, then maybe it should call netdev_state_change() when >>> the duplex and speed are available, or delay calling netif_carrier_on(). >> I have encountered this problem (NIC having carrier on before being able >> to detect speed/duplex and driver not notifying when speed/duplex >> becomes available) with netxen cards earlier. But it was eventually >> fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event >> handling.") so this example rather supports what you said. >> >> Michal Kubecek >Thanks a lot. >I checked the commit 9d01412ae76f ("netxen: Fix link event >handling."). The symptoms are the same with mine. > >The root cause is different. In my problem, the root cause is that LINKS >register[] can not provide link_up and link_speed at the same time. >There is a time span between link_up and link_speed. The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are updated simultaneously. Do you have any proof to show the delay you are referring to as I am sure our HW engineers would like to know about it. What we have seen in the case of bonding is that with some link partners there may be a rapid link flap (up, down, up) and as result the bonding driver may report the speed as unknown if just so happens that the speed is checked during the period in which the interface is re-negotiating. Thanks, Emil >My solution is to force to synchronize link_up and link_speed in ixgbe X540 NIC. > >Best Regards! >Zhu Yanjun >-- >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 -- 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
On 01/06/2016 09:26 AM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On >> Behalf Of zhuyj >> Sent: Monday, December 28, 2015 1:19 AM >> To: Michal Kubecek; Jay Vosburgh >> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org; >> Shteinbock, Boris (Wind River) >> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >> >> On 12/28/2015 04:43 PM, Michal Kubecek wrote: >>> On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >>>> <zyjzyj2000@gmail.com> wrote: >>>>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>>>> 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 be compatible with more NICs, it is >>>>> necessary to restrict the up state in 802.3ad mode. >>>> What device is this? It seems a bit odd that an Ethernet device >>>> can be carrier up but not have the duplex and speed available. >>> ... >>>> In general, though, bonding expects a speed or duplex change to >>>> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >>>> propagate to the 802.3ad logic. >>>> >>>> If the device here is going carrier up prior to having speed or >>>> duplex available, then maybe it should call netdev_state_change() when >>>> the duplex and speed are available, or delay calling netif_carrier_on(). >>> I have encountered this problem (NIC having carrier on before being able >>> to detect speed/duplex and driver not notifying when speed/duplex >>> becomes available) with netxen cards earlier. But it was eventually >>> fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event >>> handling.") so this example rather supports what you said. >>> >>> Michal Kubecek >> Thanks a lot. >> I checked the commit 9d01412ae76f ("netxen: Fix link event >> handling."). The symptoms are the same with mine. >> >> The root cause is different. In my problem, the root cause is that LINKS >> register[] can not provide link_up and link_speed at the same time. >> There is a time span between link_up and link_speed. > The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are updated > simultaneously. Do you have any proof to show the delay you are referring to > as I am sure our HW engineers would like to know about it. Sorry. I can not reproduce this problem locally. What I have is the feedback from the customer. Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: Unknown! Duplex: Unknown! (255) Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: no Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: Unknown! Duplex: Unknown! (255) Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: yes Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: Unknown! Duplex: Unknown! (255) Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: yes Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: Unknown! Duplex: Unknown! (255) Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: yes Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: Unknown! Duplex: Unknown! (255) Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: yes Settings for eth0: Supported ports: [ TP ] Supported link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Supported pause frame use: No Supports auto-negotiation: Yes Advertised link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised pause frame use: No Advertised auto-negotiation: Yes Speed: 10000Mb/s Duplex: Full Port: Twisted Pair PHYAD: 0 Transceiver: external Auto-negotiation: on MDI-X: Unknown Supports Wake-on: d Wake-on: d Current message level: 0x00000007 (7) drv probe link Link detected: yes I think the time span between link_up and link_speed lasts several seconds. From this function /** * ixgbe_service_timer - Timer Call-back * @data: pointer to adapter cast into an unsigned long **/ static void ixgbe_service_timer(unsigned long data) { struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)data; unsigned long next_event_offset; /* poll faster when waiting for link */ if (adapter->flags & IXGBE_FLAG_NEED_LINK_UPDATE) next_event_offset = HZ / 10; else next_event_offset = HZ * 2; /* Reset the timer */ mod_timer(&adapter->service_timer, next_event_offset + jiffies); ixgbe_service_event_schedule(adapter); } The timer will check link state every 100ms. In this several seconds, the link state is updated for about several dozens of times. > > What we have seen in the case of bonding is that with some link partners there > may be a rapid link flap (up, down, up) and as result the bonding driver may > report the speed as unknown if just so happens that the speed is checked during > the period in which the interface is re-negotiating. Sure. What we have done is to avoid link_up without link_speed. Unless both link_up and link_speed are ready, the bonding driver will not be triggered to check both link_up and link_speed in 802.3ad mode. Thanks a lot. Zhu Yanjun > Thanks, > Emil > >> My solution is to force to synchronize link_up and link_speed in ixgbe X540 NIC. >> >> Best Regards! >> Zhu Yanjun >> -- >> 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 -- 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
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Tuesday, January 05, 2016 7:05 PM >To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh >Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org; >Shteinbock, Boris (Wind River) >Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode > >On 01/06/2016 09:26 AM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] >On >>> Behalf Of zhuyj >>> Sent: Monday, December 28, 2015 1:19 AM >>> To: Michal Kubecek; Jay Vosburgh >>> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; >netdev@vger.kernel.org; >>> Shteinbock, Boris (Wind River) >>> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >>> >>> On 12/28/2015 04:43 PM, Michal Kubecek wrote: >>>> On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >>>>> <zyjzyj2000@gmail.com> wrote: >>>>>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>>>>> 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 be compatible with more NICs, it is >>>>>> necessary to restrict the up state in 802.3ad mode. >>>>> What device is this? It seems a bit odd that an Ethernet device >>>>> can be carrier up but not have the duplex and speed available. >>>> ... >>>>> In general, though, bonding expects a speed or duplex change to >>>>> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >>>>> propagate to the 802.3ad logic. >>>>> >>>>> If the device here is going carrier up prior to having speed or >>>>> duplex available, then maybe it should call netdev_state_change() when >>>>> the duplex and speed are available, or delay calling >netif_carrier_on(). >>>> I have encountered this problem (NIC having carrier on before being >able >>>> to detect speed/duplex and driver not notifying when speed/duplex >>>> becomes available) with netxen cards earlier. But it was eventually >>>> fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event >>>> handling.") so this example rather supports what you said. >>>> >>>> Michal >Kubecek >>> Thanks a lot. >>> I checked the commit 9d01412ae76f ("netxen: Fix link event >>> handling."). The symptoms are the same with mine. >>> >>> The root cause is different. In my problem, the root cause is that LINKS >>> register[] can not provide link_up and link_speed at the same time. >>> There is a time span between link_up and link_speed. >> The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are >updated >> simultaneously. Do you have any proof to show the delay you are referring >to >> as I am sure our HW engineers would like to know about it. >Sorry. I can not reproduce this problem locally. What I have is the >feedback from the customer. So you are assuming that there is a delay due to the issue you are seeing? >Settings for eth0: > Supported ports: [ TP ] > Supported link modes: 100baseT/Full > 1000baseT/Full > 10000baseT/Full > Supported pause frame use: No > Supports auto-negotiation: Yes > Advertised link modes: 100baseT/Full > 1000baseT/Full > 10000baseT/Full > Advertised pause frame use: No > Advertised auto-negotiation: Yes > Speed: Unknown! > Duplex: Unknown! (255) > Port: Twisted Pair > PHYAD: 0 > Transceiver: external > Auto-negotiation: on > MDI-X: Unknown > Supports Wake-on: d > Wake-on: d > Current message level: 0x00000007 (7) > drv probe link > Link detected: yes The speed and the link state here are reported from different sources: > Link detected: yes Comes from a netif_carrier_ok() check. This is done via ethtool_op_get_link(). Only the speed is reported through the LINKS register - that is why it is reported as "Unknown" - in other words link_up is false. This is a trace from the case where the bonding driver reports 0 Mbps: kworker/u48:1-27950 [010] .... 6493.084916: ixgbe_service_task: eth1: link_speed = 80, link_up = false kworker/u48:1-27950 [011] .... 6493.184894: ixgbe_service_task: eth1: link_speed = 80, link_up = false kworker/u48:1-27950 [000] .... 6494.439883: ixgbe_service_task: eth1: link_speed = 80, link_up = true kworker/u48:1-27950 [000] .... 6494.464204: ixgbe_service_task: eth1: NIC Link is Up 10 Gbps, Flow Control: RX/TX kworker/0:2-1926 [000] .... 6494.464249: ixgbe_get_settings: eth1: link_speed = 80, link_up = false NetworkManager-3819 [008] .... 6494.464484: ixgbe_get_settings: eth1: link_speed = 80, link_up = false kworker/u48:1-27950 [007] .... 6494.496886: bond_mii_monitor: bond0: link status definitely up for interface eth1, 0 Mbps full duplex NetworkManager-3819 [008] .... 6494.496967: ixgbe_get_settings: eth1: link_speed = 80, link_up = false kworker/u48:1-27950 [008] .... 6495.288798: ixgbe_service_task: eth1: link_speed = 80, link_up = true kworker/u48:1-27950 [008] .... 6495.388806: ixgbe_service_task: eth1: link_speed = 80, link_up = true As you can see the link is initially established, but then lost and if just so happens that the bonding driver is checking it at that time it will report 0 Mbps. I will give your patch a try and see if it helps in this situation. Thanks, Emil -- 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
On 01/07/2016 10:43 AM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Tuesday, January 05, 2016 7:05 PM >> To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh >> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org; >> Shteinbock, Boris (Wind River) >> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >> >> On 01/06/2016 09:26 AM, Tantilov, Emil S wrote: >>>> -----Original Message----- >>>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] >> On >>>> Behalf Of zhuyj >>>> Sent: Monday, December 28, 2015 1:19 AM >>>> To: Michal Kubecek; Jay Vosburgh >>>> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; >> netdev@vger.kernel.org; >>>> Shteinbock, Boris (Wind River) >>>> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >>>> >>>> On 12/28/2015 04:43 PM, Michal Kubecek wrote: >>>>> On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >>>>>> <zyjzyj2000@gmail.com> wrote: >>>>>>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>>>>>> 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 be compatible with more NICs, it is >>>>>>> necessary to restrict the up state in 802.3ad mode. >>>>>> What device is this? It seems a bit odd that an Ethernet device >>>>>> can be carrier up but not have the duplex and speed available. >>>>> ... >>>>>> In general, though, bonding expects a speed or duplex change to >>>>>> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >>>>>> propagate to the 802.3ad logic. >>>>>> >>>>>> If the device here is going carrier up prior to having speed or >>>>>> duplex available, then maybe it should call netdev_state_change() when >>>>>> the duplex and speed are available, or delay calling >> netif_carrier_on(). >>>>> I have encountered this problem (NIC having carrier on before being >> able >>>>> to detect speed/duplex and driver not notifying when speed/duplex >>>>> becomes available) with netxen cards earlier. But it was eventually >>>>> fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event >>>>> handling.") so this example rather supports what you said. >>>>> >>>>> Michal >> Kubecek >>>> Thanks a lot. >>>> I checked the commit 9d01412ae76f ("netxen: Fix link event >>>> handling."). The symptoms are the same with mine. >>>> >>>> The root cause is different. In my problem, the root cause is that LINKS >>>> register[] can not provide link_up and link_speed at the same time. >>>> There is a time span between link_up and link_speed. >>> The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are >> updated >>> simultaneously. Do you have any proof to show the delay you are referring >> to >>> as I am sure our HW engineers would like to know about it. >> Sorry. I can not reproduce this problem locally. What I have is the >> feedback from the customer. > So you are assuming that there is a delay due to the issue you are seeing? Sure. Before I get the further feedback from the customer, I can not make further conclusion. My patch is based on the feedback from the customer. > >> Settings for eth0: >> Supported ports: [ TP ] >> Supported link modes: 100baseT/Full >> 1000baseT/Full >> 10000baseT/Full >> Supported pause frame use: No >> Supports auto-negotiation: Yes >> Advertised link modes: 100baseT/Full >> 1000baseT/Full >> 10000baseT/Full >> Advertised pause frame use: No >> Advertised auto-negotiation: Yes >> Speed: Unknown! >> Duplex: Unknown! (255) >> Port: Twisted Pair >> PHYAD: 0 >> Transceiver: external >> Auto-negotiation: on >> MDI-X: Unknown >> Supports Wake-on: d >> Wake-on: d >> Current message level: 0x00000007 (7) >> drv probe link >> Link detected: yes > The speed and the link state here are reported from > different sources: Sure. ixgbe_get_settings->hw->mac.ops.check_link(X540)->ixgbe_check_mac_link_generic In this function ixgbe_check_mac_link_generic, the register IXGBE_LINKS is checked. link_up and link_speed is got from this register. > >> Link detected: yes > Comes from a netif_carrier_ok() check. This is done via ethtool_op_get_link() > > Only the speed is reported through the LINKS register - that is why it is reported > as "Unknown" - in other words link_up is false. Sorry. I do not agree with you. static inline bool netif_carrier_ok(const struct net_device *dev) { return !test_bit(__LINK_STATE_NOCARRIER, &dev->state); } netif_carrier_ok will check __LINK_STATE_NOCARRIER. This __LINK_STATE_NOCARRIER is set by netif_carrier_on. /** * netif_carrier_on - set carrier * @dev: network device * * Device has detected that carrier. */ void netif_carrier_on(struct net_device *dev) { if (test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state)) { if (dev->reg_state == NETREG_UNINITIALIZED) return; atomic_inc(&dev->carrier_changes); linkwatch_fire_event(dev); if (netif_running(dev)) __netdev_watchdog_up(dev); } } In ixgbe driver, in ixgbe_main.c +6506, this function ixgbe_watchdog_link_is_up runs netif_carrier_on function. ixgbe_watchdog_link_is_up is in service_task. If IXGBE_FLAG_NEED_LINK_UPDATE is set in adapter->flags, the function ixgbe_watchdog_link_is_up will run every 100ms. IXGBE_FLAG_NEED_LINK_UPDATE is set in ixgbe_check_lsc in x540. This function ixgbe_check_lsc is in irq handler. link_up will trigger it. As such, link_up will trriger ixgbe_check_lsc to set IXGBE_FLAG_NEED_LINK_UPDATE in adapter->flags. In the end, service_task will check the register IXGBE_LINKS every 100ms. So ixgbe_get_settings and netif_carrier_ok travel different paths to the function ixgbe_check_mac_link_generic. And the time span between ixgbe_get_settings and netif_carrier_ok is very tiny, about 100ms. So we can treat it simultaneous. > > This is a trace from the case where the bonding driver reports 0 Mbps: > > kworker/u48:1-27950 [010] .... 6493.084916: ixgbe_service_task: eth1: link_speed = 80, link_up = false > kworker/u48:1-27950 [011] .... 6493.184894: ixgbe_service_task: eth1: link_speed = 80, link_up = false > kworker/u48:1-27950 [000] .... 6494.439883: ixgbe_service_task: eth1: link_speed = 80, link_up = true > kworker/u48:1-27950 [000] .... 6494.464204: ixgbe_service_task: eth1: NIC Link is Up 10 Gbps, Flow Control: RX/TX > kworker/0:2-1926 [000] .... 6494.464249: ixgbe_get_settings: eth1: link_speed = 80, link_up = false > NetworkManager-3819 [008] .... 6494.464484: ixgbe_get_settings: eth1: link_speed = 80, link_up = false > kworker/u48:1-27950 [007] .... 6494.496886: bond_mii_monitor: bond0: link status definitely up for interface eth1, 0 Mbps full duplex > NetworkManager-3819 [008] .... 6494.496967: ixgbe_get_settings: eth1: link_speed = 80, link_up = false > kworker/u48:1-27950 [008] .... 6495.288798: ixgbe_service_task: eth1: link_speed = 80, link_up = true > kworker/u48:1-27950 [008] .... 6495.388806: ixgbe_service_task: eth1: link_speed = 80, link_up = true > > As you can see the link is initially established, but then lost and if just so happens that the > bonding driver is checking it at that time it will report 0 Mbps. Thanks for your reply. I will delve into the source code. Best Regards! Zhu Yanjun > > I will give your patch a try and see if it helps in this situation. > > Thanks, > Emil > -- 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
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Wednesday, January 06, 2016 7:34 PM >To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh >Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; netdev@vger.kernel.org; >Shteinbock, Boris (Wind River) >Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode > >On 01/07/2016 10:43 AM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>> Sent: Tuesday, January 05, 2016 7:05 PM >>> To: Tantilov, Emil S; Michal Kubecek; Jay Vosburgh >>> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; >netdev@vger.kernel.org; >>> Shteinbock, Boris (Wind River) >>> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >>> >>> On 01/06/2016 09:26 AM, Tantilov, Emil S wrote: >>>>> -----Original Message----- >>>>> From: netdev-owner@vger.kernel.org [mailto:netdev- >owner@vger.kernel.org] >>> On >>>>> Behalf Of zhuyj >>>>> Sent: Monday, December 28, 2015 1:19 AM >>>>> To: Michal Kubecek; Jay Vosburgh >>>>> Cc: vfalico@gmail.com; gospo@cumulusnetworks.com; >>> netdev@vger.kernel.org; >>>>> Shteinbock, Boris (Wind River) >>>>> Subject: Re: [PATCH 1/1] bonding: restrict up state in 802.3ad mode >>>>> >>>>> On 12/28/2015 04:43 PM, Michal Kubecek wrote: >>>>>> On Thu, Dec 17, 2015 at 01:57:16PM -0800, Jay Vosburgh wrote: >>>>>>> <zyjzyj2000@gmail.com> wrote: >>>>>>>> In 802.3ad mode, the speed and duplex is needed. But in some NIC, >>>>>>>> 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 be compatible with more NICs, it is >>>>>>>> necessary to restrict the up state in 802.3ad mode. >>>>>>> What device is this? It seems a bit odd that an Ethernet >device >>>>>>> can be carrier up but not have the duplex and speed available. >>>>>> ... >>>>>>> In general, though, bonding expects a speed or duplex change to >>>>>>> be announced via a NETDEV_UPDATE or NETDEV_UP notifier, which would >>>>>>> propagate to the 802.3ad logic. >>>>>>> >>>>>>> If the device here is going carrier up prior to having speed or >>>>>>> duplex available, then maybe it should call netdev_state_change() >when >>>>>>> the duplex and speed are available, or delay calling >>> netif_carrier_on(). >>>>>> I have encountered this problem (NIC having carrier on before being >>> able >>>>>> to detect speed/duplex and driver not notifying when speed/duplex >>>>>> becomes available) with netxen cards earlier. But it was eventually >>>>>> fixed in the driver by commit 9d01412ae76f ("netxen: Fix link event >>>>>> handling.") so this example rather supports what you said. >>>>>> >>>>>> Michal >>> Kubecek >>>>> Thanks a lot. >>>>> I checked the commit 9d01412ae76f ("netxen: Fix link event >>>>> handling."). The symptoms are the same with mine. >>>>> >>>>> The root cause is different. In my problem, the root cause is that >LINKS >>>>> register[] can not provide link_up and link_speed at the same time. >>>>> There is a time span between link_up and link_speed. >>>> The LINK_UP and LINK_SPEED bits in the LINKS register for ixgbe HW are >>> updated >>>> simultaneously. Do you have any proof to show the delay you are >referring >>> to >>>> as I am sure our HW engineers would like to know about it. >>> Sorry. I can not reproduce this problem locally. What I have is the >>> feedback from the customer. >> So you are assuming that there is a delay due to the issue you are >seeing? > >Sure. Before I get the further feedback from the customer, I can not >make further conclusion. >My patch is based on the feedback from the customer. Your patch is throwing an RTNL assertion warning: RTNL: assertion failed at net/core/ethtool.c (357) Looks like you may need to hold an RTNL lock for the slave before calling bond_update_speed_duplex(), though I am not sure if it's a good idea in general. Thanks, Emil -- 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 --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 9e0f8a7..0a80fb3 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1991,6 +1991,25 @@ static int bond_miimon_inspect(struct bonding *bond) link_state = bond_check_dev_link(bond, slave->dev, 0); + /* Since some NIC has time span between netif_running and + * getting speed and duples. That is, after a NIC is up (netif_running), + * there is a time span before this NIC is negotiated with speed and duplex. + * During this time span, the slave in 802.3ad is configured without speed + * and duplex. This 802.3ad bonding will not work because it needs slave's speed + * and duplex to generate key field. + * As such, we restrict up in 802.3ad mode to: netif_running && peed != SPEED_UNKNOWN && + * duplex != DUPLEX_UNKNOWN + */ + if ((BMSR_LSTATUS == link_state) && + (BOND_MODE(bond) == BOND_MODE_8023AD)) { + bond_update_speed_duplex(slave); + if ((slave->speed == SPEED_UNKNOWN) || + (slave->duplex == DUPLEX_UNKNOWN)) { + link_state = 0; + netdev_info(bond->dev, "In 802.3ad mode, it is not enough to up without speed and duplex"); + } + } + switch (slave->link) { case BOND_LINK_UP: if (link_state)