pktgen question

pktgen question

[Steve Wise]

The pktgen module provides a way to "clone" the skb its using for 
transmission, and allows passing N clones of the originally created skb 
to the driver under test.    However, it doesn't really use skb_clone(), 
but rather it just bumps the skb->users count for each "clone" and 
passes the same skb ptr to the driver.

Q: Is that a valid use of skb->users or should pktgen really be cloning 
the skbuff?

Thanks,

Steve.

Re: pktgen question

[Evgeniy Polyakov]

Hi Steve.

On Sun, Sep 23, 2007 at 11:12:12AM -0500, Steve Wise (swise@opengridcomputing.com) wrote:
> The pktgen module provides a way to "clone" the skb its using for 
> transmission, and allows passing N clones of the originally created skb 
> to the driver under test.    However, it doesn't really use skb_clone(), 
> but rather it just bumps the skb->users count for each "clone" and 
> passes the same skb ptr to the driver.
> 
> Q: Is that a valid use of skb->users or should pktgen really be cloning 
> the skbuff?

It's a hack, but since skb is owned by pktgen only (no copies in some
outside queues or some other access) it is allowed just to bump reference
counter (i.e. 'share' skb in usual notation).

-- 
	Evgeniy Polyakov

Re: pktgen question

[Steve Wise]

Evgeniy Polyakov wrote:
> Hi Steve.
> 
> On Sun, Sep 23, 2007 at 11:12:12AM -0500, Steve Wise (swise@opengridcomputing.com) wrote:
>> The pktgen module provides a way to "clone" the skb its using for 
>> transmission, and allows passing N clones of the originally created skb 
>> to the driver under test.    However, it doesn't really use skb_clone(), 
>> but rather it just bumps the skb->users count for each "clone" and 
>> passes the same skb ptr to the driver.
>>
>> Q: Is that a valid use of skb->users or should pktgen really be cloning 
>> the skbuff?
> 
> It's a hack, but since skb is owned by pktgen only (no copies in some
> outside queues or some other access) it is allowed just to bump reference
> counter (i.e. 'share' skb in usual notation).
>

Its a hack that breaks cxgb3 because cxgb3 uses the skb->cb area for 
each skb passed down.  So cxgb3 is at fault then?  IE a driver cannot 
use the skb->cb field if the users count is > 1?  Or maybe a driver can 
_never_ use the cb field?

Steve.

Re: pktgen question

[jamal]

On Sun, 2007-23-09 at 12:55 -0500, Steve Wise wrote:

> Its a hack that breaks cxgb3 because cxgb3 uses the skb->cb area for 
> each skb passed down.  So cxgb3 is at fault then?  IE a driver cannot 
> use the skb->cb field if the users count is > 1?  Or maybe a driver can 
> _never_ use the cb field?

any layer can use the cb structure whichever way they wish. There are
violations, e.g:
the vlan code also uses the cb field to pass vlan details for hardware
acceleration. How does pktgen affect it though, clone() will just copy
the cb field and pktgen doesnt touch it.
In retrospect, pktgen may have to use clone - ccing Robert Olsson.

cheers,
jamal

Re: pktgen question

[Ben Greear]

jamal wrote:
> On Sun, 2007-23-09 at 12:55 -0500, Steve Wise wrote:
>
>   
>> Its a hack that breaks cxgb3 because cxgb3 uses the skb->cb area for 
>> each skb passed down.  So cxgb3 is at fault then?  IE a driver cannot 
>> use the skb->cb field if the users count is > 1?  Or maybe a driver can 
>> _never_ use the cb field?
>>     
>
> any layer can use the cb structure whichever way they wish. There are
> violations, e.g:
> the vlan code also uses the cb field to pass vlan details for hardware
> acceleration. How does pktgen affect it though, clone() will just copy
> the cb field and pktgen doesnt touch it.
> In retrospect, pktgen may have to use clone - ccing Robert Olsson.
>   
Pktgen abuses the ref count, as far as I can tell.  It works with most 
ethernet drivers,
but that multi-pkt will also fail with things like vlans because the 
skb->dev is changed
as it is transmitted (to the lower-level device).

I'd say just don't use the multi-pkt with pktgen on devices that can't 
handle it.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen question

[Steve Wise]

