pktgen question
Subsystems (2)
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.
Hi Steve.
Quoted 8 lines · 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 useskb_clone(),
but rather it just bumps theskb->userscount for each "clone" and passes the same skb ptr to the driver.Q: Is that a valid use of
skb->usersor 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).
Signature · 1 line
Evgeniy Polyakov
Quoted 11 lines · Evgeniy Polyakov
Hi Steve.
Quoted 3 lines · 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 theskb->userscount for each "clone" and passes the same skb ptr to the driver.Q: Is that a valid use of
skb->usersor 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.
Quoted 4 lines · Steve Wise
Its a hack that breaks cxgb3 because cxgb3 uses the
skb->cbarea for each skb passed down. So cxgb3 is at fault then? IE a driver cannot use theskb->cbfield 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
Quoted 6 lines · jamal
Quoted 2 lines · Steve Wise
Its a hack that breaks cxgb3 because cxgb3 uses the
skb->cbarea for each skb passed down. So cxgb3 is at fault then? IE a driver cannot use theskb->cbfield if the users count is > 1? Or maybe a driver can _never_ use the cb field?
Quoted 7 lines
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
Signature · 1 line
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
I think pktgen should be cloning the skbs using skb_clone(). Then it will work for all devices, eh?
Quoted 24 lines · Ben Greear
Quoted 6 lines · jamal
Quoted 2 lines · Steve Wise
Its a hack that breaks cxgb3 because cxgb3 uses the
skb->cbarea for each skb passed down. So cxgb3 is at fault then? IE a driver cannot use theskb->cbfield if the users count is > 1? Or maybe a driver can _never_ use the cb field?Quoted 7 lines
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->devis 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
Quoted 1 line · Steve Wise
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
Signature · 1 line
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 3 lines · Ben Greear
Quoted 1 line · Steve Wise
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...
Quoted 3 lines
Maybe a new option:
multi_clone
If the current code is busted, I think it should be fixed.
My 2 cents.
Steve.
Quoted 7 lines · Steve Wise
Quoted 3 lines · Ben Greear
Quoted 1 line · Steve Wise
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.
Quoted 5 lines
Quoted 3 lines
Maybe a new option:
multi_cloneIf the current code is busted, I think it should be fixed.
Well, it works fine when used correctly :)
Ben
Signature · 1 line
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 4 lines
Quoted 1 line
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
Quoted 6 lines · Rick Jones
Quoted 4 lines
Quoted 1 line
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
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
Quoted 8 lines · Ben Greear
Quoted 6 lines · Rick Jones
Quoted 4 lines
Quoted 1 line
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?
Quoted 8 lines
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 withskb_clone()or similar, but it's still more overhead than theskb_getwhich 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?
Quoted 3 lines
Thanks,
Ben
Quoted 17 lines · Steve Wise
Quoted 14 lines · Ben Greear
Quoted 6 lines · Rick Jones
Quoted 4 lines
Quoted 1 line
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 withskb_clone()or similar, but it's still more overhead than theskb_getwhich only bumps a reference count.)
Quoted 8 lines
Quoted 2 lines
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
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
David Miller
maintainerFrom: Ben Greear <[email protected]>
Date: Mon, 08 Oct 2007 14:57:13 -0700
Quoted 2 lines
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.
Quoted 33 lines · David Miller
From: Ben Greear <[email protected]>
Date: Mon, 08 Oct 2007 14:57:13 -0700Quoted 2 lines
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. Thenetif_receive_skb()caller already has atomic access to the receive queue, but those other callback cases do not.
Quoted 6 lines
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?
David Miller
maintainerFrom: Steve Wise <[email protected]>
Date: Mon, 08 Oct 2007 17:46:26 -0500
Quoted 1 line
We're talking about just for pktgen...eh?
My bad, I'm happy to review a patch that uses theskb->destructor in pktgen to achieve this.
Hi,
Quoted 2 lines · Steve Wise
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
Quoted 3 lines
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
David Miller
maintainerFrom: Steve Wise <[email protected]>
Date: Mon, 24 Sep 2007 08:54:13 -0500
Quoted 2 lines
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.