[PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Javier Martinez Canillas]

This flag is scheduled for removal so we shouldn't used it.

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
v2: Initialize flags to zero suggested by Dan Carpenter.

 drivers/net/tg3.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5135655..7adbf63 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8839,12 +8839,12 @@ static int tg3_request_irq(struct tg3 *tp, int irq_num)
 		fn = tg3_msi;
 		if (tp->tg3_flags2 & TG3_FLG2_1SHOT_MSI)
 			fn = tg3_msi_1shot;
-		flags = IRQF_SAMPLE_RANDOM;
+		flags = 0;
 	} else {
 		fn = tg3_interrupt;
 		if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS)
 			fn = tg3_interrupt_tagged;
-		flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
+		flags = IRQF_SHARED;
 	}
 
 	return request_irq(tnapi->irq_vec, fn, flags, name, tnapi);
@@ -8875,7 +8875,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
 	}
 
 	err = request_irq(tnapi->irq_vec, tg3_test_isr,
-			  IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, tnapi);
+			  IRQF_SHARED, dev->name, tnapi);
 	if (err)
 		return err;
 
-- 
1.7.2.5

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Eric Dumazet]

Le lundi 28 mars 2011 à 14:15 +0200, Javier Martinez Canillas a écrit :
> This flag is scheduled for removal so we shouldn't used it.
> 
> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
> ---
> v2: Initialize flags to zero suggested by Dan Carpenter.
> 
>  drivers/net/tg3.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

1) We dont believe its Janitor material ;)

http://thread.gmane.org/gmane.linux.kernel/680723

http://kerneltrap.org/mailarchive/linux-netdev/2009/4/6/5417754

2) If removal is accepted, we should patch all drivers at once

drivers/net/xen-netfront.c:1419:					IRQF_SAMPLE_RANDOM, netdev->name,
drivers/net/cris/eth_v10.c:495:			IRQF_SAMPLE_RANDOM, cardname, (void *)dev)) {
drivers/net/ibmlana.c:785:	result = request_irq(priv->realirq, irq_handler, IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev);
drivers/net/qla3xxx.c:3471:	unsigned long irq_flags = IRQF_SAMPLE_RANDOM | IRQF_SHARED;
drivers/net/netxen/netxen_nic_main.c:908:	unsigned long flags = IRQF_SAMPLE_RANDOM;
drivers/net/atlx/atl1.c:2575:	int irq_flags = IRQF_SAMPLE_RANDOM;
drivers/net/macb.c:1174:	err = request_irq(dev->irq, macb_interrupt, IRQF_SAMPLE_RANDOM,
drivers/net/bcm63xx_enet.c:843:			  IRQF_SAMPLE_RANDOM | IRQF_DISABLED, dev->name, dev);
drivers/net/niu.c:6075:				  IRQF_SHARED | IRQF_SAMPLE_RANDOM,
drivers/net/tg3.c:8842:		flags = IRQF_SAMPLE_RANDOM;

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Javier Martinez Canillas]

On Mon, Mar 28, 2011 at 2:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> 1) We dont believe its Janitor material ;)
>
> http://thread.gmane.org/gmane.linux.kernel/680723
>
> http://kerneltrap.org/mailarchive/linux-netdev/2009/4/6/5417754
>

I wasn't aware of this discussion. In one hand network drivers are not
a good source of entropy because they can be controlled externally,
but in embedded systems with only network cards (no video, audio,
keyboard, etc) the only source of entropy they have is their network
cards (at the kernel level i.e: not using EGD to feed /dev/random).

Yes this definitely is not janitor material :)

I just sent the patch because I saw IRQF_SAMPLE_RANDOM in
Documentation/feature-removal-schedule.txt. I can resend a patch
removing the macro in the remaining network cards if the decision is
to remove IRQF_SAMPLE_RANDOM.

Best regards,

-----------------------------------------
Javier Martínez Canillas
(+34) 682 39 81 69
PhD Student in High Performance Computing
Computer Architecture and Operating System Department (CAOS)
Universitat Autònoma de Barcelona
Barcelona, Spain

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Ben Hutchings]

On Mon, 2011-03-28 at 17:46 +0200, Javier Martinez Canillas wrote:
> On Mon, Mar 28, 2011 at 2:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > 1) We dont believe its Janitor material ;)
> >
> > http://thread.gmane.org/gmane.linux.kernel/680723
> >
> > http://kerneltrap.org/mailarchive/linux-netdev/2009/4/6/5417754
> >
> 
> I wasn't aware of this discussion. In one hand network drivers are not
> a good source of entropy because they can be controlled externally,
> but in embedded systems with only network cards (no video, audio,
> keyboard, etc) the only source of entropy they have is their network
> cards (at the kernel level i.e: not using EGD to feed /dev/random).

It may be the only source of entropy, but given how poor a source it is
these drivers are basically telling sweet little lies to the kernel and
the applications that demand real random numbers.

This also applies to many servers just as much as embedded systems.

As you acknowledge, these systems can still get entropy using the EGD
protocol over a secure channel.  (Or an entropy sampling device such as
the Entropy Key.)

> Yes this definitely is not janitor material :)
> 
> I just sent the patch because I saw IRQF_SAMPLE_RANDOM in
> Documentation/feature-removal-schedule.txt. I can resend a patch
> removing the macro in the remaining network cards if the decision is
> to remove IRQF_SAMPLE_RANDOM.

It's not my call, but I would support it.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Florian Fainelli]

Hello,

