netdev - RE: [E1000-devel] [RFC 2/2] igb: skb recycling

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

Message-ID: <80769D7B14936844A23C0C43D9FBCF0F209BBD1C@orsmsx501.amr.corp.intel.com>
Date:	Fri, 24 Oct 2008 12:01:19 -0700
From:	"Duyck, Alexander H" <alexander.h.duyck@...el.com>
To:	"Duyck, Alexander H" <alexander.h.duyck@...el.com>,
	Stephen Hemminger <shemminger@...tta.com>,
	"jeffery.t.kirsher@...el.com" <jeffery.t.kirsher@...el.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
CC:	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [E1000-devel] [RFC 2/2] igb: skb recycling

Duyck, Alexander H wrote:
> Stephen Hemminger wrote:
>> This driver is multiqueue, so implement a small skb recycling queue
>> per cpu. It doesn't make sense to have a global queue since then a
>> lock would be required. Not sure if this is going to work; compile
>> tested only, needs more evaluation
> 
> I did a bit of quick testing and found an issue in the sections of
> code below.  As is the current patch generates a null pointer
> exception on unload because the percpu_alloc and
> for_each_possible_cpu are working off of two different CPU masks.   
> 
> The simple solution is to either replace percpu_alloc with
> alloc_percpu and use cpu_possible_map or replace
> for_each_possible_cpu with for_each_online_cpu and use the
> cpu_online_map.   
> 
> Attached is the version I tested at my desk that used the
> cpu_possible_map solution.  I went that route since alloc_percpu
> seemed like a fairly clean macro.  

It turns out I missed one issue in my testing that was brought to my attention.  It looks like the issue was just due to the fact that the per cpu queues weren't initialized to store packets.  This updated patch adds a loop that goes through and calls __skb_queue_init_head on all the queues.

Thanks,

Alex
Download attachment "skb_recycle.diff" of type "application/octet-stream" (3789 bytes)