netdev - Re: [E1000-devel] [PATCH] Fix link test for e1000 and e1000e when iface is down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 13 Feb 2009 04:52:52 -0800
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	André Detsch <adetsch@...ibm.com>
Cc:	e1000-devel@...ts.sourceforge.net, netdev@...r.kernel.org
Subject: Re: [E1000-devel] [PATCH] Fix link test for e1000 and e1000e when 
	iface is down

On Fri, Feb 13, 2009 at 4:08 AM, André Detsch <adetsch@...ibm.com> wrote:
> Jeff Kirsher wrote:
>>
>> I guess this comes down to what the definition of a Link test should
>> be doing.  My $0.02 is that it should be testing if the interface has
>> a link, in which case if you ifdown the interface before running the
>> Link test, I would expect it to fail.
>>
>> With this patch, if you bring down the device and run the ethtool diag
>> tests, the Link test would come back as passing which is something I
>> would not expect.
>
> With the current code, e1000_reset is called during the test anyway, so
> ethtool -t fails only the first time it is run (after ifconfig down). Is
> that the expected behavior for the test?
>
>
> # ifconfig eth0 up; ifconfig eth0 down; ethtool -t eth0
>
> The test result is FAIL
> The test extra info:
> Register test  (offline)         0
> Eeprom test    (offline)         0
> Interrupt test (offline)         0
> Loopback test  (offline)         0
> Link test   (on/offline)         1
>
>
> # ethtool -t eth0
>
> The test result is PASS
> The test extra info:
> Register test  (offline)         0
> Eeprom test    (offline)         0
> Interrupt test (offline)         0
> Loopback test  (offline)         0
> Link test   (on/offline)         0
>
>
> Thanks for the feedback.
>
> --
> André Detsch
> Kernel Software Engineer
> Linux Technology Center Brazil
> --

Yes, this is expected.

We want the Link test to correctly report whether or not the interface
currently has link, whether or not the user manually downed the
interface.  The reason for resetting is because during the testing,
the hardware settings may be changed to complete such tests as the
Loopback and Interrupt tests so we need to reset the hardware settings
to normal operation.  It has the added benefit, that if the user had
not manually downed the interface, and the interface had gone down for
some unknown reason, the reset attempts to bring the interface back
up.

For an example, if the link went down for some reason and the user had
not downed the interface manually and were to run the ethtool tests to
see what was wrong.  Your patch would report the link test as a pass
(if after the reset, the link was re-established).  This would have
falsely reported the Link status when the ethtool tests were
initiated.

-- 
Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html