netdev - Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.

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, 21 Nov 2014 06:49:42 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	"Keller, Jacob E" <jacob.e.keller@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Vick, Matthew" <matthew.vick@...el.com>
Subject: Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.

On Thu, Nov 20, 2014 at 09:28:26PM +0000, Keller, Jacob E wrote:
> Here is the problem. In igb_reset, we call e1000_reset_hw() which will
> eventually perform a hardware MAC reset. Yes maybe we shouldn't be
> calling igb_reset everywhere.. I cannot answer that, however...

It is a typical device driver "code smell". The developer thinks, 
"Hm, something isn't working in this function. I know - lets reset the
device. That _always_ works."

> Some of the ethtool settings maybe could avoid resets, but I am not
> certain during exactly which path they end up calling resets.

Jup. Surely changing the MTU is no reason to reset the PTP clock. But
as you said, fixing the reset stuff is a huge amount of work. Instead
of trying to do that, I'll just go with the flow and reset the
auxiliary functions, too.

Thanks,
Richard

--
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