Skip to contentSkip to conversation

NAPI: dev.c change to net_rx_action algorithm.

DISCUSSION Ben Greear 2 participants 6 messages Nov 9, 2002

Subsystems (2)
lore
Share & tools
lore

Ben Greear

author
#0 lore raw

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();
		}
Signature · 3 lines
Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear
#1 lore raw

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

Quoted 57 lines · Ben Greear

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 <[email protected]> <Ben_Greear AT excite.com>
President of Candela Technologies Inc http://www.candelatech.com
ScryMUD: http://scry.wanfear.com http://scry.wanfear.com/~greear

author
#2 lore raw
Quoted 10 lines · jamal

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

Signature · 3 lines
Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear
#3 lore raw
Quoted 1 line · Ben Greear

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.

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 2 lines

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

You tell me.

cheers,
jamal

author
#4 lore raw
Quoted 5 lines · jamal
Quoted 1 line · Ben Greear

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.

Quoted 2 lines

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

Quoted 5 lines
Quoted 2 lines

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();
		}
Quoted 5 lines
Quoted 2 lines

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

Signature · 3 lines
Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear
#5 lore raw
Quoted 2 lines · Ben Greear

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