Re: pktgen terminating condition

Re: pktgen terminating condition

[jamal]

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

Re: pktgen terminating condition

[Robert Olsson]

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

Re: pktgen terminating condition

[jamal]

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

Re: pktgen terminating condition

[David Miller]

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.

Re: pktgen terminating condition

[jamal]

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

Re: pktgen terminating condition

[Mandeep Baines]

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

Re: pktgen terminating condition

[jamal]

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

Re: Re: pktgen terminating condition

[Daniele Venzano]

----- 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

Re: Re: pktgen terminating condition

[jamal]

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

Re: pktgen terminating condition

[Grant Grundler]

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

Re: pktgen terminating condition

[Daniele Venzano]

----- Message d'origine -----
De: jamal <hadi@cyberus.ca>
Date: Fri, 31 Aug 2007 09:50:03 -0400
Sujet: Re: Re: pktgen terminating condition

>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?

That should be asked to the original author of the driver, that I wasn't able to contact when I took over the maintainership several years ago.
I think that since this chip is usually used on cheap/low performance (relatively speaking) devices it was felt that generating an interrupt for each transmitted packet was going to affect the performance of the system too much. At the start, if I remember correctly, this was a chip thought for consumer use, where transmissions won't happen continuously for long periods of time. Then the sis900 started to get used everywhere, from embedded systems to (cheap) server motherboards and the usage scenario changed.


--
Daniele Venzano
venza@brownhat.org