Commit Message

>-----Original Message----->From: David Miller [mailto:davem@davemloft.net]>Sent: Monday, October 04, 2010 11:33 AM>To: Duyck, Alexander H>Cc: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;>netdev@vger.kernel.org>Subject: Re: ixgbe: normalize frag_list usage>>From: Alexander Duyck <alexander.h.duyck@intel.com>>Date: Mon, 04 Oct 2010 11:04:18 -0700>>> This will not work for RSC due to the fact that it assumes only one>> RSC context is active per ring and that is not the case. There can>be>> multiple RSC combined flows interlaced on the ring.>>Thanks for looking at this Alexander.>>I must have mis-understood what the current code is doing.>>It looked like RSC packets always show up in-order in the RX ring.>>And that they are scanned for by simply combining any sequence of>non-EOP packets up to and including the final EOP one into a frag>list.>>Are the RSC packets are identified via something else entirely?
Dave,
The patch below is kind of what I had in mind for a way to do RSC and maintain
the pointer scheme you are looking for. Consider this patch an RFC for now
since I based this off of Jeff's internal testing tree and so it would need
some modification to apply cleanly to net-next.
The only deviation from a standard frag list is that we are appending the tail
before it actually has any data in it so we have to break things into three
steps. One to add the tail to the end of the frag list, one to merge the
length from the tail into the head, and finally one to clean-up the head->prev
pointer.
If this works for you I will send it to Jeff to add to his tree for testing.
Thanks,
Alex
---
ixgbe: change RSC to use a normalize frag_list usage
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change drops the RSC queue approach and instead creates a normalized
frag_list skb but the tail is kept active and regularly merged into the
host SKB every time it is completed. In order to identify the tail skb
as a tail we set the next pointer to the head skb, and the head skb prev
pointer to the tail.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ixgbe/ixgbe.h | 2 -
drivers/net/ixgbe/ixgbe_main.c | 96 +++++++++++++++++++++++-----------------
2 files changed, 56 insertions(+), 42 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Comments

From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
Date: Tue, 5 Oct 2010 15:45:32 -0700
> The patch below is kind of what I had in mind for a way to do RSC> and maintain the pointer scheme you are looking for. Consider this> patch an RFC for now since I based this off of Jeff's internal> testing tree and so it would need some modification to apply cleanly> to net-next.
Thanks a lot for doing this work Alex.
I'll take a look and give you some feedback soon.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
Date: Tue, 5 Oct 2010 15:45:32 -0700
> The patch below is kind of what I had in mind for a way to do RSC and maintain> the pointer scheme you are looking for. Consider this patch an RFC for now> since I based this off of Jeff's internal testing tree and so it would need> some modification to apply cleanly to net-next.
Can you really not remember the head somewhere?
What I wanted is for everyone to build their frag list SKBs from head to tail,
always. So that I could, as I mentioned in my original posting, do something
like:
struct sk_buff {
union {
struct list_head list;
struct {
struct sk_buff *frag_next;
struct sk_buff *frag_tail_tracker;
};
};
};
The ->frag_tail_tracker is only used in the head SKB to maintain where the
last SKB in the frag list is.
You're tracking the head from the inner SKBs, such that my intended
conventions are not being followed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On 10/6/2010 11:58 PM, David Miller wrote:
> From: "Duyck, Alexander H"<alexander.h.duyck@intel.com>> Date: Tue, 5 Oct 2010 15:45:32 -0700>>> The patch below is kind of what I had in mind for a way to do RSC and maintain>> the pointer scheme you are looking for. Consider this patch an RFC for now>> since I based this off of Jeff's internal testing tree and so it would need>> some modification to apply cleanly to net-next.>> Can you really not remember the head somewhere?
I can track it in the RSC_CB if that works for you. Right now though I
guess I am not seeing the difference between tracking this in
skb->frag_next vs IXGBE_RSC_CB(skb)->frag_head. I think it might help
if you were to provide some functions that demonstrate exactly what you
had in mind for frag list handling. Specifically if you were to add a
function for merging a frag into the frag list, and for how you want to
approach cleaning up the skb->prev/frag_tail_tracker pointer when you
are cleaning up an active frag_list.
> What I wanted is for everyone to build their frag list SKBs from head to tail,> always. So that I could, as I mentioned in my original posting, do something> like:
My change is doing just that. It is building from head to tail. The
only difference is that tail is tracking head since it has to be updated
after the tail is added, and then I can drop next/frag_next back to NULL.
> struct sk_buff {> union {> struct list_head list;> struct {> struct sk_buff *frag_next;> struct sk_buff *frag_tail_tracker;> };> };> };>> The ->frag_tail_tracker is only used in the head SKB to maintain where the> last SKB in the frag list is.
That is what I was doing in the head skb.
> You're tracking the head from the inner SKBs, such that my intended> conventions are not being followed.
What I am doing is tracking the head from the tail skb. The reason for
this is that I need some way to update the len, data_len, and truesize
after I have placed data in the tail via skb_put. All of the other SKBs
in the frag list are just tracking the next like any other SKB in the
frag_list.
Now that I think about it I guess the issue is that I am adding the SKB
to the frag_list before it is complete. I will send another patch out
in the next couple of hours that should address most of the issues.
Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Thu, 07 Oct 2010 12:59:14 -0700
> I can track it in the RSC_CB if that works for you. Right now though> I guess I am not seeing the difference between tracking this in> skb->frag_next vs IXGBE_RSC_CB(skb)->frag_head. I think it might help> if you were to provide some functions that demonstrate exactly what> you had in mind for frag list handling. Specifically if you were to> add a function for merging a frag into the frag list, and for how you> want to approach cleaning up the skb->prev/frag_tail_tracker pointer> when you are cleaning up an active frag_list.
Basically the one helper function will look like this.
static inline void skb_frag_list_add(struct sk_buff *head,
struct sk_buff *new)
{
if (!skb_shinfo(skb)->frag_list) {
skb_shinfo(skb)->frag_list = new;
skb->frag_list_tail = new;
} else {
skb->frag_list_tail->frag_list_next = new;
skb->frag_list_tail = new;
}
}
If you have to track the head from the tail packets, please do
so in a private control block.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html