Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods

Neil Horman
2011-07-20T02:07:51Z

On Tue, Jul 19, 2011 at 05:52:49PM -0700, Ben Greear wrote:
> On 07/19/2011 05:43 PM, Eric Dumazet wrote:
> >Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :
>
> >>I'm sensitive to the performance impact, but I would much rather see a lower
> >>performing pktgen that doesn't randomly crash, and bring the performance back up
> >>in a safe, reliable way. To that end, I've been starting to think about
> >>pre-allocating a ring buffer of skbs with a skb->users count biased up to
> >>prevent driver freeing. That way we could detect 'unused skb's' by a user count
> >>that was at the bias level. Thoughts?
> >>
> >
> >I dont know. I use pktgen maybe once per week and never got a single
> >crash like this. We probably are very few pktgen users in the world, and
> >we use it exactly to avoid calling skb_clone() or other expensive per
> >xmit setup.
> >
> >Just remove pktgen from RedHat kernels, if you dont trust sysadmins.
> ># CONFIG_PKTGEN is not set
> >
> >Alternatively, add a check to problematic drivers to _not_ mess skb if
> >skb_shared(skb) is true : eventually use skb_share_check()
>
> When the features-flags work gets completed so that we can start adding
> new flags, we could add a CANT_DO_MULTI_SKB flag to drivers with known
> issues and then restrict pktgen config accordingly.
>
I think this is a good idea. It lets pktgen dynamically make the clone/share
decision dynamically and only impacts performance for those systems.

> Upstream code already clears skb memory to avoid leaking kernel memory
> contents, so if you take away multi-skb too, pktgen is going to suck
> at what it is supposed to do: run fast as possible.
>
I don't want to take away multi-skb, but I do want pktgen to work reliably. I
think flagging drivers that need unshared skbs is the way to go.

> If you want real fun, use pktgen on a wlan0 device...it will crash
> regardless of whether you use multi-skb or not because of xmit-queue
> number issues :P
>
I'll try that, thanks :)

Regards
Neil