* Re: pktgen terminating condition [not found] ` <20070829044352.GA4133@ludhiana> @ 2007-08-29 13:46 ` jamal 2007-08-29 16:32 ` Robert Olsson 2007-08-29 16:59 ` Mandeep Baines [not found] ` <20070828.220402.15268351.davem@davemloft.net> 1 sibling, 2 replies; 10+ messages in thread From: jamal @ 2007-08-29 13:46 UTC (permalink / raw) To: Mandeep Singh Baines Cc: davem, rick.jones2, msb, netdev, grundler, robert.olsson, venza On Tue, 2007-28-08 at 21:43 -0700, Mandeep Singh Baines wrote: > I interpret this to mean that the interrupt gets generates after a packet > is transferred to the TFIFO on the NIC and the next packet in the ring is > NULL. iow, when tx transits to idle .. > This interrupt gets generates less often then TxOK which gets generated > after every completed packet transmit. So I'm thinking that maybe the > driver was written to use this interrupt instead of TxOK for this reason. > Really just my speculation. It does make sense if you are trying to cut down on tx interupts. It's a little extreme - and if you dont have much memory it may not be the best scheme. OTOH, you have moved to the other extreme imo ;-> You are generating an interupt per tx packet. This may not matter on modern hardware because that NIC seems to be Fast Ethernet. You may wanna heed Dave's advice and just always set the idle on a per-descriptor basis as well as txcomplete on every Xth packet (4 was suggested). Looking at sis900_start_xmit() at the spot where OWN is set on the descriptor, theres possibly another bit in tx_ring[entry].cmdsts you may could set that will ask for tx complete; which makes me wonder: is that "outl(TxENA | inl(ioaddr + cr), ioaddr + cr)" really needed on a per-packet basis? Should it not be sufficient to turn it on once at open()? > Anyway, if I rewrite the driver to use TxOK instead of TxIdle, pktgen > works fine. I think its a good thing pktgen caught this; i am unsure however if it is doing the right thing. Hoping Robert would respond. One thing pktgen could do is restrict the amount of outstanding buffers by using accounting the way sockets do - this at least wont stop progress as it did in your case. > I'm attaching the patch. Looks good to me given the desire. I would bounce it by whoever the maintainer is - they may have some insights on the lazy tx prune habit. cheers, jamal ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-29 13:46 ` pktgen terminating condition jamal @ 2007-08-29 16:32 ` Robert Olsson 2007-08-30 12:08 ` jamal 2007-08-29 16:59 ` Mandeep Baines 1 sibling, 1 reply; 10+ messages in thread From: Robert Olsson @ 2007-08-29 16:32 UTC (permalink / raw) To: hadi Cc: Mandeep Singh Baines, davem, rick.jones2, msb, netdev, grundler, robert.olsson, venza jamal writes: > On Tue, 2007-28-08 at 21:43 -0700, Mandeep Singh Baines wrote: > I think its a good thing pktgen caught this; i am unsure however if it > is doing the right thing. Hoping Robert would respond. > One thing pktgen could do is restrict the amount of outstanding buffers > by using accounting the way sockets do - this at least wont stop > progress as it did in your case. Hello, Yes it's synchronization issue... the test is over and we have sent all pkts to the device but pktgen cannot free the skb for it still has refcounts. IMO must drivers have provisions to handle situation like. I'll guess we can discuss last-resort timer if it should be us, ms or possibly seconds but shouldn't need ping to make this happen. If so we probably have a ICMP packet sitting there waiting instead. Cheers --ro ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-29 16:32 ` Robert Olsson @ 2007-08-30 12:08 ` jamal 2007-08-31 5:19 ` David Miller 0 siblings, 1 reply; 10+ messages in thread From: jamal @ 2007-08-30 12:08 UTC (permalink / raw) To: Robert Olsson Cc: Mandeep Singh Baines, davem, rick.jones2, msb, netdev, grundler, robert.olsson, venza On Wed, 2007-29-08 at 18:32 +0200, Robert Olsson wrote: > Yes it's synchronization issue... the test is over and we have sent > all pkts to the device but pktgen cannot free the skb for it still > has refcounts. Ok, right. I was confusing it with another issue where pktgen could send a lot of packets without waiting for them to be freed; there are some drivers (10G) which may hold onto 8K skbs. A gazillion ooms start spewing ;-> My thinking in resolving that was to do something like waht sockets do and charge pktgen so it doesnt have too many outstanding packets in flight. > IMO must drivers have provisions to handle situation like. Mandeep was saying he found less than a handful that didnt conform. > I'll > guess we can discuss last-resort timer if it should be us, ms > or possibly seconds but shouldn't need ping to make this happen. > If so we probably have a ICMP packet sitting there waiting instead. I think as long as it doesnt affect throughput calculation (it just adds to idle time) its fine. cheers, jamal ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-30 12:08 ` jamal @ 2007-08-31 5:19 ` David Miller 2007-08-31 13:46 ` jamal 0 siblings, 1 reply; 10+ messages in thread From: David Miller @ 2007-08-31 5:19 UTC (permalink / raw) To: hadi Cc: Robert.Olsson, mandeep.baines, rick.jones2, msb, netdev, grundler, robert.olsson, venza From: jamal <hadi@cyberus.ca> Date: Thu, 30 Aug 2007 08:08:40 -0400 > I was confusing it with another issue where pktgen could send a lot of > packets without waiting for them to be freed; there are some drivers > (10G) which may hold onto 8K skbs. A gazillion ooms start spewing ;-> My > thinking in resolving that was to do something like waht sockets do and > charge pktgen so it doesnt have too many outstanding packets in flight. You could implement this quite simply using skb->destructor. It will add some atomics, so on weaker pktgen source systems it might decrease the generators rate. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-31 5:19 ` David Miller @ 2007-08-31 13:46 ` jamal 0 siblings, 0 replies; 10+ messages in thread From: jamal @ 2007-08-31 13:46 UTC (permalink / raw) To: David Miller Cc: Robert.Olsson, mandeep.baines, rick.jones2, msb, netdev, grundler, robert.olsson, venza On Thu, 2007-30-08 at 22:19 -0700, David Miller wrote: > You could implement this quite simply using skb->destructor. Thats what i was thinking .. > It will add some atomics, so on weaker pktgen source systems > it might decrease the generators rate. Indeed. So maybe a config option instead; it has value afaics in the batching case only. Need to experiment. cheers, jamal ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-29 13:46 ` pktgen terminating condition jamal 2007-08-29 16:32 ` Robert Olsson @ 2007-08-29 16:59 ` Mandeep Baines 2007-08-30 11:33 ` jamal 2007-08-31 12:17 ` Daniele Venzano 1 sibling, 2 replies; 10+ messages in thread From: Mandeep Baines @ 2007-08-29 16:59 UTC (permalink / raw) To: hadi Cc: davem, rick.jones2, msb, netdev, grundler, robert.olsson, venza, jeff, nhorman On 8/29/07, jamal <hadi@cyberus.ca> wrote: > On Tue, 2007-28-08 at 21:43 -0700, Mandeep Singh Baines wrote: > > > I interpret this to mean that the interrupt gets generates after a packet > > is transferred to the TFIFO on the NIC and the next packet in the ring is > > NULL. > > iow, when tx transits to idle .. > > > This interrupt gets generates less often then TxOK which gets generated > > after every completed packet transmit. So I'm thinking that maybe the > > driver was written to use this interrupt instead of TxOK for this reason. > > Really just my speculation. > > It does make sense if you are trying to cut down on tx interupts. It's a > little extreme - and if you dont have much memory it may not be the best > scheme. OTOH, you have moved to the other extreme imo ;-> You are > generating an interupt per tx packet. This may not matter on modern > hardware because that NIC seems to be Fast Ethernet. > You may wanna heed Dave's advice and just always set the idle on a > per-descriptor basis as well as txcomplete on every Xth packet (4 was > suggested). My hardware at home is not so modern:( I'm running a AMD Geode NX1750. I like to punish myself:) Actually, it does the job and is very power efficient. Agreed, interrupting every packet is inefficient. A better solution is probably NAPI. I'll work on a new patch. > Looking at sis900_start_xmit() at the spot where OWN is set > on the descriptor, theres possibly another bit in tx_ring[entry].cmdsts > you may could set that will ask for tx complete; which makes me wonder: > is that "outl(TxENA | inl(ioaddr + cr), ioaddr + cr)" really needed on a > per-packet basis? Should it not be sufficient to turn it on once at > open()? > You have to set this bit to restart the Tx State Machine. You could optimize this if you use the TxIdle interrupt to tell you when you really need to set this bit. > > Anyway, if I rewrite the driver to use TxOK instead of TxIdle, pktgen > > works fine. > > I think its a good thing pktgen caught this; i am unsure however if it > is doing the right thing. Hoping Robert would respond. > One thing pktgen could do is restrict the amount of outstanding buffers > by using accounting the way sockets do - this at least wont stop > progress as it did in your case. > > > I'm attaching the patch. > > Looks good to me given the desire. I would bounce it by whoever the > maintainer is - they may have some insights on the lazy tx prune habit. > + nhorman@tuxdriver.com + jeff@garzik.org I'll work on a NAPI patch. Regards, Mandeep ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pktgen terminating condition 2007-08-29 16:59 ` Mandeep Baines @ 2007-08-30 11:33 ` jamal 2007-08-31 12:17 ` Daniele Venzano 1 sibling, 0 replies; 10+ messages in thread From: jamal @ 2007-08-30 11:33 UTC (permalink / raw) To: Mandeep Baines Cc: davem, rick.jones2, msb, netdev, grundler, robert.olsson, venza, jeff, nhorman On Wed, 2007-29-08 at 09:59 -0700, Mandeep Baines wrote: > I'll work on a NAPI patch. It's a GoodThing - go for it. cheers, jamal ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: pktgen terminating condition 2007-08-29 16:59 ` Mandeep Baines 2007-08-30 11:33 ` jamal @ 2007-08-31 12:17 ` Daniele Venzano 2007-08-31 13:50 ` jamal 1 sibling, 1 reply; 10+ messages in thread From: Daniele Venzano @ 2007-08-31 12:17 UTC (permalink / raw) To: Mandeep Baines, hadi Cc: davem, rick.jones2, msb, netdev, grundler, robert.olsson, jeff, nhorman ----- Message d'origine ----- De: "Mandeep Baines" <mandeep.baines@gmail.com> Date: Wed, 29 Aug 2007 09:59:42 -0700 Sujet: Re: pktgen terminating condition >> Looks good to me given the desire. I would bounce it by whoever the >> maintainer is - they may have some insights on the lazy tx prune habit. >> > >+ nhorman@tuxdriver.com >+ jeff@garzik.org > >I'll work on a NAPI patch. Here I'm (the maintainer). Sorry for the delay replying. I don't regard the TxOK solution as something usable for mainline, but it has its use for the users of pktgen. NAPI is the way to go. Thanks, bye. -- Daniele Venzano venza@brownhat.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: pktgen terminating condition 2007-08-31 12:17 ` Daniele Venzano @ 2007-08-31 13:50 ` jamal 0 siblings, 0 replies; 10+ messages in thread From: jamal @ 2007-08-31 13:50 UTC (permalink / raw) To: Daniele Venzano Cc: Mandeep Baines, davem, rick.jones2, msb, netdev, grundler, robert.olsson, jeff, nhorman On Fri, 2007-31-08 at 14:17 +0200, Daniele Venzano wrote: > I don't regard the TxOK solution as something usable for mainline, but it has its > use for the users of pktgen. I dont know if you followed the discussion - by defering the freeing of skbs, you will be slowing down socket apps sending from the local machine. It may be ok if the socket buffers were huge, but that comes at the cost of system memory (which may not be a big deal) Do you by any chance recall why you used the idle interupt instead of txok to kick the prunning of tx descriptors? cheers, jamal ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20070828.220402.15268351.davem@davemloft.net>]
[parent not found: <535ddc6b0708290914k21d0b71fs21ea3b96c6fd7f4d@mail.gmail.com>]
* Re: pktgen terminating condition [not found] ` <535ddc6b0708290914k21d0b71fs21ea3b96c6fd7f4d@mail.gmail.com> @ 2007-08-29 16:26 ` Grant Grundler 0 siblings, 0 replies; 10+ messages in thread From: Grant Grundler @ 2007-08-29 16:26 UTC (permalink / raw) To: Mandeep Baines Cc: David Miller, hadi, rick.jones2, msb, netdev, robert.olsson, venza On 8/29/07, Mandeep Baines <mandeep.baines@gmail.com> wrote: ... > Unfortunately, the TxIdle interrupt would not free the last odd packet. > However, if we want to quiet interrupts couldn't the driver just be > converted NAPI. If this seems reasonable I could work on a patch. It seems reasonable and that's what I was thinking when you submitted the patch to use TXOk. I think NAPI could be used with either TxOK or TxIdle interrupts. For TxIdle to work, the driver would have to recognize one more packet is still in flight and poll for completion of that packet - then re-enable interrupts and switch back to interrupt mode of operation. I just don't know if netperf TCP_RR perf numbers will suffer (assuming TCP_RR is more important than CPU utilization/efficiency on heavy TX loads like pktgen.) grant ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-08-31 13:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1188347327.4305.27.camel@localhost>
[not found] ` <20070829044352.GA4133@ludhiana>
2007-08-29 13:46 ` pktgen terminating condition jamal
2007-08-29 16:32 ` Robert Olsson
2007-08-30 12:08 ` jamal
2007-08-31 5:19 ` David Miller
2007-08-31 13:46 ` jamal
2007-08-29 16:59 ` Mandeep Baines
2007-08-30 11:33 ` jamal
2007-08-31 12:17 ` Daniele Venzano
2007-08-31 13:50 ` jamal
[not found] ` <20070828.220402.15268351.davem@davemloft.net>
[not found] ` <535ddc6b0708290914k21d0b71fs21ea3b96c6fd7f4d@mail.gmail.com>
2007-08-29 16:26 ` Grant Grundler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).