I think pktgen should be cloning the skbs using skb_clone().  Then it 
will work for all devices, eh?


Ben Greear wrote:
> jamal wrote:
>> On Sun, 2007-23-09 at 12:55 -0500, Steve Wise wrote:
>>
>>  
>>> Its a hack that breaks cxgb3 because cxgb3 uses the skb->cb area for 
>>> each skb passed down.  So cxgb3 is at fault then?  IE a driver cannot 
>>> use the skb->cb field if the users count is > 1?  Or maybe a driver 
>>> can _never_ use the cb field?
>>>     
>>
>> any layer can use the cb structure whichever way they wish. There are
>> violations, e.g:
>> the vlan code also uses the cb field to pass vlan details for hardware
>> acceleration. How does pktgen affect it though, clone() will just copy
>> the cb field and pktgen doesnt touch it.
>> In retrospect, pktgen may have to use clone - ccing Robert Olsson.
>>   
> Pktgen abuses the ref count, as far as I can tell.  It works with most 
> ethernet drivers,
> but that multi-pkt will also fail with things like vlans because the 
> skb->dev is changed
> as it is transmitted (to the lower-level device).
> 
> I'd say just don't use the multi-pkt with pktgen on devices that can't 
> handle it.
> 
> Thanks,
> Ben
> 
> 

Re: pktgen question

[Ben Greear]

Steve Wise wrote:
> I think pktgen should be cloning the skbs using skb_clone().  Then it 
> will work for all devices, eh?
That might work, but it would decrease performance slightly (or, 
increase CPU load at least).

Maybe a new option:  multi_clone

Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen question

[Steve Wise]

Ben Greear wrote:
> Steve Wise wrote:
>> I think pktgen should be cloning the skbs using skb_clone().  Then it 
>> will work for all devices, eh?
> That might work, but it would decrease performance slightly (or, 
> increase CPU load at least).

Perf-wise, you could clone the skbs up front, then deliver them to the 
nic in a tight loop.  This would mitigate the added overhead introduced 
by calling skb_clone() in the loop doing transmits...

> 
> Maybe a new option:  multi_clone
> 

If the current code is busted, I think it should be fixed.

My 2 cents.

Steve.

Re: pktgen question

[Ben Greear]

Steve Wise wrote:
>
> Ben Greear wrote:
>> Steve Wise wrote:
>>> I think pktgen should be cloning the skbs using skb_clone().  Then 
>>> it will work for all devices, eh?
>> That might work, but it would decrease performance slightly (or, 
>> increase CPU load at least).
>
> Perf-wise, you could clone the skbs up front, then deliver them to the 
> nic in a tight loop.  This would mitigate the added overhead 
> introduced by calling skb_clone() in the loop doing transmits...
That only works if you are sending a small number of skbs.  You can't 
pre-clone several minutes worth of 10Gbe traffic
with any normal amount of RAM.

>>
>> Maybe a new option:  multi_clone
>>
>
> If the current code is busted, I think it should be fixed.
Well, it works fine when used correctly :)

Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen question

[Rick Jones]

>> Perf-wise, you could clone the skbs up front, then deliver them to the 
>> nic in a tight loop.  This would mitigate the added overhead 
>> introduced by calling skb_clone() in the loop doing transmits...
> 
> That only works if you are sending a small number of skbs.  You can't 
> pre-clone several minutes worth of 10Gbe traffic
> with any normal amount of RAM.

Does pktgen really need to allocate anything more than some smallish fraction 
more than the depth of the driver's transmit queue?

Of course, even that won't fit in the L1 cache of a processor, so if one is 
really just trying to max-out the NIC it might still have too much overhead, but 
then does pktgen+driver et al actually fit in an L1 cache?

rick jones

Re: pktgen question

[Ben Greear]

Rick Jones wrote:
>>> Perf-wise, you could clone the skbs up front, then deliver them to 
>>> the nic in a tight loop.  This would mitigate the added overhead 
>>> introduced by calling skb_clone() in the loop doing transmits...
>>
>> That only works if you are sending a small number of skbs.  You can't 
>> pre-clone several minutes worth of 10Gbe traffic
>> with any normal amount of RAM.
> 
> Does pktgen really need to allocate anything more than some smallish 
> fraction more than the depth of the driver's transmit queue?

