Subject: Re: [PATCH 2/2] correct dev_alloc_skb kerneldoc

From: Christoph Hellwig <[email-masked]>
Date: Thu, 13 Jul 2006 22:36:16 +0200

On Sat, Jul 08, 2006 at 01:37:38PM -0700, David Miller wrote:
> > My plan is to give dev_alloc_skb a struct netdevice * argument and
> > introduce a alloc_netdev_node so the driver can tell what node the
> > device is on.  This gives a significant speedup for cell.  I already
> > have this implemented in fact but only converted a handfull of drivers.
> > 
> > Does this approach sound okay?
> 
> I do not think it is feasible to add a netdevice argument to
> such a widely used interface.
> 
> I would instead recommend that you add a new interface, and
> we convert drivers gradually.

I gave converting the dev_alloc_skb prototype everywhere a spin and it
doesn't look too bad.  There's only a few places that don't have a
netdevice at hand, and all these look bogus:

 - drivers/atm/{idt77252.c, nicstar.c}, drivers/usb/atm/usbatm.c:
	These use private skb queues and do odd things.  I can't see
	any point for using dev_alloc_skb with it's additional headroom
	reservation here.
 - drivers/isdn/<everything> :
	These don't even have a netdevice!  Also I can't see why they
	need the headroom.
 - drivers/net/{ppp_async.c,drivers/net/ppp_async.c}:
   	They take data from the underlying tty device.  Because of that
	there's not point for he additional headroom.
 - net/irda/*:
   	They allocate the skb in protocol code for TX.  Should probably
	do a normal alloc_skb like all the other protocol code.
 - drivers/infiniband/hw/ipath/ipath_driver.c:
   	Uses skbbuffs in a driver that has absolutely nothing to do
	with networking.  Duh..

I've added the respective maintainers to the cc list here so they can
comment on these usages.

The patchkit for this is at http://verein.lst.de/~hch/patches.skb.tgz, it
includes the first two cleanup patches I posted previously (Any plans
to put them in?), a patch to move __dev_alloc_skb out of line because
otherwise we'd need to include netdevice.h in skbuff.h which creates
lots of problems (and moving it out of lines shaves 10kb off a
allyesconfig), the dev_alloc_skb prototype changes and some experimental
patches to make dev_alloc_skb nodeaware.

> That netdevice argument is useful for other things.  For example,
> you can use it to apply a per-netdev random memory allocation
> failure setting, to test the robustness of drivers.  I came up
> with that idea something like 7 years ago but never got around
> to doing it :)

Sounds cool.  While doing the change I also noticed that assigning
skb->dev can move in there aswell, but I haven't done that change yet.