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: <20180103164129.GC18309@hmswarspite.think-freely.org>
Date:   Wed, 3 Jan 2018 11:41:29 -0500
From:   Neil Horman <nhorman@...driver.com>
To:     David Miller <davem@...emloft.net>
Cc:     nhorman@...hat.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

On Wed, Jan 03, 2018 at 10:28:31AM -0500, David Miller wrote:
> 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;
>              ^~~
Thats....odd, I built it twice here, and it didn't bomb out.  I must not have
-Werror enabled, apologies.  I'll respin
Neil