Date: Fri, 8 Dec 2017 11:45:52 +0100
From: Christian Hesse <list@eworm.de>
To: Benjamin Poirier <bpoirier@suse.com>
Cc: Ben Hutchings <ben.hutchings@codethink.co.uk>,
        Gabriel C <nix.or.die@gmail.com>,
        Jeff Kirsher <jeffrey.t.kirsher@intel.com>, stable@vger.kernel.org,
        Lennart Sorensen <lsorense@csclub.uwaterloo.ca>,
        Aaron Brown <aaron.f.brown@intel.com>,
        Amit Pundir <amit.pundir@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4.4 71/96] e1000e: Separate signaling for link
 check/link up
Message-ID: <20171208114552.5f517fd3@leda>
In-Reply-To: <20171208083441.kvyilu3hhf5pae3q@f1.synalogic.ca>
References: <20171128100503.067621614@linuxfoundation.org>
        <20171128100507.477626859@linuxfoundation.org>
        <1512676979.18523.193.camel@codethink.co.uk>
        <20171208083441.kvyilu3hhf5pae3q@f1.synalogic.ca>
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAGFBMVEUZFRFENy6KVTKEd23CiGHeqofJvrX4+vdHgItOAAAACXBIWXMAAA3XAAAN1wFCKJt4AAACUklEQVQ4y2VUTZeqMAxNxXG2Io5uGd64L35unbF9ax0b3OLxgFs4PcLff0lBHeb1QIq5uelNCEJNq/TIFGyeC+iugH0WJr+B1MvzWASpuP4CYHOB0VfoDdddwA7OIFQIEHjXDiCtV5e9QX0WMu8AG0mB7g7WP4GqeqVdsi4vv/5kFBvaF/zD7zDquL4DxbrDGDyAsgNYOsJOYzth4Q9ZF6iLV+6TLAT1pi2kuvgAtZxSjoG8cL+8vIn251uoe1OOEWwbIPU04gHsmMsoxyyhYsD2FdIigF1yxaVbBuSOCAlCoX324I7wNMhrO1bhOLsRoA6DC6wQ5eQiSG5BiWQfM4gN+uItQTRDMaJUhVbGyKWCuaaUGSVFVKpl4PdoDn3yY8J+YxQxyhlHfoYOyPgyDcO+cSQK6Bvabjcy2nwRo3pxgA8jslnCuYw23ESOzHAPYwo4ITNQMaOO+RGPEGhSlPEZBh2jmBEjQ5cKbxmr0ruAe/WCriUxW76I8T3h7vqY5VR5wXLdERodg2rHEzdxxk5KpXTL4FwnarvndKM5/MWDY5CuBBdQ+3/0ivsUJHicuHd+Xh3jOdBL+FjSGq4SPCwco+orpWlERRTNo7BHCvbNXFVSIQMp+P5QsIL9upmr8kMTUOfxEHoanwzKRcNAe76WbjBwex/RkdHu48xT5YqP70DaMOhBcTHmAVDxLaBdle93oJy1QKFUh2GXT4am+YH/GGel1CeI98GdMXsytjCKIq/9cMrlgxFCROv+3/BU1fijNpcVD6DxE8VfLBaxUGr1D5usgDYdjwiPAAAAAElFTkSuQmCC
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
 boundary="Sig_/NF2c8LkbJz3G4RXm/sAtqhU"; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4164
Lines: 104

