netdev - Re: [PATCHv2] 3c59x: fix missing dma_mapping_error check and bad ring refill logic

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180103.102831.1474605321968855786.davem@davemloft.net>
Date:   Wed, 03 Jan 2018 10:28:31 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     nhorman@...hat.com
Cc:     nhorman@...driver.com, netdev@...r.kernel.org,
        tedheadster@...il.com, klassert@...hematik.tu-chemnitz.de
Subject: Re: [PATCHv2] 3c59x: fix missing dma_mapping_error check and bad
 ring refill logic

From: David Miller <davem@...emloft.net>
Date: Wed, 03 Jan 2018 10:26:06 -0500 (EST)

> From: Neil Horman <nhorman@...hat.com>
> Date: Wed, 3 Jan 2018 10:13:33 -0500
> 
>> Thats exactly what this patch does, instead of creating a second loop to
>> traverse all the emptied ring buffers, now I:
>> 
>> 1) Pre-allocate a new skb when I know I'm going to receive the in-place skb
>> 2) Map the skb into the appropriate dma device domain
>> 3) If (1) and (2) succede, then I swap the newly allocate skb and dma address
>> with the old one and recieve the old into the network stack
>> 4) If (1) or (2) fail, then I goto clear_complete, which leaves the old skb and
>> dma address in place, sets the buffer status back to 0 (indicating completion),
>> and write the new ring status back to the hardware
>> 
>> This is what you wanted, a pre-allocate and swap-if-successful, recycle-if-not
>> approach, rather than the leave-a-hole-in-the-ring approach that is there
>> currently, no?  Or did I miss something else?
> 
> I misread the code sorry, you're absolutely right.
> 
> I'll apply this patch, thanks Neil. :)

Hmmm, maybe we need a V3 after all :)

  CC [M]  drivers/net/ethernet/3com/3c59x.o
drivers/net/ethernet/3com/3c59x.c: In function ‘boomerang_rx’:
drivers/net/ethernet/3com/3c59x.c:2605:13: warning: unused variable ‘dma’ [-Wunused-variable]
  dma_addr_t dma;
             ^~~