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

From: Jiri Pirko <[email-masked]>
Date: Tue, 19 Jul 2011 22:29:23 +0200

Tue, Jul 19, 2011 at 10:02:18PM CEST, [email-masked] wrote:
>Le mardi 19 juillet 2011 à 15:52 -0400, Neil Horman a écrit :
>> This oops was reported recently when running a pktgen script that called for a
>> transmitted skb to be cloned and sent 100 times using the clone_skb command to a
>> bridge device with several attached interface:
>> 
>...
>
>> Turns out the pktgen driver doesn't actually clone skbs, but rather shares them,
>> increasing the reference count of the skb for each send operation.  This works
>> for most drivers because most drivers don't store or care about any state in the
>> skb itself, but several do.  For instance, the above tun/tap driver and other
>> soft drivers (vlans, bonding/bridging), all requeue frames to a physical device,
>> meaning the skb next and prev pointers will be set.  Other drivers also care
>> about skb state.  The virtio_net driver for instance uses the skb->cb space to
>> store a vnet header and several converged adapters adjust the data pointer of an
>> skb to prepend a device control header to the skb.  Drivers expect skbs
>> submitted for i/o to be in their control and unshared with other users, an
>> assumption which pktgen is violating, the result being multiple skb users
>> corrupting one antohers state and producing oopses like the one above.  The
>> solution is to make pktgen clone the skb for each transmit so as to ensure the
>> drivers assumptions about private exclusive access to the skb is maintained.
>> 
>> Tested successfully by myself
>> Signed-off-by: Neil Horman <[email-masked]>
>> Reported-by: Jiri Pirko <[email-masked]>
>> CC: Eric Dumazet <[email-masked]>
>> CC: Alexey Dobriyan <[email-masked]>
>> CC: David S. Miller <[email-masked]>
>> ---
>
>This will kill pktgen performance ?
>
>pktgen is only for sysadmins, and very skilled ones :)

You are right, but it may not cause panic, right? In case this patch
would cause significant performance regression, how about to just forbid
pktgen to run on soft-netdevs ?

Jirka

>
>BTW you forgot to CC pktgen author, Robert Olsson
>
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to [email-masked]
>More majordomo info at  http://vger.kernel.org/majordomo-info.html