| Message ID | 1451356326-2919-3-git-send-email-zyjzyj2000@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
>-----Original Message----- >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >Behalf Of zyjzyj2000@gmail.com >Sent: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization of >link_up and speed > >From: Zhu Yanjun <yanjun.zhu@windriver.com> > >When the X540 NIC acts as a slave of some virtual NICs, it is very >important to synchronize link_up and link_speed, such as a bonding >driver in 802.3ad mode. When X540 NIC acts as an independent interface, >it is not necessary to synchronize link_up and link_speed. That is, >the time span between link_up and link_speed is acceptable. What exactly do you mean by "time span between link_up and link_speed"? Where is it you think the de-synchronization occurs? >Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com> >--- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > >diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >index ace21b9..1bb6056 100644 >--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >@@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct >ixgbe_adapter *adapter) > * time. To X540 NIC, there is a time span between link_up and > * link_speed. As such, only continue if link_up and link_speed are > * ready to X540 NIC. >+ * The time span between link_up and link_speed is very important >+ * when the X540 NIC acts as a slave in some virtual NICs, such as >+ * a bonding driver in 802.3ad mode. When X540 NIC acts as an >+ * independent interface, it is not necessary to synchronize link_up >+ * and link_speed. >+ * In the end, not continue if (X540 NIC && SLAVE && link_speed >UNKNOWN) This is a patch on top of your previous patch which I don't think was applied, so this is not going to apply cleanly. > */ >- if (hw->mac.type == ixgbe_mac_X540) >+ if ((hw->mac.type == ixgbe_mac_X540) && >+ (netdev->flags & IFF_SLAVE)) > if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) > return; If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would assume that you also have a dmesg that shows: "NIC Link is Up unknown speed" by the interface you use in the bond? Thanks, Emil
Emil S <emil.s.tantilov@intel.com> wrote: >> */ >> - if (hw->mac.type == ixgbe_mac_X540) >> + if ((hw->mac.type == ixgbe_mac_X540) && >> + (netdev->flags & IFF_SLAVE)) >> if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) >> return; > > If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would > assume that you also have a dmesg that shows: > "NIC Link is Up unknown speed" > > by the interface you use in the bond? It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540. -- Mark Rustad, Networking Division, Intel Corporation
On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization of >> link_up and speed >> >> From: Zhu Yanjun <yanjun.zhu@windriver.com> >> >> When the X540 NIC acts as a slave of some virtual NICs, it is very >> important to synchronize link_up and link_speed, such as a bonding >> driver in 802.3ad mode. When X540 NIC acts as an independent interface, >> it is not necessary to synchronize link_up and link_speed. That is, >> the time span between link_up and link_speed is acceptable. > What exactly do you mean by "time span between link_up and link_speed"? In the previous mail, I show you some ethtool logs. In these logs, there is some time with NIC up while speed is unknown. I think this "some time" is time span between link_up and link_speed. Please see the previous mail for details. > Where is it you think the de-synchronization occurs? When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in netdevice struct. Before we enter this function, we check IFF_SLAVE flag. If this flag is set, we continue to check link_speed. If not, this function is executed whether this link_speed is unknown or not. > >> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index ace21b9..1bb6056 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct >> ixgbe_adapter *adapter) >> * time. To X540 NIC, there is a time span between link_up and >> * link_speed. As such, only continue if link_up and link_speed are >> * ready to X540 NIC. >> + * The time span between link_up and link_speed is very important >> + * when the X540 NIC acts as a slave in some virtual NICs, such as >> + * a bonding driver in 802.3ad mode. When X540 NIC acts as an >> + * independent interface, it is not necessary to synchronize link_up >> + * and link_speed. >> + * In the end, not continue if (X540 NIC && SLAVE && link_speed >> UNKNOWN) > This is a patch on top of your previous patch which I don't think was applied, > so this is not going to apply cleanly. > >> */ >> - if (hw->mac.type == ixgbe_mac_X540) >> + if ((hw->mac.type == ixgbe_mac_X540) && >> + (netdev->flags & IFF_SLAVE)) >> if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) >> return; > If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would > assume that you also have a dmesg that shows: > "NIC Link is Up unknown speed" > > by the interface you use in the bond? Sure. There is a dmesg log from the customer. " ... 2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link status definitely up for interface eth0, 0 Mbps full duplex. ... " Thanks a lot. Zhu Yanjun > > Thanks, > Emil >
On 12/30/2015 03:17 AM, Rustad, Mark D wrote: > Emil S <emil.s.tantilov@intel.com> wrote: > >>> */ >>> - if (hw->mac.type == ixgbe_mac_X540) >>> + if ((hw->mac.type == ixgbe_mac_X540) && >>> + (netdev->flags & IFF_SLAVE)) >>> if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) >>> return; >> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then I would >> assume that you also have a dmesg that shows: >> "NIC Link is Up unknown speed" >> >> by the interface you use in the bond? > It seems odd to be checking the MAC type for this. Is this behavior perhaps more related to the copper phy? If so, then the check should be changed. Or would the check for unknown link speed be sufficient? It seems like an interface as a slave would not work with an unknown link speed, so it may as well wait in all cases, not just for X540. > > -- > Mark Rustad, Networking Division, Intel Corporation Hi, Mark Thanks for your suggestions. The following is the feedback from the customer. " ... We observing this issue on x540T interfaces (8086:1528), but not on 82599_SFP (8086:10FB). ... " To narrow this problem, I restrict mac.type to ixgbe_mac_X540. I agree with you. Maybe this problem is related with the copper phy. But I only have X540 NIC to test. So it is difficult for me to confirm whether this problem occurs on other mac type or not, such as X550. I will consider your suggestions in the latest patch. Thanks again. Zhu Yanjun
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict synchronization >of link_up and speed > >On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization >of >>> link_up and speed >>> >>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>> >>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>> important to synchronize link_up and link_speed, such as a bonding >>> driver in 802.3ad mode. When X540 NIC acts as an independent interface, >>> it is not necessary to synchronize link_up and link_speed. That is, >>> the time span between link_up and link_speed is acceptable. >> What exactly do you mean by "time span between link_up and link_speed"? > >In the previous mail, I show you some ethtool logs. In these logs, there >is some >time with NIC up while speed is unknown. I think this "some time" is >time span between >link_up and link_speed. Please see the previous mail for details. Was this when reporting the link state from check_link() (reading the LINKS register) or reporting the adapter->link_speed? >> Where is it you think the de-synchronization occurs? > >When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >netdevice struct. >Before we enter this function, we check IFF_SLAVE flag. If this flag is >set, we continue to check >link_speed. If not, this function is executed whether this link_speed is >unknown or not. I can already see this in your patch. I was asking about the reason why your change is needed. >> >>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com> >>> --- >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> index ace21b9..1bb6056 100644 >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct >>> ixgbe_adapter *adapter) >>> * time. To X540 NIC, there is a time span between link_up and >>> * link_speed. As such, only continue if link_up and link_speed are >>> * ready to X540 NIC. >>> + * The time span between link_up and link_speed is very important >>> + * when the X540 NIC acts as a slave in some virtual NICs, such as >>> + * a bonding driver in 802.3ad mode. When X540 NIC acts as an >>> + * independent interface, it is not necessary to synchronize link_up >>> + * and link_speed. >>> + * In the end, not continue if (X540 NIC && SLAVE && link_speed >>> UNKNOWN) >> This is a patch on top of your previous patch which I don't think was >applied, >> so this is not going to apply cleanly. >> >>> */ >>> - if (hw->mac.type == ixgbe_mac_X540) >>> + if ((hw->mac.type == ixgbe_mac_X540) && >>> + (netdev->flags & IFF_SLAVE)) >>> if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) >>> return; >> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then >I would >> assume that you also have a dmesg that shows: >> "NIC Link is Up unknown speed" >> >> by the interface you use in the bond? >Sure. There is a dmesg log from the customer. >" >... >2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link >status definitely up for interface eth0, 0 Mbps full duplex. This message is from the bonding driver not from ixgbe. In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up() if that condition was true then you should also see "unknown link" being reported by ixgbe. Thanks, Emil
On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict synchronization >> of link_up and speed >> >> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization >> of >>>> link_up and speed >>>> >>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>> >>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>> important to synchronize link_up and link_speed, such as a bonding >>>> driver in 802.3ad mode. When X540 NIC acts as an independent interface, >>>> it is not necessary to synchronize link_up and link_speed. That is, >>>> the time span between link_up and link_speed is acceptable. >>> What exactly do you mean by "time span between link_up and link_speed"? >> In the previous mail, I show you some ethtool logs. In these logs, there >> is some >> time with NIC up while speed is unknown. I think this "some time" is >> time span between >> link_up and link_speed. Please see the previous mail for details. > Was this when reporting the link state from check_link() (reading the LINKS > register) or reporting the adapter->link_speed? > >>> Where is it you think the de-synchronization occurs? >> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >> netdevice struct. >> Before we enter this function, we check IFF_SLAVE flag. If this flag is >> set, we continue to check >> link_speed. If not, this function is executed whether this link_speed is >> unknown or not. > I can already see this in your patch. I was asking about the reason why your > change is needed. an extreme example, let us assume this scenario: An ixgbe NIC directly connects to another NIC (let us call it NIC-a). And auto-negotiate is off while no static speed is set in the 2 NICs. These 2 NICs acts as 2 independent interfaces. As such, at this time, there is no speed in the both 2 NICs. That is, link_speed is unknown. When the user run "ifconfig or ethtool", NIC-a will show "Link detected: yes" while ixgbe NIC will show "Link detected: no" if the flag IFF_SLAVE is not set. NIC-a stands for most NIC, such as e1000, e1000e and so on. Best Regards! Zhu Yanjun > >>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@windriver.com> >>>> --- >>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- >>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> index ace21b9..1bb6056 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >>>> @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct >>>> ixgbe_adapter *adapter) >>>> * time. To X540 NIC, there is a time span between link_up and >>>> * link_speed. As such, only continue if link_up and link_speed are >>>> * ready to X540 NIC. >>>> + * The time span between link_up and link_speed is very important >>>> + * when the X540 NIC acts as a slave in some virtual NICs, such as >>>> + * a bonding driver in 802.3ad mode. When X540 NIC acts as an >>>> + * independent interface, it is not necessary to synchronize link_up >>>> + * and link_speed. >>>> + * In the end, not continue if (X540 NIC && SLAVE && link_speed >>>> UNKNOWN) >>> This is a patch on top of your previous patch which I don't think was >> applied, >>> so this is not going to apply cleanly. >>> >>>> */ >>>> - if (hw->mac.type == ixgbe_mac_X540) >>>> + if ((hw->mac.type == ixgbe_mac_X540) && >>>> + (netdev->flags & IFF_SLAVE)) >>>> if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) >>>> return; >>> If you were to enter ixgbe_watchdog_link_is_up() with unknown speed, then >> I would >>> assume that you also have a dmesg that shows: >>> "NIC Link is Up unknown speed" >>> >>> by the interface you use in the bond? >> Sure. There is a dmesg log from the customer. >> " >> ... >> 2015-10-05T06:14:34.350 controller-0 kernel: info bonding: bond0: link >> status definitely up for interface eth0, 0 Mbps full duplex. > This message is from the bonding driver not from ixgbe. > > In your patch you are adding a check for unknown link to ixgbe_watchdog_link_is_up() > if that condition was true then you should also see "unknown link" being reported by ixgbe. > > Thanks, > Emil >
Hi, all According to Rustad, Mark D, maybe it is related with copper phy. To make fiber phy more robust, synchronize both the link_up and link_speed of a slave interface in ixgbe driver. Best Regards! Zhu Yanjun
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Wednesday, December 30, 2015 12:20 AM >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 2/2] ixgbe: restrict synchronization >of link_up and speed > >On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict >synchronization >>> of link_up and speed >>> >>> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization >>> of >>>>> link_up and speed >>>>> >>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>>> >>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>>> important to synchronize link_up and link_speed, such as a bonding >>>>> driver in 802.3ad mode. When X540 NIC acts as an independent >interface, >>>>> it is not necessary to synchronize link_up and link_speed. That is, >>>>> the time span between link_up and link_speed is acceptable. >>>> What exactly do you mean by "time span between link_up and link_speed"? >>> In the previous mail, I show you some ethtool logs. In these logs, there >>> is some >>> time with NIC up while speed is unknown. I think this "some time" is >>> time span between >>> link_up and link_speed. Please see the previous mail for details. >> Was this when reporting the link state from check_link() (reading the >LINKS >> register) or reporting the adapter->link_speed? >> >>>> Where is it you think the de-synchronization occurs? >>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >>> netdevice struct. >>> Before we enter this function, we check IFF_SLAVE flag. If this flag is >>> set, we continue to check >>> link_speed. If not, this function is executed whether this link_speed is >>> unknown or not. >> I can already see this in your patch. I was asking about the reason why >>your change is needed. > >an extreme example, let us assume this scenario: Is this the scenario you are trying to fix? >An ixgbe NIC directly connects to another NIC (let us call it NIC-a). >And auto-negotiate is off while no static speed is set in the 2 NICs. The ixgbe driver does not support disabling auto-negotiation directly. The only time this is true is when the advertised speed is restricted, so the above scenario is not possible (you either have autoneg or advertised speed set) with the current driver. Is this example in theory or do you have your interface configured this way somehow? Thanks, Emil
On 12/31/2015 12:37 AM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Wednesday, December 30, 2015 12:20 AM >> 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 2/2] ixgbe: restrict synchronization >> of link_up and speed >> >> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >>>> -----Original Message----- >>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict >> synchronization >>>> of link_up and speed >>>> >>>> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict synchronization >>>> of >>>>>> link_up and speed >>>>>> >>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>>>> >>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>>>> important to synchronize link_up and link_speed, such as a bonding >>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent >> interface, >>>>>> it is not necessary to synchronize link_up and link_speed. That is, >>>>>> the time span between link_up and link_speed is acceptable. >>>>> What exactly do you mean by "time span between link_up and link_speed"? >>>> In the previous mail, I show you some ethtool logs. In these logs, there >>>> is some >>>> time with NIC up while speed is unknown. I think this "some time" is >>>> time span between >>>> link_up and link_speed. Please see the previous mail for details. >>> Was this when reporting the link state from check_link() (reading the >> LINKS >>> register) or reporting the adapter->link_speed? >>> >>>>> Where is it you think the de-synchronization occurs? >>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >>>> netdevice struct. >>>> Before we enter this function, we check IFF_SLAVE flag. If this flag is >>>> set, we continue to check >>>> link_speed. If not, this function is executed whether this link_speed is >>>> unknown or not. >>> I can already see this in your patch. I was asking about the reason why >>> your change is needed. >> an extreme example, let us assume this scenario: > Is this the scenario you are trying to fix? Sure. If IFF_SLAVE is checked, this scenario will not happen. Zhu Yanjun > >> An ixgbe NIC directly connects to another NIC (let us call it NIC-a). >> And auto-negotiate is off while no static speed is set in the 2 NICs. > The ixgbe driver does not support disabling auto-negotiation directly. > The only time this is true is when the advertised speed is restricted, > so the above scenario is not possible (you either have autoneg or > advertised speed set) with the current driver. > > Is this example in theory or do you have your interface configured this > way somehow? > > Thanks, > Emil > >
>-----Original Message----- >From: zhuyj [mailto:zyjzyj2000@gmail.com] >Sent: Tuesday, January 05, 2016 9:42 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 2/2] ixgbe: restrict synchronization >of link_up and speed > >On 12/31/2015 12:37 AM, Tantilov, Emil S wrote: >>> -----Original Message----- >>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>> Sent: Wednesday, December 30, 2015 12:20 AM >>> 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 2/2] ixgbe: restrict >synchronization >>> of link_up and speed >>> >>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >>>>> -----Original Message----- >>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>>> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict >>> synchronization >>>>> of link_up and speed >>>>> >>>>> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict >synchronization >>>>> of >>>>>>> link_up and speed >>>>>>> >>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>>>>> >>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>>>>> important to synchronize link_up and link_speed, such as a bonding >>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent >>> interface, >>>>>>> it is not necessary to synchronize link_up and link_speed. That is, >>>>>>> the time span between link_up and link_speed is acceptable. >>>>>> What exactly do you mean by "time span between link_up and >link_speed"? >>>>> In the previous mail, I show you some ethtool logs. In these logs, >there >>>>> is some >>>>> time with NIC up while speed is unknown. I think this "some time" is >>>>> time span between >>>>> link_up and link_speed. Please see the previous mail for details. >>>> Was this when reporting the link state from check_link() (reading the >>> LINKS >>>> register) or reporting the adapter->link_speed? >>>> >>>>>> Where is it you think the de-synchronization occurs? >>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >>>>> netdevice struct. >>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag >is >>>>> set, we continue to check >>>>> link_speed. If not, this function is executed whether this link_speed >is >>>>> unknown or not. >>>> I can already see this in your patch. I was asking about the reason why >>>> your change is needed. >>> an extreme example, let us assume this scenario: >> Is this the scenario you are trying to fix? >Sure. If IFF_SLAVE is checked, this scenario will not happen. I already explained why this is not a valid scenario, but if you were able to set it up somehow I'd like to know how you did it If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would be an issue regardless of whether the interface is a part of a bond or not, but you haven't provided any proof that this is the case. Do you have a dmesg log that shows ixgbe reporting unknown speed? Was your patch tested by the customer that reported this issue? Thanks, Emil
On 01/06/2016 11:30 PM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Tuesday, January 05, 2016 9:42 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 2/2] ixgbe: restrict synchronization >> of link_up and speed >> >> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote: >>>> -----Original Message----- >>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>> Sent: Wednesday, December 30, 2015 12:20 AM >>>> 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 2/2] ixgbe: restrict >> synchronization >>>> of link_up and speed >>>> >>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >>>>>> -----Original Message----- >>>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>>>> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict >>>> synchronization >>>>>> of link_up and speed >>>>>> >>>>>> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict >> synchronization >>>>>> of >>>>>>>> link_up and speed >>>>>>>> >>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>>>>>> >>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>>>>>> important to synchronize link_up and link_speed, such as a bonding >>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent >>>> interface, >>>>>>>> it is not necessary to synchronize link_up and link_speed. That is, >>>>>>>> the time span between link_up and link_speed is acceptable. >>>>>>> What exactly do you mean by "time span between link_up and >> link_speed"? >>>>>> In the previous mail, I show you some ethtool logs. In these logs, >> there >>>>>> is some >>>>>> time with NIC up while speed is unknown. I think this "some time" is >>>>>> time span between >>>>>> link_up and link_speed. Please see the previous mail for details. >>>>> Was this when reporting the link state from check_link() (reading the >>>> LINKS >>>>> register) or reporting the adapter->link_speed? >>>>> >>>>>>> Where is it you think the de-synchronization occurs? >>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >>>>>> netdevice struct. >>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag >> is >>>>>> set, we continue to check >>>>>> link_speed. If not, this function is executed whether this link_speed >> is >>>>>> unknown or not. >>>>> I can already see this in your patch. I was asking about the reason why >>>>> your change is needed. >>>> an extreme example, let us assume this scenario: >>> Is this the scenario you are trying to fix? >> Sure. If IFF_SLAVE is checked, this scenario will not happen. > I already explained why this is not a valid scenario, but if you were able > to set it up somehow I'd like to know how you did it If it is not a valid scenario, maybe there is something wrong with NIC driver/hardware. We should pay attention to it. Zhu Yanjun > > If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would > be an issue regardless of whether the interface is a part of a bond or not, > but you haven't provided any proof that this is the case. Do you have a > dmesg log that shows ixgbe reporting unknown speed? > > Was your patch tested by the customer that reported this issue? > > Thanks, > Emil >
On 01/06/2016 11:30 PM, Tantilov, Emil S wrote: >> -----Original Message----- >> From: zhuyj [mailto:zyjzyj2000@gmail.com] >> Sent: Tuesday, January 05, 2016 9:42 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 2/2] ixgbe: restrict synchronization >> of link_up and speed >> >> On 12/31/2015 12:37 AM, Tantilov, Emil S wrote: >>>> -----Original Message----- >>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>> Sent: Wednesday, December 30, 2015 12:20 AM >>>> 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 2/2] ixgbe: restrict >> synchronization >>>> of link_up and speed >>>> >>>> On 12/30/2015 02:55 PM, Tantilov, Emil S wrote: >>>>>> -----Original Message----- >>>>>> From: zhuyj [mailto:zyjzyj2000@gmail.com] >>>>>> Sent: Tuesday, December 29, 2015 6:49 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 2/2] ixgbe: restrict >>>> synchronization >>>>>> of link_up and speed >>>>>> >>>>>> On 12/30/2015 12:18 AM, 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: Monday, December 28, 2015 6:32 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 2/2] ixgbe: restrict >> synchronization >>>>>> of >>>>>>>> link_up and speed >>>>>>>> >>>>>>>> From: Zhu Yanjun <yanjun.zhu@windriver.com> >>>>>>>> >>>>>>>> When the X540 NIC acts as a slave of some virtual NICs, it is very >>>>>>>> important to synchronize link_up and link_speed, such as a bonding >>>>>>>> driver in 802.3ad mode. When X540 NIC acts as an independent >>>> interface, >>>>>>>> it is not necessary to synchronize link_up and link_speed. That is, >>>>>>>> the time span between link_up and link_speed is acceptable. >>>>>>> What exactly do you mean by "time span between link_up and >> link_speed"? >>>>>> In the previous mail, I show you some ethtool logs. In these logs, >> there >>>>>> is some >>>>>> time with NIC up while speed is unknown. I think this "some time" is >>>>>> time span between >>>>>> link_up and link_speed. Please see the previous mail for details. >>>>> Was this when reporting the link state from check_link() (reading the >>>> LINKS >>>>> register) or reporting the adapter->link_speed? >>>>> >>>>>>> Where is it you think the de-synchronization occurs? >>>>>> When a NIC interface acts as a slave, a flag "IFF_SLAVE" is set in >>>>>> netdevice struct. >>>>>> Before we enter this function, we check IFF_SLAVE flag. If this flag >> is >>>>>> set, we continue to check >>>>>> link_speed. If not, this function is executed whether this link_speed >> is >>>>>> unknown or not. >>>>> I can already see this in your patch. I was asking about the reason why >>>>> your change is needed. >>>> an extreme example, let us assume this scenario: >>> Is this the scenario you are trying to fix? >> Sure. If IFF_SLAVE is checked, this scenario will not happen. > I already explained why this is not a valid scenario, but if you were able > to set it up somehow I'd like to know how you did it Sorry. Why can we not disable auto-negotiate on ixgbe NIC ? To be honest, I am interested in it. This is related with hardware or driver? Thanks a lot. Zhu Yanjun > > If we are to enter ixgbe_watchdog_link_is_up() with unknown link this would > be an issue regardless of whether the interface is a part of a bond or not, > but you haven't provided any proof that this is the case. Do you have a > dmesg log that shows ixgbe reporting unknown speed? > > Was your patch tested by the customer that reported this issue? > > Thanks, > Emil > >
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index ace21b9..1bb6056 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -6436,8 +6436,15 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter) * time. To X540 NIC, there is a time span between link_up and * link_speed. As such, only continue if link_up and link_speed are * ready to X540 NIC. + * The time span between link_up and link_speed is very important + * when the X540 NIC acts as a slave in some virtual NICs, such as + * a bonding driver in 802.3ad mode. When X540 NIC acts as an + * independent interface, it is not necessary to synchronize link_up + * and link_speed. + * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN) */ - if (hw->mac.type == ixgbe_mac_X540) + if ((hw->mac.type == ixgbe_mac_X540) && + (netdev->flags & IFF_SLAVE)) if (link_speed == IXGBE_LINK_SPEED_UNKNOWN) return;