--Sig_/NF2c8LkbJz3G4RXm/sAtqhU
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Benjamin Poirier <bpoirier@suse.com> on Fri, 2017/12/08 17:34:
> On 2017/12/07 20:02, Ben Hutchings wrote:
> > On Tue, 2017-11-28 at 11:23 +0100, Greg Kroah-Hartman wrote: =20
> > > 4.4-stable review patch.=C2=A0=C2=A0If anyone has any objections, ple=
ase let me
> > > know.
> > >=20
> > > ------------------
> > >=20
> > > From: Benjamin Poirier <bpoirier@suse.com>
> > >=20
> > > commit 19110cfbb34d4af0cdfe14cd243f3b09dc95b013 upstream. =20
> > [...] =20
> > > --- a/drivers/net/ethernet/intel/e1000e/mac.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/mac.c
> > > @@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e
> > > =C2=A0 *=C2=A0=C2=A0Checks to see of the link status of the hardware =
has changed.=C2=A0=C2=A0If a
> > > =C2=A0 *=C2=A0=C2=A0change in link status has been detected, then we =
read the PHY
> > > registers
> > > =C2=A0 *=C2=A0=C2=A0to get the current speed/duplex if link exists.
> > > + *
> > > + *=C2=A0=C2=A0Returns a negative error code (-E1000_ERR_*) or 0 (lin=
k down) or 1
> > > (link
> > > + *=C2=A0=C2=A0up).
> > > =C2=A0 **/
> > > =C2=A0s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
> > > =C2=A0{ =20
> > [...] =20
> > > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > > @@ -5017,7 +5017,7 @@ static bool e1000e_has_link(struct e1000 =20
> > > > =C2=A0	case e1000_media_type_copper:
> > > > =C2=A0		if (hw->mac.get_link_status) {
> > > > =C2=A0			ret_val =3D hw->mac.ops.check_for_link(hw);
> > > > -			link_active =3D !hw->mac.get_link_status;
> > > > +			link_active =3D ret_val > 0;
> > > > =C2=A0		} else {
> > > > =C2=A0			link_active =3D true;
> > > > =C2=A0		} =20
> >=20
> > As this change in e1000e_has_link() is conditional only on the media
> > type, doesn't e1000_check_for_copper_link_ich8lan() also need to be
> > changed to return 1 for link up? =20
>=20
> You're right. I looked at it again, in the commit log I wrote that
> "hw->mac.ops.check_for_link(hw) =3D=3D=3D e1000e_check_for_copper_link" w=
hich
> is true for the race condition reported (because that's the function in
> use on adapters that have msix vectors mac.type =3D=3D e1000_82574) but n=
ot
> generally true. The other check_for_link callback needs to be adjusted
> likewise.
>=20
> However, I happen to have a I218-LM (e1000_pch_lpt) so I tested 4.14.3
> and this error only delays link up, it doesn't prevent it.
> e1000_check_for_copper_link_ich8lan() sets mac->get_link_status =3D false;
> and on the next watchdog execution, we fall in the second branch of the
> following e1000e_has_link code:
>=20
> 	case e1000_media_type_copper:
> 		if (hw->mac.get_link_status) {
> 			ret_val =3D hw->mac.ops.check_for_link(hw);
> 			link_active =3D ret_val > 0;
> 		} else {
> 			link_active =3D true;
>=20
> OTOH, there are multiple reports in
> https://bugzilla.kernel.org/show_bug.cgi?id=3D198047
> that reverting 830466993daf ("e1000e: Separate signaling for link
> check/link up") fixes the issue so there's something I'm missing.
>=20
> Gabriel and Christian, can you test the following patch?

With this patch applied my connection is up and running again. Thanks!
--=20
main(a){char*c=3D/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=3D0;b=3Dc[a+=
+];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

--Sig_/NF2c8LkbJz3G4RXm/sAtqhU
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEXHmveYAHrRp+prOviUUh18yA9HYFAloqbWAACgkQiUUh18yA
9HZ3gQf/ddbnlCjgw7cYSAyekNC5FN4rgKX6ThMzPF4nTg0UxHVTw7XPCtmgqV71
yNz38vmkuvKT0qppHV7fcuq7uPph/2L0W0uH2lATochwpG3z1i0d3Yn9d8e4Oz6m
Bla8h7Pw1UBMYmB7rvp7DFCXXylzDsJDnL9jWUPe3Io1tOtcYP2qsTwygX9R92+r
Lpg6E4f+/5hmDuttCEdaSzHap/Sc3g8vHGICRSJVfcY7ZVMvilwq/s90m/XVhGG6
6ZOS8oncdTkp7JUBXIFnxabqV3LFjWvUWuNfD6PoWAjqS9sOYx4t6R8iETu7EelG
jyE8ccDT701L/dy4ds6OtYohzkk7Iw==
=gneF
-----END PGP SIGNATURE-----

--Sig_/NF2c8LkbJz3G4RXm/sAtqhU--