netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* NAPI:  dev.c change to net_rx_action algorithm.
@ 2002-11-09  4:23 Ben Greear
  2002-11-09  4:32 ` jamal
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Greear @ 2002-11-09  4:23 UTC (permalink / raw)
  To: 'netdev@oss.sgi.com'

I tried making this change to dev.c in the net_rx_action method.

So far, I have passed about 25 million packets and only dropped
about 1.5k.  This is with 4 interfaces tx + rx 8kpps (757 byte packets).

rx-ring size is 1024, weight is 24, skb-hotlist is 2048.  This is
the best results so far, but it may be that this code change does
not fare so well in other cases....

Comments?


/*
		if (dev->quota <= 0 || dev->poll(dev, &budget)) {
			local_irq_disable();
			list_del(&dev->poll_list);
			list_add_tail(&dev->poll_list, &queue->poll_list);
			if (dev->quota < 0)
                                 dev->quota += dev->weight;
                         else
                                 dev->quota = dev->weight;
		} else {
			dev_put(dev);
			local_irq_disable();
		}
*/

                 /* This scheme should allow devices to build up 2x their weight in quota
                  * credit.  Heavy users will only get their normal quota.  This should
                  * help let bursty traffic get higher priority. --Ben
                  */
                 if (dev->poll(dev, &budget)) {
                         /* More to do, put these guys back on the poll list */
			local_irq_disable();
			list_del(&dev->poll_list);
			list_add_tail(&dev->poll_list, &queue->poll_list);
                         dev->quota = dev->weight;
                 }
                 else {
                         /* These guys are done, they come off of the poll list */
                         if (dev->quota >= dev->weight) {
                                 dev->quota = (dev->weight << 1); /* max quota of 2x weight */
                         }
                         else {
                                 dev->quota += dev->weight;
                         }
			dev_put(dev);
			local_irq_disable();
		}

-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: NAPI:  dev.c change to net_rx_action algorithm.
  2002-11-09  4:23 NAPI: dev.c change to net_rx_action algorithm Ben Greear
@ 2002-11-09  4:32 ` jamal
  2002-11-09  4:55   ` Ben Greear
  0 siblings, 1 reply; 6+ messages in thread
From: jamal @ 2002-11-09  4:32 UTC (permalink / raw)
  To: Ben Greear; +Cc: 'netdev@oss.sgi.com'



The only thing i can think of is that if it was legal to flog people
youd be on my list. And i would hope you dont enjoy it.
Why do you like to put band-aids to things?
Find the real cause of why you may be having problems then cure it;
putting arbitrary hacks just because you can is rather against
the basic principles of engineering.

cheers,
jamal


On Fri, 8 Nov 2002, Ben Greear wrote:

> I tried making this change to dev.c in the net_rx_action method.
>
> So far, I have passed about 25 million packets and only dropped
> about 1.5k.  This is with 4 interfaces tx + rx 8kpps (757 byte packets).
>
> rx-ring size is 1024, weight is 24, skb-hotlist is 2048.  This is
> the best results so far, but it may be that this code change does
> not fare so well in other cases....
>
> Comments?
>
>
> /*
> 		if (dev->quota <= 0 || dev->poll(dev, &budget)) {
> 			local_irq_disable();
> 			list_del(&dev->poll_list);
> 			list_add_tail(&dev->poll_list, &queue->poll_list);
> 			if (dev->quota < 0)
>                                  dev->quota += dev->weight;
>                          else
>                                  dev->quota = dev->weight;
> 		} else {
> 			dev_put(dev);
> 			local_irq_disable();
> 		}
> */
>
>                  /* This scheme should allow devices to build up 2x their weight in quota
>                   * credit.  Heavy users will only get their normal quota.  This should
>                   * help let bursty traffic get higher priority. --Ben
>                   */
>                  if (dev->poll(dev, &budget)) {
>                          /* More to do, put these guys back on the poll list */
> 			local_irq_disable();
> 			list_del(&dev->poll_list);
> 			list_add_tail(&dev->poll_list, &queue->poll_list);
>                          dev->quota = dev->weight;
>                  }
>                  else {
>                          /* These guys are done, they come off of the poll list */
>                          if (dev->quota >= dev->weight) {
>                                  dev->quota = (dev->weight << 1); /* max quota of 2x weight */
>                          }
>                          else {
>                                  dev->quota += dev->weight;
>                          }
> 			dev_put(dev);
> 			local_irq_disable();
> 		}
>
> --
> Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
> President of Candela Technologies Inc      http://www.candelatech.com
> ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear
>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: NAPI:  dev.c change to net_rx_action algorithm.
  2002-11-09  4:32 ` jamal
@ 2002-11-09  4:55   ` Ben Greear
  2002-11-09  4:58     ` jamal
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Greear @ 2002-11-09  4:55 UTC (permalink / raw)
  To: jamal; +Cc: 'netdev@oss.sgi.com'

jamal wrote:
> 
> The only thing i can think of is that if it was legal to flog people
> youd be on my list. And i would hope you dont enjoy it.
> Why do you like to put band-aids to things?
> Find the real cause of why you may be having problems then cure it;
> putting arbitrary hacks just because you can is rather against
> the basic principles of engineering.
> 
> cheers,
> jamal

I'm looking for the real cause.  The current code does not work
as I want it to.  So, I'm changing things.  I would love to try
out any patch you might feel like contributing.

 From what I can tell, the old code punishes interfaces who are
running slower at any given time.  My change, I believe, will
allow slower interfaces to gather a bit more quota so that when
they are hit with a burst of traffic, they can spend a little
extra time to clear their buffers.