On Monday 28 March 2011 19:20:22 Ben Hutchings wrote:
> On Mon, 2011-03-28 at 17:46 +0200, Javier Martinez Canillas wrote:
> > On Mon, Mar 28, 2011 at 2:25 PM, Eric Dumazet <eric.dumazet@gmail.com> 
wrote:
> > > 1) We dont believe its Janitor material ;)
> > > 
> > > http://thread.gmane.org/gmane.linux.kernel/680723
> > > 
> > > http://kerneltrap.org/mailarchive/linux-netdev/2009/4/6/5417754
> > 
> > I wasn't aware of this discussion. In one hand network drivers are not
> > a good source of entropy because they can be controlled externally,
> > but in embedded systems with only network cards (no video, audio,
> > keyboard, etc) the only source of entropy they have is their network
> > cards (at the kernel level i.e: not using EGD to feed /dev/random).
> 
> It may be the only source of entropy, but given how poor a source it is
> these drivers are basically telling sweet little lies to the kernel and
> the applications that demand real random numbers.
> 
> This also applies to many servers just as much as embedded systems.

Indeed, this is the reason why bcm63xx_enet.c for instance contributes to the 
general entropy of the system, because it is used on MIPS based ADSL routers 
where we have little or no other source of entropy.

Still even poor entropy is better than none on such devices imho.

> 
> As you acknowledge, these systems can still get entropy using the EGD
> protocol over a secure channel.  (Or an entropy sampling device such as
> the Entropy Key.)



> 
> > Yes this definitely is not janitor material :)
> > 
> > I just sent the patch because I saw IRQF_SAMPLE_RANDOM in
> > Documentation/feature-removal-schedule.txt. I can resend a patch
> > removing the macro in the remaining network cards if the decision is
> > to remove IRQF_SAMPLE_RANDOM.
> 
> It's not my call, but I would support it.
> 
> Ben.

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Ben Hutchings]

On Mon, 2011-03-28 at 22:08 +0200, Florian Fainelli wrote:
> Hello,
> 
> On Monday 28 March 2011 19:20:22 Ben Hutchings wrote:
> > On Mon, 2011-03-28 at 17:46 +0200, Javier Martinez Canillas wrote:
> > > On Mon, Mar 28, 2011 at 2:25 PM, Eric Dumazet <eric.dumazet@gmail.com> 
> wrote:
> > > > 1) We dont believe its Janitor material ;)
> > > > 
> > > > http://thread.gmane.org/gmane.linux.kernel/680723
> > > > 
> > > > http://kerneltrap.org/mailarchive/linux-netdev/2009/4/6/5417754
> > > 
> > > I wasn't aware of this discussion. In one hand network drivers are not
> > > a good source of entropy because they can be controlled externally,
> > > but in embedded systems with only network cards (no video, audio,
> > > keyboard, etc) the only source of entropy they have is their network
> > > cards (at the kernel level i.e: not using EGD to feed /dev/random).
> > 
> > It may be the only source of entropy, but given how poor a source it is
> > these drivers are basically telling sweet little lies to the kernel and
> > the applications that demand real random numbers.
> > 
> > This also applies to many servers just as much as embedded systems.
> 
> Indeed, this is the reason why bcm63xx_enet.c for instance contributes to the 
> general entropy of the system, because it is used on MIPS based ADSL routers 
> where we have little or no other source of entropy.
> 
> Still even poor entropy is better than none on such devices imho.
[...]

Well, only if it's treated as such.

add_timer_randomness() does try to measure how much entropy it's getting
from the time, which should address the problem of interrupt moderation
causing predictable interrupts.  However I strongly suspect that its
tests cannot protect against deliberately timed packets, particularly if
get_cycles() has low resolution.  (On MIPS, get_cycles() always returns
0, although the comment before suggests that this is possible to do
better.)

I notice that there is an internal option 'dont_count_entropy' which
would allow interrupts to add entropy without it increasing the
estimated total entropy in the pool.  If that was set for network
interrupts, they could be used to improve the quality of /dev/urandom,
get_random_bits(), etc. without compromising applications that demand
real randomness.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 28 Mar 2011 18:20:22 +0100

> On Mon, 2011-03-28 at 17:46 +0200, Javier Martinez Canillas wrote:
>> Yes this definitely is not janitor material :)
>> 
>> I just sent the patch because I saw IRQF_SAMPLE_RANDOM in
>> Documentation/feature-removal-schedule.txt. I can resend a patch
>> removing the macro in the remaining network cards if the decision is
>> to remove IRQF_SAMPLE_RANDOM.
> 
> It's not my call, but I would support it.

FWIW, I support it too.

Re: [PATCH 2/2 v2] tg3: Don't use IRQF_SAMPLE_RANDOM

[Javier Martinez Canillas]

On Tue, Mar 29, 2011 at 1:53 AM, David Miller <davem@davemloft.net> wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 28 Mar 2011 18:20:22 +0100
>
>> On Mon, 2011-03-28 at 17:46 +0200, Javier Martinez Canillas wrote:
>>> Yes this definitely is not janitor material :)
>>>
>>> I just sent the patch because I saw IRQF_SAMPLE_RANDOM in
>>> Documentation/feature-removal-schedule.txt. I can resend a patch
>>> removing the macro in the remaining network cards if the decision is
>>> to remove IRQF_SAMPLE_RANDOM.
>>
>> It's not my call, but I would support it.
>
> FWIW, I support it too.
>

Ok, I just sent a patch removing the IRQF_SAMPLE_RANDOM flag in all
the network drivers in the case everyone agrees.

Best regards,

-----------------------------------------
Javier Martínez Canillas
(+34) 682 39 81 69
PhD Student in High Performance Computing
Computer Architecture and Operating System Department (CAOS)
Universitat Autònoma de Barcelona
Barcelona, Spain