linux-kernel - Re: [PATCH net-next v1 3/4] net-e1000e: reworked carrier detection logic

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHo-OozdBn5jBNsRVzbCnmxozquJnTAYa0bNbK5VoaiKaK=7qQ@mail.gmail.com>
Date:	Tue, 22 Nov 2011 12:32:45 -0800
From:	Maciej Żenczykowski <zenczykowski@...il.com>
To:	David Decotigny <david.decotigny@...gle.com>
Cc:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Bruce Allan <bruce.w.allan@...el.com>,
	Carolyn Wyborny <carolyn.wyborny@...el.com>,
	Don Skidmore <donald.c.skidmore@...el.com>,
	Greg Rose <gregory.v.rose@...el.com>,
	Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@...el.com>,
	Alex Duyck <alexander.h.duyck@...el.com>,
	John Ronciak <john.ronciak@...el.com>,
	e1000-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Ian Campbell <ian.campbell@...rix.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: Re: [PATCH net-next v1 3/4] net-e1000e: reworked carrier detection logic

David,

I know you asked for a review of this on Friday, before you sent this
out to netdev, and I was planning on doing so, but I see you've sent
it out to a wider audience now.
So, I'll attach comments directly here.

You took my patch (which was for 2.6.34) and split it in two (it was
erroneously marked author zenczykowski@...il.com instead of
maze@...gle.com, so that should be fixed) and while you seem to have
reduced the amount of lines-of-code-churn, I'm not sure you've
actually made the resulting code (as a whole) more readable.

I'm not sure how much of these changes were a result of changes
elsewhere in the driver between 2.6.34 and now.

This was very tricky code to get right, and I'm not sure whether the
patch as you posted it gets all the corner cases right...
Indeed I actually think the changes you've made reduce the robustness
wrt. timeouts caused by firmware concurrent accesses.  In particular
the first read access you make is very likely to fail.
I think renaming success to link and iterations to intervals was
worthwhile since it improved readability...

For reference, the original patch, cherrypicked onto net-next and
compile (but not anything else) tested will be a reply to this email.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/