netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
@ 2010-10-09 23:45 Patrick Simmons
  2010-10-10  1:09 ` Ben Hutchings
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick Simmons @ 2010-10-09 23:45 UTC (permalink / raw)
  To: netdev

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, 
allowing the interrupt timing for forcedeth to be used for entropy 
generation.  This should help /dev/random generate more secure random 
numbers on machines using this driver.

Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>

Please CC me with any comments as I am not subscribed to the list.

--- linux/drivers/net/forcedeth.c.orig    2010-10-09 17:12:08.400000015 
-0600
+++ linux/drivers/net/forcedeth.c    2010-10-09 17:14:44.880000015 -0600
@@ -3819,7 +3819,7 @@ static int nv_request_irq(struct net_dev
                  /* Request irq for rx handling */
                  sprintf(np->name_rx, "%s-rx", dev->name);
                  if 
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector,
-                        nv_nic_irq_rx, IRQF_SHARED, np->name_rx, dev) 
!= 0) {
+                        nv_nic_irq_rx, IRQF_SHARED | 
IRQF_SAMPLE_RANDOM, np->name_rx, dev) != 0) {
                      printk(KERN_INFO "forcedeth: request_irq failed 
for rx %d\n", ret);
                      pci_disable_msix(np->pci_dev);
                      np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3828,7 +3828,7 @@ static int nv_request_irq(struct net_dev
                  /* Request irq for tx handling */
                  sprintf(np->name_tx, "%s-tx", dev->name);
                  if 
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector,
-                        nv_nic_irq_tx, IRQF_SHARED, np->name_tx, dev) 
!= 0) {
+                        nv_nic_irq_tx, IRQF_SHARED | 
IRQF_SAMPLE_RANDOM, np->name_tx, dev) != 0) {
                      printk(KERN_INFO "forcedeth: request_irq failed 
for tx %d\n", ret);
                      pci_disable_msix(np->pci_dev);
                      np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3837,7 +3837,7 @@ static int nv_request_irq(struct net_dev
                  /* Request irq for link and timer handling */
                  sprintf(np->name_other, "%s-other", dev->name);
                  if 
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector,
-                        nv_nic_irq_other, IRQF_SHARED, np->name_other, 
dev) != 0) {
+                        nv_nic_irq_other, IRQF_SHARED | 
IRQF_SAMPLE_RANDOM, np->name_other, dev) != 0) {
                      printk(KERN_INFO "forcedeth: request_irq failed 
for link %d\n", ret);
                      pci_disable_msix(np->pci_dev);
                      np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3851,7 +3851,7 @@ static int nv_request_irq(struct net_dev
                  set_msix_vector_map(dev, NV_MSI_X_VECTOR_OTHER, 
NVREG_IRQ_OTHER);
              } else {
                  /* Request irq for all interrupts */
-                if 
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, 
IRQF_SHARED, dev->name, dev) != 0) {
+                if 
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, 
IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
                      printk(KERN_INFO "forcedeth: request_irq failed 
%d\n", ret);
                      pci_disable_msix(np->pci_dev);
                      np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3868,7 +3868,7 @@ static int nv_request_irq(struct net_dev
          if ((ret = pci_enable_msi(np->pci_dev)) == 0) {
              np->msi_flags |= NV_MSI_ENABLED;
              dev->irq = np->pci_dev->irq;
-            if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED, 
dev->name, dev) != 0) {
+            if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED | 
IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
                  printk(KERN_INFO "forcedeth: request_irq failed %d\n", 
ret);
                  pci_disable_msi(np->pci_dev);
                  np->msi_flags &= ~NV_MSI_ENABLED;
@@ -3884,7 +3884,7 @@ static int nv_request_irq(struct net_dev
          }
      }
      if (ret != 0) {
-        if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED, 
dev->name, dev) != 0)
+        if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED | 
IRQF_SAMPLE_RANDOM, dev->name, dev) != 0)
              goto out_err;

      }


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
  2010-10-09 23:45 [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth Patrick Simmons
@ 2010-10-10  1:09 ` Ben Hutchings
  2010-10-10  3:15   ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2010-10-10  1:09 UTC (permalink / raw)
  To: Patrick Simmons; +Cc: netdev

Patrick Simmons wrote:
> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, 
> allowing the interrupt timing for forcedeth to be used for entropy 
> generation.  This should help /dev/random generate more secure random 
> numbers on machines using this driver.
[...]

We don't enable this for network drivers any more because:

