Subject: Re: [PATCH] forcedeth: new backoff implementation

From: Ayaz Abdulla <[email-masked]>
Date: Tue, 08 Apr 2008 15:13:07 -0400

Thanks for the review. I will update the patch and resend it.

Manfred Spraul wrote:
> Andrew Morton wrote:
> 
>> On Mon, 07 Apr 2008 17:40:28 -0400 Ayaz Abdulla <[email-masked]> 
>> wrote:
>>  
>>
>>> +static void nv_gear_backoff_reseed(struct net_device *dev)
>>> +{
>>> +    u8 __iomem *base = get_hwbase(dev);
>>> +    u32 miniseed1, miniseed2, miniseed2_reversed, miniseed3, 
>>> miniseed3_reversed;
>>> +    u32 temp, seedset, combinedSeed;
>>> +    int i;
>>> +
>>> +    /* Setup seed for free running LFSR */
>>> +    /* We are going to read the time stamp counter 3 times and 
>>> swizzle bits around to increase randomness */
>>>     
>>
>>
>> I see this driver rigorously observes the 800-column-xterm convention.
>>
>>  
>>
>>> +    rdtscl(miniseed1);
>>>     
>>
>>
>> err, no.  We don't have sufficient Kconfig dependencies in place to be 
>> able
>> to do this.  You just broke all non-x86.
>>
>> Suitable fixes would be
>>
>> a) add #ifdef CONFIG_X86 all over the place
>>
>> b) use do_gettimeofday() (might be tricky if called from interrupt)
>>
>> c) use cpu_clock().
>>
>>   
> 
> d) use get_random_bytes()?
> It's irq-safe, used e.g. by the network code for the TCP sequence numbers.
> 
>>> +    /* Ensure seeds are not the same */
>>> +    if ((combinedSeed & NVREG_BKOFFCTRL_SEED_MASK) ==
>>> +        (combinedSeed & (NVREG_BKOFFCTRL_SEED_MASK << 
>>> NVREG_BKOFFCTRL_GEAR)))
>>> +        combinedSeed = 0x3FF3FF;
>>> +
>>>     
> 
> I don't understand this block: The seed consists of two parts, each 12 
> bits.
> Both parts mustn't be identical. If both parts are identical, then both 
> are set to 0x3ff.
> But if both are 0x3ff, then they are identical. Is that intentional?
> 
> -- 
>    Manfred

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------