I would also welcome any argument as to why my code is worse
than the existing code.  And theoretical papers be damned,
I see better results with the change below.  Why is that?

Ben

-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: NAPI:  dev.c change to net_rx_action algorithm.
  2002-11-09  4:55   ` Ben Greear
@ 2002-11-09  4:58     ` jamal
  2002-11-09  5:30       ` Ben Greear
  0 siblings, 1 reply; 6+ messages in thread
From: jamal @ 2002-11-09  4:58 UTC (permalink / raw)
  To: Ben Greear; +Cc: 'netdev@oss.sgi.com'



On Fri, 8 Nov 2002, Ben Greear wrote:

> I'm looking for the real cause.

No you are not. You are looking for something that will give you some
quick fix given your current setup.

> The current code does not work
> as I want it to.  So, I'm changing things.  I would love to try
> out any patch you might feel like contributing.

Be systematic about and prove where things are wrong. I even made some
suggestions to you a while back.

>
>  From what I can tell, the old code punishes interfaces who are
> running slower at any given time.

You are totaly wrong. Go and read the paper on DRR.

> My change, I believe, will
> allow slower interfaces to gather a bit more quota so that when
> they are hit with a burst of traffic, they can spend a little
> extra time to clear their buffers.
>
> I would also welcome any argument as to why my code is worse
> than the existing code.  And theoretical papers be damned,
>

You havent proved anything up to this point other than handwaving.
So i dont think you should be damning anything.

> I see better results with the change below.  Why is that?
>

You tell me.

cheers,
jamal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: NAPI:  dev.c change to net_rx_action algorithm.
  2002-11-09  4:58     ` jamal
@ 2002-11-09  5:30       ` Ben Greear
  2002-11-09 14:41         ` jamal
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Greear @ 2002-11-09  5:30 UTC (permalink / raw)
  To: jamal; +Cc: 'netdev@oss.sgi.com'

jamal wrote:
> 
> On Fri, 8 Nov 2002, Ben Greear wrote: 
>>I'm looking for the real cause.
> No you are not. You are looking for something that will give you some
> quick fix given your current setup.

Guilty, and as soon as I get this particular config optomized, I will
move on to the next scenario.  If the next scenario is better by some
other hack, then I will figure out how to reconcile the two, or make
the algorithms dynamic if I have to.

> Be systematic about and prove where things are wrong. I even made some
> suggestions to you a while back.

Things are wrong:  I can't tx and rx 8kpps (50Mbps) on 4 ports simultaneously.
I have fiddled with every magic static constant I could find, so now I'm
fidling with algoritms.  Primary culprit is rx-drop, which means the NIC
is not getting serviced in time, if I understand correctly.  I have 1024
rx-ring, which most agree is too large already, and yet it fills before
I can empty it.

Btw, and you'll love this one, if I change the priority of the irq thread
to -18, things get even better ;)

>> From what I can tell, the old code punishes interfaces who are
>>running slower at any given time.
> 
> 
> You are totaly wrong. Go and read the paper on DRR.

I read the code.  It's likely I'm confused, but here is how
I interpret it.

#  If we are out of quota, refill quota but do not poll.
#    Why shouldn't we poll here?  We wouldn't be in this list
#    If there was nothing to poll, eh?

#  If we have quota, poll, but only refill quota if we still have
#  more work to do.  Busy devices have more work to do.  Slow ones
#  will not.  So, slow devices will have < dev->weight quota much of
#  the time.  If the formerly slow device suddenly gets a large burst of
#  traffic, it's first poll will likely be for a quota smaller than
#  dev->weight.  Why would this be good?

#  If we have quota, but do not not have more work to do after polling,
#  then pop out of the queue.  Note quota is not refilled in this case,
#  again punishing devices that are running slower.

		if (dev->quota <= 0 || dev->poll(dev, &budget)) {
			local_irq_disable();
			list_del(&dev->poll_list);
			list_add_tail(&dev->poll_list, &queue->poll_list);
			if (dev->quota < 0)
                                 dev->quota += dev->weight;
                         else
                                 dev->quota = dev->weight;
		} else {
			dev_put(dev);
			local_irq_disable();
		}

>>I see better results with the change below.  Why is that?
>>
> 
> 
> You tell me.

I think my algorithm is better, at least for this case.  It may
be worse for others, though I'm not sure why it would be.  The
one thing you can be certain of is that I'll let you know if
I figure it out :)

Ben

-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: NAPI:  dev.c change to net_rx_action algorithm.
  2002-11-09  5:30       ` Ben Greear
@ 2002-11-09 14:41         ` jamal
  0 siblings, 0 replies; 6+ messages in thread
From: jamal @ 2002-11-09 14:41 UTC (permalink / raw)
  To: Ben Greear; +Cc: 'netdev@oss.sgi.com'



On Fri, 8 Nov 2002, Ben Greear wrote:

> Btw, and you'll love this one, if I change the priority of the irq thread
> to -18, things get even better ;)

;-> Ok, go ahead and hack the scheduler now.

All, i am saying is that you should stop wildly pointing fingers at a
million things and diagnose what the issue is then come up with a fix;
So far, you have not. It was the rx ring then it is napi/DRR and now it is
the scheduler; and before you find the real cause please stop posting
code like a lunatic.

cheers,
jamal

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2002-11-09 14:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-11-09  4:23 NAPI: dev.c change to net_rx_action algorithm Ben Greear
2002-11-09  4:32 ` jamal
2002-11-09  4:55   ` Ben Greear
2002-11-09  4:58     ` jamal
2002-11-09  5:30       ` Ben Greear
2002-11-09 14:41         ` jamal

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