If you want to send sustained high rates of traffic, for more than
just a trivial amount of time, then you either have to play the current
trick with the skb_get(), or you have to allocate a real packet each time
(maybe with skb_clone() or similar, but it's still more overhead than the skb_get
which only bumps a reference count.)

I see no other way, but if you can think of one, please let me know.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen question

[Steve Wise]

Ben Greear wrote:
> Rick Jones wrote:
>>>> Perf-wise, you could clone the skbs up front, then deliver them to 
>>>> the nic in a tight loop.  This would mitigate the added overhead 
>>>> introduced by calling skb_clone() in the loop doing transmits...
>>>
>>> That only works if you are sending a small number of skbs.  You can't 
>>> pre-clone several minutes worth of 10Gbe traffic
>>> with any normal amount of RAM.
>>
>> Does pktgen really need to allocate anything more than some smallish 
>> fraction more than the depth of the driver's transmit queue?
> 
> If you want to send sustained high rates of traffic, for more than
> just a trivial amount of time, then you either have to play the current
> trick with the skb_get(), or you have to allocate a real packet each time
> (maybe with skb_clone() or similar, but it's still more overhead than 
> the skb_get
> which only bumps a reference count.)
> 
> I see no other way, but if you can think of one, please let me know.
> 

You can keep freed skb's that were cloned on a free list, then reuse 
them once freed.  You can detect when the driver frees them by adding a 
destroy function to the skb.  So what will happen is the set of cloned 
skbs needed will eventually settled down to a constent amount and the 
amount will be based on the latency involved in transmitting a single 
skb.  And it should be bounded by the max txq depth.  Yes?  (or am I all 
wet :)

So you would pay the overhead of cloning only until you hit this steady 
state.

Whatchathink?


> Thanks,
> Ben
> 

Re: pktgen question

[Ben Greear]

Steve Wise wrote:
> 
> 
> Ben Greear wrote:
>> Rick Jones wrote:
>>>>> Perf-wise, you could clone the skbs up front, then deliver them to 
>>>>> the nic in a tight loop.  This would mitigate the added overhead 
>>>>> introduced by calling skb_clone() in the loop doing transmits...
>>>>
>>>> That only works if you are sending a small number of skbs.  You 
>>>> can't pre-clone several minutes worth of 10Gbe traffic
>>>> with any normal amount of RAM.
>>>
>>> Does pktgen really need to allocate anything more than some smallish 
>>> fraction more than the depth of the driver's transmit queue?
>>
>> If you want to send sustained high rates of traffic, for more than
>> just a trivial amount of time, then you either have to play the current
>> trick with the skb_get(), or you have to allocate a real packet each time
>> (maybe with skb_clone() or similar, but it's still more overhead than 
>> the skb_get
>> which only bumps a reference count.)
>>
>> I see no other way, but if you can think of one, please let me know.
>>
> 
> You can keep freed skb's that were cloned on a free list, then reuse 
> them once freed.  You can detect when the driver frees them by adding a 
> destroy function to the skb.  So what will happen is the set of cloned 
> skbs needed will eventually settled down to a constent amount and the 
> amount will be based on the latency involved in transmitting a single 
> skb.  And it should be bounded by the max txq depth.  Yes?  (or am I all 
> wet :)
> 
> So you would pay the overhead of cloning only until you hit this steady 
> state.
> 
> Whatchathink?

This skb recycling can certainly work and has been done several
times before.  It never gets into the kernel though.

Also, still a lot more overhead than incrementing the in-use counter
since you will have to re-initialize the pkt (you don't know what
all the underlying devices might have done to it.)

The number needed would be bound by all of the queues involved.  There
could be several queues if you are running pktgen on virtual devices
(with potential queues between virtual devs and hardware devs).

Still, I at least would be interested in seeing a patch if you put
one together.  You should get buy-in from Robert and/or DaveM if you
want it to have a chance to get into the kernel.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen question

[David Miller]

From: Ben Greear <greearb@candelatech.com>
Date: Mon, 08 Oct 2007 14:57:13 -0700

> This skb recycling can certainly work and has been done several
> times before.  It never gets into the kernel though.

Because it doesn't work.

A socket can hang onto a receive packet essentially forever.

You cannot therefore rely upon the network stack to give you the
packet back in some reasonable finite amount of time.  This is simply
the nature of the beast.