1. At high packet rates, interrupt moderation makes interrupts very
regular.
2. At low packet rates, a malicious sender can control the interrupt
timing.

Ben.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
  2010-10-10  1:09 ` Ben Hutchings
@ 2010-10-10  3:15   ` David Miller
  2010-10-10  3:23     ` Patrick Simmons
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2010-10-10  3:15 UTC (permalink / raw)
  To: bhutchings; +Cc: linuxrocks123, netdev

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Sun, 10 Oct 2010 02:09:24 +0100

> Patrick Simmons wrote:
>> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, 
>> allowing the interrupt timing for forcedeth to be used for entropy 
>> generation.  This should help /dev/random generate more secure random 
>> numbers on machines using this driver.
> [...]
> 
> We don't enable this for network drivers any more because:
> 
> 1. At high packet rates, interrupt moderation makes interrupts very
> regular.
> 2. At low packet rates, a malicious sender can control the interrupt
> timing.

Agreed on all counts, I'm not applying this patch.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
  2010-10-10  3:15   ` David Miller
@ 2010-10-10  3:23     ` Patrick Simmons
  2010-10-10  8:57       ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Patrick Simmons @ 2010-10-10  3:23 UTC (permalink / raw)
  To: David Miller; +Cc: bhutchings, netdev

On 10/09/10 21:15, David Miller wrote:
> From: Ben Hutchings<bhutchings@solarflare.com>
> Date: Sun, 10 Oct 2010 02:09:24 +0100
>
>> Patrick Simmons wrote:
>>> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
>>> allowing the interrupt timing for forcedeth to be used for entropy
>>> generation.  This should help /dev/random generate more secure random
>>> numbers on machines using this driver.
>> [...]
>>
>> We don't enable this for network drivers any more because:
>>
>> 1. At high packet rates, interrupt moderation makes interrupts very
>> regular.
>> 2. At low packet rates, a malicious sender can control the interrupt
>> timing.
>
> Agreed on all counts, I'm not applying this patch.

It's enabled for other network drivers, which is where I got the idea 
from.  Has anyone actually done an experiment to see whether these two 
concerns are valid?

--Patrick

-- 
If I'm not here, I've gone out to find myself.  If I get back before I 
return, please keep me here.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth
  2010-10-10  3:23     ` Patrick Simmons
@ 2010-10-10  8:57       ` Eric Dumazet
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2010-10-10  8:57 UTC (permalink / raw)
  To: Patrick Simmons; +Cc: David Miller, bhutchings, netdev

Le samedi 09 octobre 2010 à 21:23 -0600, Patrick Simmons a écrit :
> On 10/09/10 21:15, David Miller wrote:
> > From: Ben Hutchings<bhutchings@solarflare.com>
> > Date: Sun, 10 Oct 2010 02:09:24 +0100
> >
> >> Patrick Simmons wrote:
> >>> This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver,
> >>> allowing the interrupt timing for forcedeth to be used for entropy
> >>> generation.  This should help /dev/random generate more secure random
> >>> numbers on machines using this driver.
> >> [...]
> >>
> >> We don't enable this for network drivers any more because:
> >>
> >> 1. At high packet rates, interrupt moderation makes interrupts very
> >> regular.
> >> 2. At low packet rates, a malicious sender can control the interrupt
> >> timing.
> >
> > Agreed on all counts, I'm not applying this patch.
> 
> It's enabled for other network drivers, which is where I got the idea 
> from.  Has anyone actually done an experiment to see whether these two 
> concerns are valid?

Several attemps in the past tried to go into one direction or another

(Add the flag to some driver, then remove it from others)

Please read commit 9d9b8fb0e5ebf4b0398e579
http://lkml.org/lkml/2009/4/6/283

A third reason not adding is : At moderate packet rates, _no_ entropy is
feeded at all because add_interrupt_randomness()/add_timer_randomness is
_very_ conservative, with first, second-order and third-order estimates.

credit_entropy_bits() is called with 0 bit

Adding this stuff has a high cost, I can see it in profiles on machines
with tg3 nics. I often remove the IRQF_SAMPLE_RANDOM flag localy.




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-10-10  8:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-09 23:45 [PATCH] Add IRQF_SAMPLE_RANDOM Flag to forcedeth Patrick Simmons
2010-10-10  1:09 ` Ben Hutchings
2010-10-10  3:15   ` David Miller
2010-10-10  3:23     ` Patrick Simmons
2010-10-10  8:57       ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).