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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 25 Jul 2016 10:56:48 -0700
From:	Jesse Brandeburg <jesse.brandeburg@...el.com>
To:	Jarod Wilson <jarod@...hat.com>
Cc:	<linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, jesse.brandeburg@...el.com,
	raanan.avargil@...el.com
Subject: Re: [Intel-wired-lan] [PATCH v2 net-next 0/2] e1000e: fix PTP on
 e1000_pch_variants

On Sat, 23 Jul 2016 12:44:32 -0400
Jarod Wilson <jarod@...hat.com> wrote:

> This little series factors out the systim sanitization code first, then
> adds e1000_pch_lpt as a new case in the switch that calls the sanitize
> function, fixing PTP clock issues I've had reported against an Intel
> I-218V NIC in an Intel NUC5ik5RYH system.
> 
> Jarod Wilson (2):
>   e1000e: factor out systim sanitization
>   e1000e: fix PTP on e1000_pch_lpt variants
> 

Thanks for your patch Jarod, the refactor itself is fine and a good
idea, and thanks for working on the fix!

This code should have been using a feature flag, and the alert that
you're having to add more device IDs to the switch statement makes it
even more obvious.

Please see line 406 of e1000e/e1000.h, where the flags are declared,
add a flag for this workaround (to flags2), and and add some code in the
e1000_info_tbl entry to set the flag for the appropriate mac(s)

Then the runtime code should only check the flag, and if any further
devices require the workaround we just add the flag to that device, or
if this is init code, just always call the workaround funtion and have
the function itself make sure the right flags2 is set or return.

The code has somehow gotten away from this model in some places and any
new code we add should be doing it the right way.

Thanks,
 Jesse

Powered by blists - more mailing lists