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

[Jiri Pirko <jpirko@redhat.com>]

Tue, Jul 19, 2011 at 10:02:18PM CEST, eric.dumazet@gmail.com 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 <nhorman@tuxdriver.com>
>> Reported-by: Jiri Pirko <jpirko@redhat.com>
>> CC: Eric Dumazet <eric.dumazet@gmail.com>
>> CC: Alexey Dobriyan <adobriyan@gmail.com>
>> CC: David S. Miller <davem@davemloft.net>
>> ---
>
>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 majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html