Re: [PATCH 0/2] NET: Multiple queue network device support REPOST

Re: [PATCH 0/2] NET: Multiple queue network device support REPOST

[David Miller]

You didn't address my correction the other day wherein I clarified
for you that my idea was not to store the queue mapping in
skb->priority but rather to shrink skb->priority to a u16 and
add a new u16 skb->queue_mapping or whatever field to store the
necessary information.

You're just posting a set of patches using the same approach again
plus some bug fixes, so there is essentially nothing new for anyone to
review.

Why ask for feedback if you fail to take any of it into consideration?
:-/

RE: [PATCH 0/2] NET: Multiple queue network device support REPOST

[Waskiewicz Jr, Peter P]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net] 
> Sent: Thursday, March 08, 2007 10:22 PM
> To: Waskiewicz Jr, Peter P
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 
> Leech, Christopher
> Subject: Re: [PATCH 0/2] NET: Multiple queue network device 
> support REPOST
> 
> 
> You didn't address my correction the other day wherein I 
> clarified for you that my idea was not to store the queue mapping in
> skb->priority but rather to shrink skb->priority to a u16 and
> add a new u16 skb->queue_mapping or whatever field to store 
> the necessary information.
> 
> You're just posting a set of patches using the same approach 
> again plus some bug fixes, so there is essentially nothing 
> new for anyone to review.
> 
> Why ask for feedback if you fail to take any of it into consideration?
> :-/
> 

This was taken into consideration, and I did reply that my concern for
doing that could cause stale data in the skb if the queue mapping
changed.  If a qdisc was implemented that could change the band to queue
mapping without having to reload the qdisc, the result could have skb's
heading for the wrong queues until the old data was drained from the
bands.  An example:

->enqueue() - maps queue, commits to skb, adds to band
netif_stop_queue(dev) - event is triggered that could cause a qdisc to
remap bands to queues, drain hardware queues
netif_wake_queue(dev) - reconfiguration is complete, resume transmission
->dequeue() - grab an skb enqueued prior to reconfiguration, read queue
from skb, hard_start_xmit() to the wrong queue

Re: [PATCH 0/2] NET: Multiple queue network device support REPOST

[Kok, Auke]

David Miller wrote:
> You didn't address my correction the other day wherein I clarified
> for you that my idea was not to store the queue mapping in
> skb->priority but rather to shrink skb->priority to a u16 and
> add a new u16 skb->queue_mapping or whatever field to store the
> necessary information.
> 
> You're just posting a set of patches using the same approach again
> plus some bug fixes, so there is essentially nothing new for anyone to
> review.
> 
> Why ask for feedback if you fail to take any of it into consideration?
> :-/

Where are the patches anyway? They didn't seem to make it either to netdev nor lkml!

Auke

Re: [PATCH 0/2] NET: Multiple queue network device support REPOST

[David Miller]

From: "Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com>
Date: Thu, 8 Mar 2007 22:42:19 -0800

> This was taken into consideration, and I did reply that my concern for
> doing that could cause stale data in the skb if the queue mapping
> changed.

This is not a problem.

Since the ->enqueue function stores references to the SKBs,
any change of the dev->qdisc has to flush those references
somehow, and it is at that point that you can fixup the
skb queue mappings.

This happens via invoking the qdisc->ops->reset() method.

RE: [PATCH 0/2] NET: Multiple queue network device support REPOST

[Waskiewicz Jr, Peter P]

> This is not a problem.
> 
> Since the ->enqueue function stores references to the SKBs, 
> any change of the dev->qdisc has to flush those references 
> somehow, and it is at that point that you can fixup the skb 
> queue mappings.
> 
> This happens via invoking the qdisc->ops->reset() method.
> 

Thanks Dave.  It seems expensive to change all the skb's if this type of
event occurs, but I suppose it is a corner case and can be tolerated.
Please let me make this change and I'll resubmit.

Also, the first patch set was RFC, and you were the only one to submit
feedback to me.  :-/  So I figured submitting as an official patch for
feedback might get more attention.

And to reiterate what Auke asked, what did happen to the patches?  I see
things getting through the netdev mailing list, but they're not showing
up on www.spinics.net/lists/netdev.  Was it something I did when
sending, or did majordomo hiccup?

Thanks,

-PJ Waskiewicz

Re: [PATCH 0/2] NET: Multiple queue network device support REPOST

[David Miller]

From: "Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com>
Date: Thu, 8 Mar 2007 23:16:58 -0800

> It seems expensive to change all the skb's if this type of
> event occurs,

The reset functions have to walk all the SKBs anyways.