Which means that you either:

1) Starve and stop receiving packets when the recycling ring
   runs out because all of those packets are stuck in socket
   buffers.  This is easily DoS'able by users on your system

2) End up allocating new packets anyways, but then what's the
   point of the recycling ring?  It's just a hack that works
   when everything goes as planned, and in real life that is
   rarely the case.

It also makes the driver locking more complicated.  Packet
input occurs in NAPI drivers via netif_receive_skb() which
would be capable of recycling packets back to the same
driver in a recycling scheme.  But the recycling can occur
from other contexts too.  The netif_receive_skb() caller
already has atomic access to the receive queue, but those
other callback cases do not.

That locking issue is just the tip of the iceberg.  Once you
start discussing solutions, all sorts of new issues begin to
pop up.

SKB recycling, just say no.

Re: pktgen question

[Steve Wise]

David Miller wrote:
> From: Ben Greear <greearb@candelatech.com>
> Date: Mon, 08 Oct 2007 14:57:13 -0700
> 
>> This skb recycling can certainly work and has been done several
>> times before.  It never gets into the kernel though.
> 
> Because it doesn't work.
> 
> A socket can hang onto a receive packet essentially forever.
> 
> You cannot therefore rely upon the network stack to give you the
> packet back in some reasonable finite amount of time.  This is simply
> the nature of the beast.
> 
> Which means that you either:
> 
> 1) Starve and stop receiving packets when the recycling ring
>    runs out because all of those packets are stuck in socket
>    buffers.  This is easily DoS'able by users on your system
> 
> 2) End up allocating new packets anyways, but then what's the
>    point of the recycling ring?  It's just a hack that works
>    when everything goes as planned, and in real life that is
>    rarely the case.
> 
> It also makes the driver locking more complicated.  Packet
> input occurs in NAPI drivers via netif_receive_skb() which
> would be capable of recycling packets back to the same
> driver in a recycling scheme.  But the recycling can occur
> from other contexts too.  The netif_receive_skb() caller
> already has atomic access to the receive queue, but those
> other callback cases do not.
> 
> That locking issue is just the tip of the iceberg.  Once you
> start discussing solutions, all sorts of new issues begin to
> pop up.
> 
> SKB recycling, just say no.
> 

We're talking about just for pktgen...eh?

Re: pktgen question

[David Miller]

From: Steve Wise <swise@opengridcomputing.com>
Date: Mon, 08 Oct 2007 17:46:26 -0500

> We're talking about just for pktgen...eh?

My bad, I'm happy to review a patch that uses the
skb->destructor in pktgen to achieve this.

Re: pktgen question

[Robert Olsson]

Hi,

Steve Wise writes:
 > I think pktgen should be cloning the skbs using skb_clone().  Then it 
 > will work for all devices, eh?

 pktgen assumes for "fastpath" sending exclusive ownership of
 the skb. And does a skb_get to avoid final skb destruction so 
 the same skb can be sent over and over. The idea is to avoid 
 memory allocation and keep things in cache to give very high
 packet rates with identical packets.
   I
 But if you need to alter the packet then the skb_get trick can't 
 be done. And you have to turn off "fastpath" with clone_skb 

 > Perf-wise, you could clone the skbs up front, then deliver them to the 
 > nic in a tight loop.  This would mitigate the added overhead introduced 
 > by calling skb_clone() in the loop doing transmits...

 Sure it's can be done. It could replay sequences etc but it will not
 beat the skb_get trick in sending identical packets. It has been
 proposed before but I've avoided such efforts to keep things relatively 
 small and simple. Really pktgen should be reworked to have s small 
 skim in kernel and move the rest of the stuff to userland.

 Cheers.
					--ro

Re: pktgen question

[David Miller]

From: Steve Wise <swise@opengridcomputing.com>
Date: Mon, 24 Sep 2007 08:54:13 -0500

> I think pktgen should be cloning the skbs using skb_clone().  Then it 
> will work for all devices, eh?

The problem is that skb_clone() is (relatively) expensive and
pktgen is trying to just grab a reference to the SKB in
the absolutely cheapest way possible.

In my personal opinion, I think what the drivers that don't
work with pktgen are doing is wrong and they should find
another way to pass state around other than to depend upon
being able to use the ->cb[] area at-will.