| Message ID | 1450853205-27133-1-git-send-email-zyjzyj2000@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Jeff Kirsher |
| Headers | show |
On Wed, 2015-12-23 at 14:46 +0800, zyjzyj2000@gmail.com wrote: > From: Zhu Yanjun <zyjzyj2000@gmail.com> > > In X540 NIC, there is a time span between reporting "link on" and > getting the speed and duplex. To a bonding driver in 802.3ad mode, > this time span will make it not work well if the time span is big > enough. The big time span will make bonding driver change the state > of > the slave device to up while the speed and duplex of the slave device > can not be gotten. Later the bonding driver will not have change to > get the speed and duplex of the slave device. The speed and duplex of > the slave device are important to a bonding driver in 802.3ad mode. > > To 82599_SFP NIC and other kinds of NICs, this problem does > not exist. As such, it is necessary for X540 to report"link on" when > the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 > +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index aed8d02..cb9d310 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct > ixgbe_adapter *adapter) > (flow_rx ? "RX" : > (flow_tx ? "TX" : "None")))); > > - netif_carrier_on(netdev); > + /* > + * In X540 NIC, there is a time span between reporting "link > on" > + * and getting the speed and duplex. To a bonding driver in > 802.3ad > + * mode, this time span will make it not work well if the > time span > + * is big enough. To 82599_SFP NIC and other kinds of NICs, > this > + * problem does not exist. As such, it is better for X540 to > report > + * "link on" when the link speed is not > IXGBE_LINK_SPEED_UNKNOWN. > + */ > + if ((hw->mac.type == ixgbe_mac_X540) && > + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { > + netif_carrier_on(netdev); > + } else { > + netif_carrier_on(netdev); > + } > + > ixgbe_check_vf_rate_limit(adapter); > > /* enable transmits */ This patch only adds a needless test before calling netif_carrier_on(netdev), since the call happens no matter the branch you take, it appears your running into a timing issue. So adding a wait() before calling netif_carrier_on(netdev) will accomplish the same result and you do not have to add a useless test.
Hello. On 12/23/2015 9:46 AM, zyjzyj2000@gmail.com wrote: > From: Zhu Yanjun <zyjzyj2000@gmail.com> > > In X540 NIC, there is a time span between reporting "link on" and > getting the speed and duplex. To a bonding driver in 802.3ad mode, > this time span will make it not work well if the time span is big > enough. The big time span will make bonding driver change the state of > the slave device to up while the speed and duplex of the slave device > can not be gotten. Later the bonding driver will not have change to > get the speed and duplex of the slave device. The speed and duplex of > the slave device are important to a bonding driver in 802.3ad mode. > > To 82599_SFP NIC and other kinds of NICs, this problem does > not exist. As such, it is necessary for X540 to report"link on" when > the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index aed8d02..cb9d310 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter) > (flow_rx ? "RX" : > (flow_tx ? "TX" : "None")))); > > - netif_carrier_on(netdev); > + /* > + * In X540 NIC, there is a time span between reporting "link on" > + * and getting the speed and duplex. To a bonding driver in 802.3ad > + * mode, this time span will make it not work well if the time span > + * is big enough. To 82599_SFP NIC and other kinds of NICs, this > + * problem does not exist. As such, it is better for X540 to report > + * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > + */ > + if ((hw->mac.type == ixgbe_mac_X540) && > + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { > + netif_carrier_on(netdev); > + } else { > + netif_carrier_on(netdev); > + } > + {} not needed here. And you do the same thing regardless of whether your check succeeds or not, this doesn't make sense. [...] MBR, Sergei
>-----Original Message----- >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >Behalf Of zyjzyj2000@gmail.com >Sent: Tuesday, December 22, 2015 10:47 PM >To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch >A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >devel@lists.sourceforge.net >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >Vincent (Wind River) >Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >reporting "link on" and getting speed and duplex > >From: Zhu Yanjun <zyjzyj2000@gmail.com> > >In X540 NIC, there is a time span between reporting "link on" and >getting the speed and duplex. To a bonding driver in 802.3ad mode, >this time span will make it not work well if the time span is big >enough. The big time span will make bonding driver change the state of >the slave device to up while the speed and duplex of the slave device >can not be gotten. Later the bonding driver will not have change to >get the speed and duplex of the slave device. The speed and duplex of >the slave device are important to a bonding driver in 802.3ad mode. > >To 82599_SFP NIC and other kinds of NICs, this problem does >not exist. As such, it is necessary for X540 to report"link on" when >the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > >Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >--- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > >diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >index aed8d02..cb9d310 100644 >--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct >ixgbe_adapter *adapter) > (flow_rx ? "RX" : > (flow_tx ? "TX" : "None")))); > >- netif_carrier_on(netdev); >+ /* >+ * In X540 NIC, there is a time span between reporting "link on" >+ * and getting the speed and duplex. To a bonding driver in 802.3ad >+ * mode, this time span will make it not work well if the time span >+ * is big enough. To 82599_SFP NIC and other kinds of NICs, this >+ * problem does not exist. As such, it is better for X540 to report >+ * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >+ */ >+ if ((hw->mac.type == ixgbe_mac_X540) && >+ (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { >+ netif_carrier_on(netdev); >+ } else { >+ netif_carrier_on(netdev); >+ } >+ > ixgbe_check_vf_rate_limit(adapter); > > /* enable transmits */ >-- >1.7.9.5 NAK I have already submitted a patch that will address the issue with bonding reporting unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps: http://patchwork.ozlabs.org/patch/552485/ The bonding driver gets the speed from ethtool and this is where the reporting needs to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a certain rate and as such will not be able to detect rapid link changes. If there is a case where link_speed is unknown when entering this function it's probably better to just bail rather than have this hack around the netif_carrier_on() especially after the driver already reported link status change. Rapid link changes can occur between link partners and not just for X540. Thanks, Emil
On Wed, 23 Dec 2015 15:59:24 +0000 "Tantilov, Emil S" <emil.s.tantilov@intel.com> wrote: > >-----Original Message----- > >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On > >Behalf Of zyjzyj2000@gmail.com > >Sent: Tuesday, December 22, 2015 10:47 PM > >To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, > >Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch > >A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- > >devel@lists.sourceforge.net > >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, > >Vincent (Wind River) > >Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize > >reporting "link on" and getting speed and duplex > > > >From: Zhu Yanjun <zyjzyj2000@gmail.com> > > > >In X540 NIC, there is a time span between reporting "link on" and > >getting the speed and duplex. To a bonding driver in 802.3ad mode, > >this time span will make it not work well if the time span is big > >enough. The big time span will make bonding driver change the state of > >the slave device to up while the speed and duplex of the slave device > >can not be gotten. Later the bonding driver will not have change to > >get the speed and duplex of the slave device. The speed and duplex of > >the slave device are important to a bonding driver in 802.3ad mode. > > > >To 82599_SFP NIC and other kinds of NICs, this problem does > >not exist. As such, it is necessary for X540 to report"link on" when > >the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > > > >Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> > >--- > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > >b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > >index aed8d02..cb9d310 100644 > >--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > >+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > >@@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct > >ixgbe_adapter *adapter) > > (flow_rx ? "RX" : > > (flow_tx ? "TX" : "None")))); > > > >- netif_carrier_on(netdev); > >+ /* > >+ * In X540 NIC, there is a time span between reporting "link on" > >+ * and getting the speed and duplex. To a bonding driver in 802.3ad > >+ * mode, this time span will make it not work well if the time span > >+ * is big enough. To 82599_SFP NIC and other kinds of NICs, this > >+ * problem does not exist. As such, it is better for X540 to report > >+ * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. > >+ */ > >+ if ((hw->mac.type == ixgbe_mac_X540) && > >+ (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { > >+ netif_carrier_on(netdev); > >+ } else { > >+ netif_carrier_on(netdev); > >+ } > >+ > > ixgbe_check_vf_rate_limit(adapter); > > > > /* enable transmits */ > >-- > >1.7.9.5 > > NAK > > I have already submitted a patch that will address the issue with bonding reporting > unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps: > http://patchwork.ozlabs.org/patch/552485/ > > The bonding driver gets the speed from ethtool and this is where the reporting needs > to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a > certain rate and as such will not be able to detect rapid link changes. Kernel has netdev notifiers, these should really always be used rather than polling. The polling stuff was added back in ancient kernel days when netdev notifiers didn't work because most drivers were broken.
On 12/23/2015 11:59 PM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >> Behalf Of zyjzyj2000@gmail.com >> Sent: Tuesday, December 22, 2015 10:47 PM >> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch >> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >> devel@lists.sourceforge.net >> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >> Vincent (Wind River) >> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >> reporting "link on" and getting speed and duplex >> >> From: Zhu Yanjun <zyjzyj2000@gmail.com> >> >> In X540 NIC, there is a time span between reporting "link on" and >> getting the speed and duplex. To a bonding driver in 802.3ad mode, >> this time span will make it not work well if the time span is big >> enough. The big time span will make bonding driver change the state of >> the slave device to up while the speed and duplex of the slave device >> can not be gotten. Later the bonding driver will not have change to >> get the speed and duplex of the slave device. The speed and duplex of >> the slave device are important to a bonding driver in 802.3ad mode. >> >> To 82599_SFP NIC and other kinds of NICs, this problem does >> not exist. As such, it is necessary for X540 to report"link on" when >> the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >> >> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index aed8d02..cb9d310 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct >> ixgbe_adapter *adapter) >> (flow_rx ? "RX" : >> (flow_tx ? "TX" : "None")))); >> >> - netif_carrier_on(netdev); >> + /* >> + * In X540 NIC, there is a time span between reporting "link on" >> + * and getting the speed and duplex. To a bonding driver in 802.3ad >> + * mode, this time span will make it not work well if the time span >> + * is big enough. To 82599_SFP NIC and other kinds of NICs, this >> + * problem does not exist. As such, it is better for X540 to report >> + * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >> + */ >> + if ((hw->mac.type == ixgbe_mac_X540) && >> + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { >> + netif_carrier_on(netdev); >> + } else { >> + netif_carrier_on(netdev); >> + } >> + >> ixgbe_check_vf_rate_limit(adapter); >> >> /* enable transmits */ >> -- >> 1.7.9.5 > NAK > > I have already submitted a patch that will address the issue with bonding reporting > unknown speed (in /proc/bonding/bondX) after the link is established due to link flaps: > http://patchwork.ozlabs.org/patch/552485/ > > The bonding driver gets the speed from ethtool and this is where the reporting needs > to be fixed. The issue is that the bonding driver polls for netif_carrier_ok() at a > certain rate and as such will not be able to detect rapid link changes. Thanks for your reply. The root cause is different from my problem. My problem is that "link up" is prior to "speed and duplex". That is, the physical NIC reports "link up" while the speed is unknown at the same time. We can run "ethtool ethx" to confirm it. Any way, thanks a lot. Zhu Yanjun > > If there is a case where link_speed is unknown when entering this function it's probably > better to just bail rather than have this hack around the netif_carrier_on() especially > after the driver already reported link status change. Rapid link changes can occur between > link partners and not just for X540. > > Thanks, > Emil >
Hi, Jeff Thanks for your reply. Changes: Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready. Best Regards! Zhu Yanjun
On 12/24/2015 11:12 AM, zyjzyj2000@gmail.com wrote: > Hi, Jeff > > Thanks for your reply. > > Changes: > Since there is a time span between link_up and link_speed to X540 NIC, it is not appropriate to continue in the function ixgbe_watchdog_link_is_up if only link_up is ready. > > 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 > Hi, Jeff The following logs are from the customer. From these logs, there is a time span between link_up and link_speed. This time span sometimes will make bonding in 802.3ad mode not work well. What I have done in the patch is to prevent this time span in the ixgbe driver. Best Regards! Zhu Yanjun 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
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Wednesday, December 23, 2015 6:28 PM >To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, >Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, >John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org; >netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >Vincent (Wind River) >Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >reporting "link on" and getting speed and duplex > >On 12/23/2015 11:59 PM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] >On >>> Behalf Of zyjzyj2000@gmail.com >>> Sent: Tuesday, December 22, 2015 10:47 PM >>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, >Mitch >>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >>> devel@lists.sourceforge.net >>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); >Bourg, >>> Vincent (Wind River) >>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >>> reporting "link on" and getting speed and duplex >>> >>> From: Zhu Yanjun <zyjzyj2000@gmail.com> >>> >>> In X540 NIC, there is a time span between reporting "link on" and >>> getting the speed and duplex. To a bonding driver in 802.3ad mode, >>> this time span will make it not work well if the time span is big >>> enough. The big time span will make bonding driver change the state of >>> the slave device to up while the speed and duplex of the slave device >>> can not be gotten. Later the bonding driver will not have change to >>> get the speed and duplex of the slave device. The speed and duplex of >>> the slave device are important to a bonding driver in 802.3ad mode. >>> >>> To 82599_SFP NIC and other kinds of NICs, this problem does >>> not exist. As such, it is necessary for X540 to report"link on" when >>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >>> >>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >>> --- >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- >>> 1 file changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> index aed8d02..cb9d310 100644 >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct >>> ixgbe_adapter *adapter) >>> (flow_rx ? "RX" : >>> (flow_tx ? "TX" : "None")))); >>> >>> - netif_carrier_on(netdev); >>> + /* >>> + * In X540 NIC, there is a time span between reporting "link on" >>> + * and getting the speed and duplex. To a bonding driver in 802.3ad >>> + * mode, this time span will make it not work well if the time span >>> + * is big enough. To 82599_SFP NIC and other kinds of NICs, this >>> + * problem does not exist. As such, it is better for X540 to report >>> + * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >>> + */ >>> + if ((hw->mac.type == ixgbe_mac_X540) && >>> + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { >>> + netif_carrier_on(netdev); >>> + } else { >>> + netif_carrier_on(netdev); >>> + } >>> + >>> ixgbe_check_vf_rate_limit(adapter); >>> >>> /* enable transmits */ >>> -- >>> 1.7.9.5 >> NAK >> >> I have already submitted a patch that will address the issue with bonding >reporting >> unknown speed (in /proc/bonding/bondX) after the link is established due >to link flaps: >> http://patchwork.ozlabs.org/patch/552485/ >> >> The bonding driver gets the speed from ethtool and this is where the >reporting needs >> to be fixed. The issue is that the bonding driver polls for >netif_carrier_ok() at a >> certain rate and as such will not be able to detect rapid link changes. >Thanks for your reply. The root cause is different from my problem. My >problem is that >"link up" is prior to "speed and duplex". That is, the physical NIC >reports "link up" while The "link up" event is a result of an LSC interrupt, the speed is determined as result of that interrupt by checking the LINKS register. If the LINKS register reports link as unknown then that is the actual state of the PHY - meaning the device is re-negotiating the speed for some reason. >the speed is unknown at the same time. We can run "ethtool ethx" to >confirm it. Prior to my patch the ethtool call will read the LINKS register which can show speed as unknown due to a link flap (for example). You are seeing the momentary state of the device. If you are still seeing the bond reporting "unknown" speed after the patch I pointed out please file a bug either through e1000.sf.net or via Intel support and provide detailed information about the bonding setup, the type of the link partner (switch model etc) and full dmesg from the failed scenario along with the output from /proc/bonding/bond0 Thanks, Emil
>-----Original Message----- >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >Behalf Of zhuyj >Sent: Wednesday, December 23, 2015 9:11 PM >To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, Mitch >A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >devel@lists.sourceforge.net >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >Vincent (Wind River) >Subject: Re: [Intel-wired-lan] [V2 PATCH 1/1] ixgbe: force to synchronize >reporting "link on" and > >On 12/24/2015 11:12 AM, zyjzyj2000@gmail.com <mailto:zyjzyj2000@gmail.com> >wrote: > > > > Hi, Jeff > > Thanks for your reply. > > Changes: > Since there is a time span between link_up and link_speed to X540 >NIC, it is not appropriate to continue in the function >ixgbe_watchdog_link_is_up if only link_up is ready. > > 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 ><mailto:majordomo@vger.kernel.org> > More majordomo info at http://vger.kernel.org/majordomo-info.html > > >Hi, Jeff > >The following logs are from the customer. From these logs, there is a time >span between link_up and link_speed. >This time span sometimes will make bonding in 802.3ad mode not work well. > >What I have done in the patch is to prevent this time span in the ixgbe >driver. > >Best Regards! >Zhu Yanjun > >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 Can you provide the related dmesg output? What did the driver report in the "Link Up" messages? Thanks, Emil
On 12/24/2015 01:58 PM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Wednesday, December 23, 2015 6:28 PM >> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, >> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, >> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org; >> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net >> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >> Vincent (Wind River) >> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >> reporting "link on" and getting speed and duplex >> >> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote: >>>> -----Original Message----- >>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] >> On >>>> Behalf Of zyjzyj2000@gmail.com >>>> Sent: Tuesday, December 22, 2015 10:47 PM >>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, >> Mitch >>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >>>> devel@lists.sourceforge.net >>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); >> Bourg, >>>> Vincent (Wind River) >>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >>>> reporting "link on" and getting speed and duplex >>>> >>>> From: Zhu Yanjun <zyjzyj2000@gmail.com> >>>> >>>> In X540 NIC, there is a time span between reporting "link on" and >>>> getting the speed and duplex. To a bonding driver in 802.3ad mode, >>>> this time span will make it not work well if the time span is big >>>> enough. The big time span will make bonding driver change the state of >>>> the slave device to up while the speed and duplex of the slave device >>>> can not be gotten. Later the bonding driver will not have change to >>>> get the speed and duplex of the slave device. The speed and duplex of >>>> the slave device are important to a bonding driver in 802.3ad mode. >>>> >>>> To 82599_SFP NIC and other kinds of NICs, this problem does >>>> not exist. As such, it is necessary for X540 to report"link on" when >>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >>>> >>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >>>> --- >>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- >>>> 1 file changed, 15 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> index aed8d02..cb9d310 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct >>>> ixgbe_adapter *adapter) >>>> (flow_rx ? "RX" : >>>> (flow_tx ? "TX" : "None")))); >>>> >>>> - netif_carrier_on(netdev); >>>> + /* >>>> + * In X540 NIC, there is a time span between reporting "link on" >>>> + * and getting the speed and duplex. To a bonding driver in 802.3ad >>>> + * mode, this time span will make it not work well if the time span >>>> + * is big enough. To 82599_SFP NIC and other kinds of NICs, this >>>> + * problem does not exist. As such, it is better for X540 to report >>>> + * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >>>> + */ >>>> + if ((hw->mac.type == ixgbe_mac_X540) && >>>> + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { >>>> + netif_carrier_on(netdev); >>>> + } else { >>>> + netif_carrier_on(netdev); >>>> + } >>>> + >>>> ixgbe_check_vf_rate_limit(adapter); >>>> >>>> /* enable transmits */ >>>> -- >>>> 1.7.9.5 >>> NAK >>> >>> I have already submitted a patch that will address the issue with bonding >> reporting >>> unknown speed (in /proc/bonding/bondX) after the link is established due >> to link flaps: >>> http://patchwork.ozlabs.org/patch/552485/ >>> >>> The bonding driver gets the speed from ethtool and this is where the >> reporting needs >>> to be fixed. The issue is that the bonding driver polls for >> netif_carrier_ok() at a >>> certain rate and as such will not be able to detect rapid link changes. >> Thanks for your reply. The root cause is different from my problem. My >> problem is that >> "link up" is prior to "speed and duplex". That is, the physical NIC >> reports "link up" while > The "link up" event is a result of an LSC interrupt, the speed is > determined as result of that interrupt by checking the LINKS register. Hi, Sorry. I do not agree with you. Please see the followings for details. /** * ixgbe_watchdog_update_link - update the link status * @adapter: pointer to the device adapter structure * @link_speed: pointer to a u32 to store the link_speed **/ static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter) From this function, link_up and link_speed is from watchdo poll. Thanks for your reply. Zhu Yanjun > If the LINKS register reports link as unknown then that is the actual state > of the PHY - meaning the device is re-negotiating the speed for some reason. > >> the speed is unknown at the same time. We can run "ethtool ethx" to >> confirm it. > Prior to my patch the ethtool call will read the LINKS register which can show > speed as unknown due to a link flap (for example). You are seeing the momentary > state of the device. > > If you are still seeing the bond reporting "unknown" speed after the patch I pointed > out please file a bug either through e1000.sf.net or via Intel support and provide > detailed information about the bonding setup, the type of the link partner (switch > model etc) and full dmesg from the failed scenario along with the output from > /proc/bonding/bond0 > > Thanks, > Emil >
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Wednesday, December 23, 2015 10:24 PM >To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, >Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, >John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org; >netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net >Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); Bourg, >Vincent (Wind River) >Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >reporting "link on" and getting speed and duplex > >On 12/24/2015 01:58 PM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>> Sent: Wednesday, December 23, 2015 6:28 PM >>> To: Tantilov, Emil S; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, >>> Shannon; Wyborny, Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, >>> John; Williams, Mitch A; intel-wired-lan@lists.osuosl.org; >>> netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net >>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); >Bourg, >>> Vincent (Wind River) >>> Subject: Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >>> reporting "link on" and getting speed and duplex >>> >>> On 12/23/2015 11:59 PM, Tantilov, Emil S wrote: >>>>> -----Original Message----- >>>>> From: Intel-wired-lan [mailto:intel-wired-lan- >bounces@lists.osuosl.org] >>> On >>>>> Behalf Of zyjzyj2000@gmail.com >>>>> Sent: Tuesday, December 22, 2015 10:47 PM >>>>> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, >>>>> Carolyn; Skidmore, Donald C; Allan, Bruce W; Ronciak, John; Williams, >>> Mitch >>>>> A; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; e1000- >>>>> devel@lists.sourceforge.net >>>>> Cc: Viswanathan, Ven (Wind River); Shteinbock, Boris (Wind River); >>> Bourg, >>>>> Vincent (Wind River) >>>>> Subject: [Intel-wired-lan] [PATCH 1/1] ixgbe: force to synchronize >>>>> reporting "link on" and getting speed and duplex >>>>> >>>>> From: Zhu Yanjun <zyjzyj2000@gmail.com> >>>>> >>>>> In X540 NIC, there is a time span between reporting "link on" and >>>>> getting the speed and duplex. To a bonding driver in 802.3ad mode, >>>>> this time span will make it not work well if the time span is big >>>>> enough. The big time span will make bonding driver change the state of >>>>> the slave device to up while the speed and duplex of the slave device >>>>> can not be gotten. Later the bonding driver will not have change to >>>>> get the speed and duplex of the slave device. The speed and duplex of >>>>> the slave device are important to a bonding driver in 802.3ad mode. >>>>> >>>>> To 82599_SFP NIC and other kinds of NICs, this problem does >>>>> not exist. As such, it is necessary for X540 to report"link on" when >>>>> the link speed is not IXGBE_LINK_SPEED_UNKNOWN. >>>>> >>>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com> >>>>> --- >>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++++++- >>>>> 1 file changed, 15 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>>> index aed8d02..cb9d310 100644 >>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>>> @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct >>>>> ixgbe_adapter *adapter) >>>>> (flow_rx ? "RX" : >>>>> (flow_tx ? "TX" : "None")))); >>>>> >>>>> - netif_carrier_on(netdev); >>>>> + /* >>>>> + * In X540 NIC, there is a time span between reporting "link >on" >>>>> + * and getting the speed and duplex. To a bonding driver in >802.3ad >>>>> + * mode, this time span will make it not work well if the time >span >>>>> + * is big enough. To 82599_SFP NIC and other kinds of NICs, >this >>>>> + * problem does not exist. As such, it is better for X540 to >report >>>>> + * "link on" when the link speed is not >IXGBE_LINK_SPEED_UNKNOWN. >>>>> + */ >>>>> + if ((hw->mac.type == ixgbe_mac_X540) && >>>>> + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { >>>>> + netif_carrier_on(netdev); >>>>> + } else { >>>>> + netif_carrier_on(netdev); >>>>> + } >>>>> + >>>>> ixgbe_check_vf_rate_limit(adapter); >>>>> >>>>> /* enable transmits */ >>>>> -- >>>>> 1.7.9.5 >>>> NAK >>>> >>>> I have already submitted a patch that will address the issue with >bonding >>> reporting >>>> unknown speed (in /proc/bonding/bondX) after the link is established >due >>> to link flaps: >>>> http://patchwork.ozlabs.org/patch/552485/ >>>> >>>> The bonding driver gets the speed from ethtool and this is where the >>> reporting needs >>>> to be fixed. The issue is that the bonding driver polls for >>> netif_carrier_ok() at a >>>> certain rate and as such will not be able to detect rapid link changes. >>> Thanks for your reply. The root cause is different from my problem. My >>> problem is that >>> "link up" is prior to "speed and duplex". That is, the physical NIC >>> reports "link up" while >> The "link up" event is a result of an LSC interrupt, the speed is >> determined as result of that interrupt by checking the LINKS register. >Hi, > >Sorry. I do not agree with you. Please see the followings for details. > >/** > * ixgbe_watchdog_update_link - update the link status > * @adapter: pointer to the device adapter structure > * @link_speed: pointer to a u32 to store the link_speed > **/ >static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter) > > From this function, link_up and link_speed is from watchdo poll. > >Thanks for your reply. ixgbe_watchdog_update_link() only runs when the IXGBE_FLAG_NEED_LINK_UPDATE is set which can happen in 2 situations - LSC is received or the interface is brought up. At that point the driver will call hw->mac.ops.check_link() which sets adapter->link_up based on the LINKS register and if we have link_up then we update the link status: 6669 if (adapter->link_up) 6670 ixgbe_watchdog_link_is_up(adapter); The thing is if the LINKS.LINK_UP bit was set then at that point we also have a speed which is contrary to what you are suggesting. If you want to debug your issue you have to monitor the LINKS register. That is why I asked for the dmesg output - if your claim is correct then the "Link Up" event in dmesg will be reporting "unknown speed" and if not then we are not dealing with delayed speed, but a simple link flap. Thanks, Emil
Hi, all During the discussion with Emil, I think the time span between link_up and link_speed is not important when the X540 NIC acts as an independent interface. As such, a new patch is made to restrict synchronization between link_up and link_speed. Any reply is appreciated. Zhu Yanjun
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index aed8d02..cb9d310 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -6479,7 +6479,21 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter) (flow_rx ? "RX" : (flow_tx ? "TX" : "None")))); - netif_carrier_on(netdev); + /* + * In X540 NIC, there is a time span between reporting "link on" + * and getting the speed and duplex. To a bonding driver in 802.3ad + * mode, this time span will make it not work well if the time span + * is big enough. To 82599_SFP NIC and other kinds of NICs, this + * problem does not exist. As such, it is better for X540 to report + * "link on" when the link speed is not IXGBE_LINK_SPEED_UNKNOWN. + */ + if ((hw->mac.type == ixgbe_mac_X540) && + (link_speed != IXGBE_LINK_SPEED_UNKNOWN)) { + netif_carrier_on(netdev); + } else { + netif_carrier_on(netdev); + } + ixgbe_check_vf_rate_limit(adapter); /* enable transmits */